Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design for support of HMAC precomputed keys #1574

Merged
merged 31 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fa25f37
Initial design for export of Merkle-Damgard hash state
May 6, 2024
7058ed3
Initial design for support of HMAC precomputed keys
May 6, 2024
3a3a9ad
Addressing comments from review of PR #1574
Jun 10, 2024
f4b67b1
Clarifying definition of EVP_MAX_MD_CHAINING_LENGTH following review
Jun 10, 2024
7148248
Merge branch 'main' into hmac-precompute
Jun 10, 2024
8b5dbea
Remove redundant function declaration in HMAC/SHA256 trampoline
Jun 10, 2024
6734e02
Function comments improvements from review of PR #1574
Jun 13, 2024
7c68e2f
Update function comment in crypto/fipsmodule/hmac/internal.h
fabrice102 Jun 13, 2024
1f6b510
Function comments improvements from review of PR #1574
Jun 14, 2024
a11cb33
Improve error management and check out_len - from review of PR #1574
Jun 14, 2024
41fd25d
Apply suggestions from code review
fabrice102 Jun 14, 2024
6f14823
Apply suggestions from code review
Jun 14, 2024
73debcf
Function comments improvements from review of PR #1574
fabrice102 Jun 14, 2024
580159c
Extend PR #1574 to the other hash functions
Jun 19, 2024
35b4f90
Fix warnings when assert disabled in release mode
Jun 19, 2024
31a27df
Improving comment
Jun 19, 2024
e867e8f
Fix bug in HMAC_with_precompute
Jun 20, 2024
57aec5b
Add service indicator tests for HMAC with precomputed keys
Jun 20, 2024
e16bd40
Fix SHA-512 Init_with_stae/get_state + comment improvements
Jul 5, 2024
866fd7f
Unit test for HMAC_with_precompute service indicator
Jul 5, 2024
7a99180
Unit test for hash Init_with_state/get_state after hashing > 2^32 bits
Jul 8, 2024
7227a2c
Fixing type of some constants
Jul 8, 2024
6df5d2e
Adding unit tests to increase coverage
Jul 8, 2024
43510e5
Style and comment improvements from review of PR aws/aws-lc#1574
Jul 11, 2024
220b8d3
Merge branch 'main' into hmac-precompute
Jul 11, 2024
4ec5b37
Add hmac.errordata
Jul 17, 2024
34e5089
python3 ./util/generate_build_files.py
skmcgrail Jul 17, 2024
f096cfb
Merge branch 'main' into hmac-precompute
nebeid Jul 18, 2024
cd102d6
Fix Windows ARM64 compilation + comment improvements
Jul 18, 2024
1fd75d9
Merge branch 'main' into hmac-precompute
nebeid Jul 19, 2024
d77ee73
Merge branch 'main' into hmac-precompute
nebeid Jul 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions crypto/fipsmodule/hmac/hmac.c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In many external API calls here, we're directly accessing ctx->state or other members of ctx without checking it's not null. I think we should handle this as was done in #1398. This can be its own PR.

Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ struct hmac_methods_st {
void *, const uint8_t *, uint64_t); \
static int AWS_LC_TRAMPOLINE_##HASH_NAME##_get_state(void *, uint8_t *, \
uint64_t *); \
static int AWS_LC_TRAMPOLINE_##HASH_NAME##_Final(uint8_t *, void *); \
static int AWS_LC_TRAMPOLINE_##HASH_NAME##_Init(void *ctx) { \
return HASH_NAME##_Init((HASH_CTX *)ctx); \
} \
Expand Down Expand Up @@ -128,14 +127,14 @@ struct hmac_methods_st {
#define HMAC_METHOD_MAX 8

// FIXME: all hashes but SHA256 have been disabled to check first SHA256
// MD_TRAMPOLINES_EXPLICIT(MD5, MD5_CTX, MD5_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(SHA1, SHA_CTX, SHA_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(SHA224, SHA256_CTX, SHA256_CBLOCK);
MD_TRAMPOLINES_EXPLICIT(SHA256, SHA256_CTX, SHA256_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(SHA384, SHA512_CTX, SHA512_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(SHA512, SHA512_CTX, SHA512_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(SHA512_224, SHA512_CTX, SHA512_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(SHA512_256, SHA512_CTX, SHA512_CBLOCK);
// MD_TRAMPOLINES_EXPLICIT(MD5, MD5_CTX, MD5_CBLOCK)
// MD_TRAMPOLINES_EXPLICIT(SHA1, SHA_CTX, SHA_CBLOCK)
// MD_TRAMPOLINES_EXPLICIT(SHA224, SHA256_CTX, SHA256_CBLOCK)
MD_TRAMPOLINES_EXPLICIT(SHA256, SHA256_CTX, SHA256_CBLOCK)
// MD_TRAMPOLINES_EXPLICIT(SHA384, SHA512_CTX, SHA512_CBLOCK)
// MD_TRAMPOLINES_EXPLICIT(SHA512, SHA512_CTX, SHA512_CBLOCK)
// MD_TRAMPOLINES_EXPLICIT(SHA512_224, SHA512_CTX, SHA512_CBLOCK)
// MD_TRAMPOLINES_EXPLICIT(SHA512_256, SHA512_CTX, SHA512_CBLOCK)

struct hmac_method_array_st {
HmacMethods methods[HMAC_METHOD_MAX];
Expand Down Expand Up @@ -251,8 +250,6 @@ uint8_t *HMAC_with_precompute(const EVP_MD *evp_md, const void *key,

// We have to avoid the underlying SHA services updating the indicator
// state, so we lock the state here.
// TODO: Here and also in HMAC, isn't the service indicator locked
// by the underlying functions anyways?
FIPS_service_indicator_lock_state();

uint8_t precomputed_key[HMAC_MAX_PRECOMPUTED_KEY_SIZE];
Expand All @@ -264,15 +261,20 @@ uint8_t *HMAC_with_precompute(const EVP_MD *evp_md, const void *key,
HMAC_get_precomputed_key(&ctx, precomputed_key, &precomputed_key_len) &&
HMAC_Init_from_precomputed_key(&ctx, precomputed_key, precomputed_key_len,
evp_md) &&
HMAC_Update(&ctx, data, data_len) && HMAC_Final(&ctx, out, out_len);
HMAC_Update(&ctx, data, data_len) &&
HMAC_Final(&ctx, out, out_len);

FIPS_service_indicator_unlock_state();

// Regardless of our success we need to zeroize our working state.
HMAC_CTX_cleanup(&ctx);
OPENSSL_cleanse(precomputed_key, HMAC_MAX_PRECOMPUTED_KEY_SIZE);
if (result) {
HMAC_verify_service_indicator(evp_md);
// Contrary to what happens in the |HMAC| function, we do not update the
// service indicator here (i.e., we do not call
// |HMAC_verify_service_indicator|), because the function
// |HMAC_with_precompute| is not FIPS-approved per se and is only used in
// tests.
return out;
} else {
OPENSSL_cleanse(out, EVP_MD_size(evp_md));
Expand Down Expand Up @@ -485,19 +487,20 @@ int HMAC_set_precomputed_key_export(HMAC_CTX *ctx) {
return 1;
}

size_t HMAC_precomputed_key_size(const HMAC_CTX *ctx) {
return 2 * ctx->methods->chaining_length;
}

int HMAC_get_precomputed_key(HMAC_CTX *ctx, uint8_t *out, size_t *out_len) {
if (HMAC_STATE_PRECOMPUTED_KEY_EXPORT_READY != ctx->state) {
return 0;
}

const size_t chaining_length = ctx->methods->chaining_length;
size_t block_size = EVP_MD_block_size(ctx->md);
assert(2 * chaining_length <= HMAC_MAX_PRECOMPUTED_KEY_SIZE);

*out_len = chaining_length * 2;
assert(*out_len <= HMAC_MAX_PRECOMPUTED_KEY_SIZE);
if (NULL == out) {
// When out is NULL, we just set out_len.
// We keep the state as HMAC_STATE_PRECOMPUTED_KEY_EXPORT_READY
// to allow an actual export of the precomputed key immediately afterward.
return 1;
}
uint64_t i_ctx_n;
uint64_t o_ctx_n;

Expand All @@ -509,10 +512,12 @@ int HMAC_get_precomputed_key(HMAC_CTX *ctx, uint8_t *out, size_t *out_len) {
assert(ok);

// Sanity check: we must have processed a single block at this time
size_t block_size = EVP_MD_block_size(ctx->md);
assert(8 * block_size == i_ctx_n);
assert(8 * block_size == o_ctx_n);

*out_len = chaining_length * 2;
// The context is ready to be used to compute HMAC values at this point.
ctx->state = HMAC_STATE_INIT_NO_DATA;

return 1;
}
Expand Down
1 change: 1 addition & 0 deletions crypto/fipsmodule/hmac/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ extern "C" {
// the computation of HMAC using precomputed keys (internally). It should not
// be used for any other purposes as it outputs the same results as HMAC and is
// slower than HMAC.
// This function does not update the FIPS indicator.
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
OPENSSL_EXPORT uint8_t *HMAC_with_precompute(const EVP_MD *evp_md,
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
const void *key, size_t key_len,
const uint8_t *data,
Expand Down
4 changes: 2 additions & 2 deletions crypto/fipsmodule/sha/internal.h
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void sha512_block_data_order(uint64_t *state, const uint8_t *in,
// custom state. |h| is the hash state in big endian. |n| is the number of bits
// processed at this point. It must be a multiple of |SHA256_CBLOCK*8|.
// It returns one on success and zero on programmer error.
// External users should never use directly this function.
// This function is for internal use only and should never be directly called.
OPENSSL_EXPORT int SHA256_Init_from_state(
SHA256_CTX *sha, const uint8_t h[SHA256_CHAINING_LENGTH], uint64_t n);
nebeid marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -118,7 +118,7 @@ OPENSSL_EXPORT int SHA256_Init_from_state(
// SHA256_Update must be a multiple of the block length |SHA256_CBLOCK|
// (otherwise it fails). It returns one on success and zero on programmer
// error.
// External users should never use directly this function.
// This function is for internal use only and should never be directly called.
OPENSSL_EXPORT int SHA256_get_state(SHA256_CTX *ctx,
uint8_t out_h[SHA256_CHAINING_LENGTH],
uint64_t *out_n);
Expand Down
2 changes: 1 addition & 1 deletion crypto/fipsmodule/sha/sha256.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ static int sha256_final_impl(uint8_t *out, size_t md_len, SHA256_CTX *c) {
return 1;
}

int SHA256_Final(uint8_t out[SHA256_CHAINING_LENGTH], SHA256_CTX *c) {
int SHA256_Final(uint8_t out[SHA256_DIGEST_LENGTH], SHA256_CTX *c) {
return sha256_final_impl(out, SHA256_DIGEST_LENGTH, c);
}

Expand Down
32 changes: 24 additions & 8 deletions crypto/hmac_extra/hmac_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,6 @@ TEST(HMACTest, TestVectors) {
EXPECT_EQ(Bytes(output), Bytes(mac.get(), mac_len));
OPENSSL_memset(mac.get(), 0, expected_mac_len); // Clear the prior correct answer

// Test the precomputed key size is at most HMAC_MAX_PRECOMPUTED_KEY_SIZE
const size_t precomputed_key_len = HMAC_precomputed_key_size(ctx.get());
ASSERT_LE(precomputed_key_len, (size_t) HMAC_MAX_PRECOMPUTED_KEY_SIZE);
uint8_t precomputed_key[HMAC_MAX_PRECOMPUTED_KEY_SIZE];

// Test that the precomputed key cannot be exported without calling
Expand All @@ -252,16 +249,27 @@ TEST(HMACTest, TestVectors) {
ASSERT_FALSE(HMAC_set_precomputed_key_export(ctx2.get()));
ASSERT_FALSE(HMAC_get_precomputed_key(ctx2.get(), precomputed_key, &precomputed_key_len_out));

// Export the precomputed key for later use
// Get the precomputed key length for later use
// And test the precomputed key size is at most HMAC_MAX_PRECOMPUTED_KEY_SIZE
ASSERT_TRUE(HMAC_set_precomputed_key_export(ctx.get()));
ASSERT_TRUE(HMAC_get_precomputed_key(ctx.get(), precomputed_key, &precomputed_key_len_out));
ASSERT_EQ(precomputed_key_len, precomputed_key_len_out);
size_t precomputed_key_len;
HMAC_get_precomputed_key(ctx.get(), nullptr, &precomputed_key_len);
ASSERT_LE(precomputed_key_len, (size_t) HMAC_MAX_PRECOMPUTED_KEY_SIZE);

// Test that at this point the context cannot be used with HMAC_Update
// nor HMAC_Final
// Test that at this point, the context can be used with HMAC_Update
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
ASSERT_FALSE(HMAC_Update(ctx.get(), input.data(), input.size()));
ASSERT_FALSE(HMAC_Final(ctx.get(), mac.get(), &mac_len));

// Export the precomputed key for later use
ASSERT_TRUE(HMAC_get_precomputed_key(ctx.get(), precomputed_key, &precomputed_key_len_out));
ASSERT_EQ(precomputed_key_len, precomputed_key_len_out);

// Test that at this point, the context can be used with HMAC_Update
ASSERT_TRUE(HMAC_Update(ctx.get(), input.data(), input.size()));
ASSERT_TRUE(HMAC_Final(ctx.get(), mac.get(), &mac_len));
EXPECT_EQ(Bytes(output), Bytes(mac.get(), mac_len));
OPENSSL_memset(mac.get(), 0, expected_mac_len); // Clear the prior correct answer

// Test that an HMAC_CTX may be reset with the same key.
ASSERT_TRUE(HMAC_Init_ex(ctx.get(), nullptr, 0, digest, nullptr));
ASSERT_TRUE(HMAC_Update(ctx.get(), input.data(), input.size()));
Expand Down Expand Up @@ -343,6 +351,14 @@ TEST(HMACTest, TestVectors) {
OPENSSL_memset(precomputed_key2, 0, HMAC_MAX_PRECOMPUTED_KEY_SIZE); // Clear the prior correct answer
precomputed_key_len_out2 = 0; // Clear the prior correct answer

// Test that at this point, the context cannot be used to re-export the precomputed key
ASSERT_FALSE(HMAC_get_precomputed_key(ctx.get(), precomputed_key2, &precomputed_key_len_out2));
// Check that precomputed_key_len_out2 and precomputed_key2 were not modified and are still zero
uint8_t zero_precomputed_key[HMAC_MAX_PRECOMPUTED_KEY_SIZE];
OPENSSL_memset(zero_precomputed_key, 0, HMAC_MAX_PRECOMPUTED_KEY_SIZE);
ASSERT_EQ((size_t)0, precomputed_key_len_out2);
ASSERT_EQ(Bytes(zero_precomputed_key, HMAC_MAX_PRECOMPUTED_KEY_SIZE), Bytes(precomputed_key2, HMAC_MAX_PRECOMPUTED_KEY_SIZE));

// Same but initializing the ctx using the precompute key in the first place
ctx.Reset();
ASSERT_TRUE(HMAC_Init_from_precomputed_key(ctx.get(), precomputed_key, precomputed_key_len, digest));
Expand Down
4 changes: 3 additions & 1 deletion include/openssl/digest.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ OPENSSL_EXPORT int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *data,

// EVP_MAX_MD_CHAINING_LENGTH is the largest chaining length supported, in
// bytes. This constant is only for Merkle-Damgard-based hashed functions
// like SHA-1, SHA-2, and MD5.
// like SHA-1, SHA-2, and MD5. The chaining length is defined as the output
// length of the hash in bytes, before any truncation (e.g., 32 for SHA-224 and
// SHA-256, 64 for SHA-384 and SHA-512).
// This constant is only used internally by HMAC.
#define EVP_MAX_MD_CHAINING_LENGTH 64 // SHA-512 has the longest chaining length so far
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
37 changes: 21 additions & 16 deletions include/openssl/hmac.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,19 +169,23 @@ OPENSSL_EXPORT void HMAC_CTX_reset(HMAC_CTX *ctx);
// terminology).
OPENSSL_EXPORT int HMAC_set_precomputed_key_export(HMAC_CTX *ctx);
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved

// HMAC_precompute_size returns the size, in bytes, of the HMAC precomputed key
// that will be produced by |ctx|. On entry, |ctx| must have been set up with
// |HMAC_Init_ex|.
OPENSSL_EXPORT size_t HMAC_precomputed_key_size(const HMAC_CTX *ctx);

// HMAC_get_precomputed_key writes the precomputed key to |out| and sets
// |*out_len| to the length of the result. On entry, the function
// HMAC_set_precomputed_key_export must have been called on |ctx|. Furthermore,
// |out| must contain at least |HMAC_precomputed_key_size| bytes of space. An
// output size of |HMAC_MAX_PRECOMPUTED_KEY_SIZE| will always be large enough.
// After a successful call to HMAC_get_precomputed_key, the context need to be
// set up again by HMAC_Init_*, like after a call to HMAC_Final.
// It returns one on success and zero on programmer error.
// HMAC_set_precomputed_key_export must have been called on |ctx|.
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
//
// If |out| is NULL, only |out_len| is set. After such a call,
// HMAC_get_precomputed_key can directly be called again with a non-null |out|.
// But HMAC_Update and HMAC_Final will still fail.
//
// If |out| is not NULL, |out| must contain at least |out_len| bytes of space
// (as output by HMAC_get_precomputed_key). An output size of
// |HMAC_MAX_PRECOMPUTED_KEY_SIZE| will always be large enough. After a
// successful call to HMAC_get_precomputed_key with a non-NULL |out|, the
// context can directly be used for computing an HMAC, as if it was set up by
// HMAC_Init_*. In particular, in particular, HMAC_Update and HMAC_Final can be
// called. Further calls to HMAC_get_precomputed_key will fail.
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
//
// The function returns one on success and zero on programmer error.
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
//
// The precomputed key is the concatenation:
// precomputed_key = key_ipad || key_opad
Expand All @@ -206,17 +210,18 @@ OPENSSL_EXPORT int HMAC_get_precomputed_key(HMAC_CTX *ctx, uint8_t *out,
// changed and |precomputed_key| is NULL, |ctx| reuses the previous key. It
// returns one on success or zero on failure.
//
// It is possible to mix and match HMAC_Init_ex and
// HMAC_Init_from_precomputed_key.
// If precomputed_key=NULL and md=previous MD or NULL,
// |HMAC_Init_ex| and |HMAC_Init_from_precomputed_key| are interchangeable when
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved
// the input key is NULL. If precomputed_key=NULL and md=previous MD or NULL,
// and if HMAC_Init_ex or HMAC_Init_from_precomputed_key was successfully called
// before, then
// HMAC_Init_ex(ctx, NULL, precomputed_key_len, md, engine=NULL)
// is equivalent to
// HMAC_Init_from_precomputed_key(ctx, NULL, precomputed_key_len, md)
//
// Note: Contrary to HMAC_Init_ex, it is not possible to use the empty key with
// that function. Passing NULL to |precomputed_key| is only allowed on a
// Note: Contrary to input keys to HMAC_Init_ex, which can be the empty key,
// an input precomputed key (for HMAC_Init_from_precomputed_key)
// cannot be the empty key. The notion of empty precomputed key is not
// well-defined. Passing NULL to |precomputed_key| is only allowed on a
// non-initial call where the same md is provided or md == NULL. Any other
// case will make the function fail and return zero.
OPENSSL_EXPORT int HMAC_Init_from_precomputed_key(HMAC_CTX *ctx,
Expand Down