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 1 commit
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
20 changes: 18 additions & 2 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 @@ -481,6 +481,7 @@ int HMAC_set_precomputed_key_export(HMAC_CTX *ctx) {
if (HMAC_STATE_INIT_NO_DATA != ctx->state &&
HMAC_STATE_PRECOMPUTED_KEY_EXPORT_READY != ctx->state) {
// HMAC_set_precomputed_key_export can only be called after Hmac_Init_*
OPENSSL_PUT_ERROR(HMAC, HMAC_R_NOT_CALLED_JUST_AFTER_INIT);
return 0;
}
ctx->state = HMAC_STATE_PRECOMPUTED_KEY_EXPORT_READY;
Expand All @@ -489,18 +490,32 @@ int HMAC_set_precomputed_key_export(HMAC_CTX *ctx) {

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

if (NULL == out_len) {
OPENSSL_PUT_ERROR(EVP, HMAC_R_MISSING_PARAMETERS);
return 0;
}

const size_t chaining_length = ctx->methods->chaining_length;
*out_len = chaining_length * 2;
assert(*out_len <= HMAC_MAX_PRECOMPUTED_KEY_SIZE);
size_t actual_out_len = chaining_length * 2;
assert(actual_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.
*out_len = actual_out_len;
return 1;
}
// When out is not NULL, we need to check that *out_len is large enough
if (*out_len < actual_out_len) {
OPENSSL_PUT_ERROR(HMAC, HMAC_R_BUFFER_TOO_SMALL);
return 0;
}
*out_len = actual_out_len;

uint64_t i_ctx_n;
uint64_t o_ctx_n;

Expand Down Expand Up @@ -559,6 +574,7 @@ int HMAC_Init_from_precomputed_key(HMAC_CTX *ctx,

// We require precomputed_key to be non-NULL, since here md changed
if (NULL == precomputed_key) {
OPENSSL_PUT_ERROR(HMAC, HMAC_R_MISSING_PARAMETERS);
return 0;
}

Expand Down
21 changes: 15 additions & 6 deletions crypto/hmac_extra/hmac_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,15 @@ TEST(HMACTest, TestVectors) {

// Test that the precomputed key cannot be exported without calling
// HMAC_set_precomputed_key_export
size_t precomputed_key_len_out;
size_t precomputed_key_len_out = HMAC_MAX_PRECOMPUTED_KEY_SIZE;
ASSERT_TRUE(HMAC_Init_ex(ctx.get(), key.data(), key.size(), digest, nullptr));
ASSERT_FALSE(HMAC_get_precomputed_key(ctx.get(), precomputed_key, &precomputed_key_len_out));

// Test that the precomputed key cannot be exported if ctx not initialized
// and the precomputed_key_export flag cannot be set
bssl::ScopedHMAC_CTX ctx2;
ASSERT_FALSE(HMAC_set_precomputed_key_export(ctx2.get()));
precomputed_key_len_out = HMAC_MAX_PRECOMPUTED_KEY_SIZE;
ASSERT_FALSE(HMAC_get_precomputed_key(ctx2.get(), precomputed_key, &precomputed_key_len_out));

// Get the precomputed key length for later use
Expand All @@ -261,6 +262,7 @@ TEST(HMACTest, TestVectors) {
ASSERT_FALSE(HMAC_Final(ctx.get(), mac.get(), &mac_len));

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

Expand Down Expand Up @@ -339,24 +341,31 @@ 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 that we get the same precompute_key the second time
ASSERT_TRUE(HMAC_Init_ex(ctx.get(), key.data(), key.size(), digest, nullptr));
// Test that we get an error if the out_len is not large enough
uint8_t precomputed_key2[HMAC_MAX_PRECOMPUTED_KEY_SIZE];
size_t precomputed_key_len_out2;
ASSERT_TRUE(HMAC_Init_ex(ctx.get(), key.data(), key.size(), digest, nullptr));
ASSERT_TRUE(HMAC_set_precomputed_key_export(ctx.get()));
ASSERT_TRUE(HMAC_set_precomputed_key_export(ctx.get())); // testing we can set it twice
precomputed_key_len_out2 = precomputed_key_len - 1; // slightly too short
ASSERT_FALSE(HMAC_get_precomputed_key(ctx.get(), precomputed_key2, &precomputed_key_len_out2));
precomputed_key_len_out2 = 0; // 0-size should also fail
ASSERT_FALSE(HMAC_get_precomputed_key(ctx.get(), precomputed_key2, &precomputed_key_len_out2));

// Test that we get the same precompute_key the second time we correctly call HMAC_get_precomputed_key
precomputed_key_len_out2 = precomputed_key_len; // testing with the out_len is the exact value
ASSERT_TRUE(HMAC_get_precomputed_key(ctx.get(), precomputed_key2, &precomputed_key_len_out2));
ASSERT_EQ(precomputed_key_len, precomputed_key_len_out2);
ASSERT_EQ(Bytes(precomputed_key, precomputed_key_len), Bytes(precomputed_key2, precomputed_key_len));
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
precomputed_key_len_out2 = HMAC_MAX_PRECOMPUTED_KEY_SIZE;
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
// Check that precomputed_key_len_out2 and precomputed_key2 were not modified and are still their original value
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((size_t)HMAC_MAX_PRECOMPUTED_KEY_SIZE, 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
Expand Down
20 changes: 13 additions & 7 deletions include/openssl/hmac.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,14 @@ 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_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|.
//
// If |out| is NULL, only |out_len| is set. After such a call,
// HMAC_get_precomputed_key exports the precomputed key. If |out| is NULL,
// |out_len| is set to the size of the precomputed key. 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
// If |out| is not NULL, |*out_len| must contain the number of bytes of space
// available at |out|. If sufficient, the precomputed key will be written in
// |out| and |out_len| will be updated with the true length. 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 be directly used for computing an HMAC using |HMAC_Update| and
Expand Down Expand Up @@ -220,6 +218,14 @@ OPENSSL_EXPORT int HMAC_Init_from_precomputed_key(HMAC_CTX *ctx,
const EVP_MD *md);


// Errors
fabrice102 marked this conversation as resolved.
Show resolved Hide resolved

#define HMAC_R_MISSING_PARAMETERS 100
#define HMAC_R_BUFFER_TOO_SMALL 102
#define HMAC_R_SET_PRECOMPUTED_KEY_EXPORT_NOT_CALLED 103
#define HMAC_R_NOT_CALLED_JUST_AFTER_INIT 104


// Deprecated functions.

OPENSSL_EXPORT int HMAC_Init(HMAC_CTX *ctx, const void *key, int key_len,
Expand Down
Loading