Implement HMAC over SHA3 truncated variants #2484

WillChilds-Klein · 2025-06-12T23:25:52Z

Description

This PR implements HMAC over truncated SHA3 variants as specified in NIST SP 800-224. We do so without externalizing any SHA3/keccak internals, including its context struct. This is done intentionally, as OpenSSL has not yet externalized its SHA3/keccak context struct and we want to leave the door open to future interoperability. To work around this in hmac.h's md_ctx_union, we hard-code the context struct size and add a compile-time assertion that it does not grow larger than the hard-coded value.

SHA3 is unique in supported HMAC digests in that, due to differences between its sponge construction 8000 and others' Merkle-Dåmgard constructions, it does not support pre-computed keys. To accommodate this difference, we refactor the relevant code generation macros and relevant unit tests.

The HMAC-SHA3 feature gap was discovered in a somewhat roundabout way. While preparing a pull request to add AWS-LC to upstream CPython's CI, I discovered that CPython's ./configure script's compile probe failed to detect libcrypto support for linking hashlib. The compile probe referenced NID_blake2b512, which AWS-LC does not support. The consequence of this was that CPython used its HACL implementations for hashlib instead of linking AWS-LC. This did not affect our ssl integration, as AWS-LC always uses its own hash functions for TLS.

Call-outs

KAT vectors in hmac_tests.txt were calculated using a simple python (linked against system OpenSSL, not AWS-LC) session like so:

$ python3
Python 3.10.12 (main, May 27 2025, 17:12:29) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import hmac, hashlib
>>> def mac(h, k):
...     return hmac.new(k, b"My test data", h).hexdigest()
...
>>> mac(hashlib.sha3_224, b"123456")
'43e3747b2309db263d7459de1c7fab297bc2bea07094250929f96188'

Testing

converted relevant wycheproof JSON files to .txt, added them to unit tests
CPython integration test, now modified to actually link hashlib against AWS-LC

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

codecov-commenter · 2025-06-12T23:48:50Z

Codecov Report

Attention: Patch coverage is 98.97959% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.87%. Comparing base (0f76ff1) to head (e59c709).
Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
crypto/hmac_extra/hmac_test.cc	97.36%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2484      +/-   ##
==========================================
- Coverage   78.91%   78.87%   -0.05%     
==========================================
  Files         622      640      +18     
  Lines      108849   109638     +789     
  Branches    15421    15528     +107     
==========================================
+ Hits        85902    86475     +573     
- Misses      22276    22466     +190     
- Partials      671      697      +26

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

crypto/hmac_extra/hmac_test.cc

crypto/fipsmodule/hmac/hmac.c

andrewhop · 2025-06-24T20:34:53Z

crypto/hmac_extra/hmac_test.cc

+    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 and a null md


Why did you move these tests? Did you change anything else?

I moved them here to retain full coverage on all HMAC variants, trying to minimize duplication/redundancy in TestVectorsPrecomputedKey. I changed the comments, but the test code changes here are strictly code motion -- teasing out precomputed-specific cases and copying some portions of the test for setup and coverage.

crypto/fipsmodule/hmac/hmac.c

andrewhop · 2025-06-24T20:58:10Z

crypto/fipsmodule/sha/sha3.c

  FIPS_service_indicator_update_state();
  return 1;
 }

+int SHA3_224_Init(KECCAK1600_CTX *ctx) {


We already have all of these in digest.c, can you just update those to not be static?

I don't think so. The signatures are different (KECCAK1600_CTX vs. EVP_MD_CTX), so digests.c has those static wrappers to wrap SHA1 and SHA2's ctx-specific public functions. For SHA3, we follow the same pattern (e.g. SHA512_256_Update also has a corresponding static function in digests.c), only the ctx-specific functions here aren't exposed in public headers.

andrewhop · 2025-06-24T20:59:01Z

include/openssl/digest.h

@@ -181,7 +181,7 @@ OPENSSL_EXPORT int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *data,

 // EVP_MAX_MD_BLOCK_SIZE is the largest digest block size supported, in
 // bytes.
-#define EVP_MAX_MD_BLOCK_SIZE 128  // SHA-512 is the longest so far.
+#define EVP_MAX_MD_BLOCK_SIZE 144      // SHA3-224 has the largest block size so far


Does anything check/enforce this?

Indirectly, for digests used with HMAC. It's checked here at compile time.

andrewhop · 2025-06-24T21:30:19Z

crypto/fipsmodule/hmac/hmac.c

+    out->methods[idx-1].get_state =                                          \
+        AWS_LC_TRAMPOLINE_##HASH_NAME##_get_state;                           \
+  }
+
 DEFINE_LOCAL_DATA(struct hmac_method_array_st, AWSLC_hmac_in_place_methods) {
  OPENSSL_memset((void*) out->methods, 0, sizeof(out->methods));
  int idx = 0;
  // Since we search these linearly it helps (just a bit) to put the most common ones first.
  // This isn't based on hard metrics and will not make a significant different on performance.
  // FIXME: all hashes but SHA256 have been disabled to check first SHA256


Do you know what this FIXME is referencing?

I do not. It was added with the pre-computed keys change. Do you know @fabrice102?

My mistake. It should be removed. It was for an intermediary commit to show how the changes would look like. But this is no more true.

andrewhop · 2025-06-24T21:32:25Z

crypto/fipsmodule/hmac/hmac.c

+// For merkle-daamgard constructions, we also define functions for  manipulating
+// hash state |h| to calculate or use precomputed keys. These are not applicable
+// to Keccak/SHA3.


Is that not allowed or just not implemented (yet)?

The internal hash state is not readily exportable in SHA3/Keccak in the way that it is in merkle-damgard, so these functions are not applicable. I'll clarify the comment.

tests/ci/integration/python_patch/main/aws-lc-cpython.patch

crypto/fipsmodule/hmac/hmac.c

samuel40791765 · 2025-06-24T22:02:40Z

include/openssl/hmac.h

@@ -250,12 +250,17 @@ OPENSSL_EXPORT int HMAC_CTX_copy(HMAC_CTX *dest, const HMAC_CTX *src);
 // Private functions
 typedef struct hmac_methods_st HmacMethods;

-// We use a union to ensure that enough space is allocated and never actually bother with the named members.
+// We use a union to ensure that enough space is allocated and never actually
+// bother with the named members. We do not externalize SHA3 ctx definition,


Should we mention that SHA3 ctx is referring to KECCAK1600_CTX here? It doesn't seem immediately obvious.

I've intentionally kept mentions of KECCAK1600_CTX out of the public headers. Developers will be able to read explanatory comments in sha/internal.h to figure out what's going on.

samuel40791765 · 2025-06-24T22:05:06Z

include/openssl/hmac.h

 union md_ctx_union {
  MD5_CTX md5;
  SHA_CTX sha1;
  SHA256_CTX sha256;
  SHA512_CTX sha512;
+  uint8_t sha3[400];
 };


A bit of a bolder change, but could we move the md_ctx_union union to our internal headers? I don't see any existing OpenSSL reference to the union. That way we could abstract 400 to a #define variable instead of a hard-coded number.

If we're not comfortable doing so, we can leave this as is though.

I agree that it would be preferable to keep this internal and make HMAC_CTX fully opaque, but it's in our public headers today, so moving it has the potential to break downstream builds. I'd rather not make a breaking change unless we need to.

crypto/fipsmodule/sha/internal.h

samuel40791765 · 2025-06-24T22:09:42Z

include/openssl/digest.h

@@ -181,7 +181,7 @@ OPENSSL_EXPORT int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *data,

 // EVP_MAX_MD_BLOCK_SIZE is the largest digest block size supported, in
 // bytes.
-#define EVP_MAX_MD_BLOCK_SIZE 128  // SHA-512 is the longest so far.
+#define EVP_MAX_MD_BLOCK_SIZE 144      // SHA3-224 has the largest block size so far


Had to cross reference the math being done in crypto/fipsmodule/sha/internal.h. Seems a bit magical that SHA3-224 has the largest block size now.

SHA3's block size reduces as its output size grows. I would link to that SP here, but it's not applicable to all digests -- just those approved for HMAC by NIST.

fabrice102 · 2025-06-26T15:37:32Z

This looks good to me.

Minor comment:

SHA3 is unique in supported HMAC digests in that, due to differences between its sponge construction and others' Merkle-Dåmgard constructions, it does not support pre-computed keys. To accommodate this difference, we refactor the relevant code generation macros and relevant unit tests.

Technically, I think we could do the same and support pre-computed keys by exporting the state after absorbing the HMAC key. However, as mentioned by @WillChilds-Klein in #2484 (comment), exporting the state is not readily available. Furthermore, I don't think there is a good use case to support pre-computed keys for HMAC-SHA-3. If there ever is in the future, it would not be difficult to add this support and still be backward compatible.

So I agree that we should not be supporting HMAC-SHA-3 pre-computed keys.

This comment was marked as spam.

Sign in to view

WillChilds-Klein added 2 commits June 13, 2025 21:28

Build works, untested against python

a99220c

Partial fix, still hitting assert failure

cda6444

WillChilds-Klein force-pushed the hmac-sha3 branch from 8513c0a to cda6444 Compare June 13, 2025 21:28

WillChilds-Klein added 2 commits June 16, 2025 15:15

Fix SHA3 CBLOCK block size constants, python test_hmac passing!!

2d7708f

Revert HMAC MD order

5b7fd44

WillChilds-Klein mentioned this pull request Jun 16, 2025

gh-135401: Test AWS-LC as a cryptography library in CI python/cpython#135402

Draft

WillChilds-Klein added 2 commits June 16, 2025 19:48

Revert edit to EVP_MAX_MD_SIZE

f6e042a

Re-internalize KECCAK1600_CTX and use compile-time assertion for safety

08891f1

This comment was marked as spam.

Sign in to view

WillChilds-Klein added 4 commits June 17, 2025 17:02

Clean up TODOs, code formatting, unimpld stubs

94546aa

Remove unnecessary exports from sha/internal.h

5846475

Re-export SH3_Init for tests

09c1e3c

Split out precomputed key macros, delete unnecessary stubs

8ad9088

This comment was marked as spam.

Sign in to view

WillChilds-Klein added 2 commits June 17, 2025 19:35

Add wycheproof KATs to test harness

ce03e84

Add SHA3 entries to hmac_tests.txt, factor out pre-computed key tests

bf66a63

This comment was marked as spam.

Sign in to view

WillChilds-Klein added 2 commits June 19, 2025 16:54

Cover precomputed-functions called with unsupported digest

0d09cd1

Run "python3 ./util/generate_build_files.py"

eccc231

This comment was marked as spam.

Sign in to view

WillChilds-Klein added 2 commits June 19, 2025 17:24

Check digest support first thing

d2587a3

Tweak non-precomputed key coverage

c27e9a5

WillChilds-Klein requested review from samuel40791765 and justsmth June 20, 2025 15:21

WillChilds-Klein marked this pull request as ready for review June 20, 2025 15:21

WillChilds-Klein requested a review from a team as a code owner June 20, 2025 15:21

Fix internal doc comments

578ab1f

WillChilds-Klein enabled auto-merge (squash) June 20, 2025 16:52

picnixz reviewed Jun 22, 2025

View reviewed changes

crypto/hmac_extra/hmac_test.cc Outdated Show resolved Hide resolved

crypto/fipsmodule/hmac/hmac.c Outdated Show resolved Hide resolved

WillChilds-Klein added 2 commits June 23, 2025 15:58

Remove redundant null check, update KAT names

1a40c87

Run "python3 ./util/generate_build_files.py"

3d44f1a

andrewhop reviewed Jun 24, 2025

View reviewed changes

samuel40791765 reviewed Jun 24, 2025

View reviewed changes

Null-check, clarify comments

bdf8beb

WillChilds-Klein requested a review from andrewhop June 24, 2025 22:59

More PR feedback

e59c709

WillChilds-Klein requested a review from samuel40791765 June 24, 2025 23:21

samuel40791765 approved these changes Jun 25, 2025

View reviewed changes

Implement HMAC over SHA3 truncated variants #2484

Are you sure you want to change the base?

Implement HMAC over SHA3 truncated variants #2484

Conversation

Uh oh!

Description

Call-outs

Testing

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Codecov Report

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!