SHA3 and SHAKE - New API Design #2084

manastasova · 2024-12-31T00:00:11Z

Issues:

Resolves #CryptoAlg-2810

Description of changes:

AWS-LC supports SHA3 and SHAKE algorithms though low level SHA3_init, SHA3_Update, SHA3_Final and SHAKE_init, SHAKE_Final APIs. Currently, there are two issues with the implementation and usage of SHA3 and SHAKE:

There is no support for SHAKE_Update function. SHAKE is implemented by calling SHAKE_Init, SHA3_Update and SHAKE_Final.
SHAKE_Final allows multiple consecutive calls to enable incremental XOF output generation.

This PR addresses both of them as follows:

Introduce a new API layers - Keccak1600, FIPS202, SHA3 and SHAKE.
- Keccak1600 layer implements KeccakF1600 Absorb and Squeeze functions; Keccak1600 layer does not manage internal input/output buffers.
- FIPS202 layer implements Reset, Init, Update, and Finalize functionalities; FIPS202 layer manages the internal input/output buffers, allowing incremental requests (not necessarily multiple of block size) to Update (Absorb) and Squeeze for input/output processing. (Other functionalities, such as zero-ing of bitstate, block size checks, etc. are also handled by FIPS202 API layer)
- SHA3 layer implements Init, Update, and Final functionalities; SHA3 layer only implements SHA3 algorithm, thus, offers a single-call SHA3_Final function. SHA3_Final will not update internal flags related to XOF functionalities.
- SHAKE layer implements Init, Update, and Finalize and Squeeze functionalities; SHAKE layer implements XOF SHAKE algorithm, therefore, offers a single-call SHAKE_Finalize function that finilized the absorb phase and initializes the squeeze phase. SHAKE_Final will process the first output value (based on the requested output length). Incremental XOF output generation is handled by SHAKE_Squeeze. SHAKE_Squeeze can be called multiple time. SHAKE_Finalize must be called before first call to SHAKE_Squeeze.
SHA3 digest generation: SHA3_Init; SHA3_Update; SHA3_Final;
SHAKE (incremental) output generation: SHAKE_Init; SHAKE_Update; SHAKE_Finalize; (SHAKE_Squeeze*);

Call-outs:

Service indicator is updated:

Inside SHA3 and SHAKE single shot APIs (as previously in AWS-LC);
Inside SHA3_Final (as previously in AWS-LC);
Not updated Inside SHAKE_Finalize (Incremental XOF output generation does not allow to define the last SHAKE call)

All other algorithms that use SHA3/SHAKE APIs are updated:

ML-KEM (SHA3/SHAKE calls will be inlined, thus, additional XOF flag checks and branching will be avoided)
ML-DSA (SHAKE_Squeeze (incremental XOF output functionality) inside ML-DSA is never invoked with the KAT test vectors and gtests)

Testing:

./crypto/crypto_test --gtest_filter="KeccakInternalTest.*"
./crypto/crypto_test --gtest_filter="SHA3Test.*"
./crypto/crypto_test --gtest_filter="SHAKETest.*"

./crypto/crypto_test --gtest_filter="All/PerKEMTest.*"
./crypto/crypto_test --gtest_filter="All/PQDSAParameterTest.*"

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 · 2024-12-31T00:23:07Z

Codecov Report

Attention: Patch coverage is 86.13861% with 14 lines in your changes missing coverage. Please review.

Project coverage is 78.75%. Comparing base (71809b1) to head (6597af1).

Files with missing lines	Patch %	Lines
crypto/fipsmodule/sha/sha3.c	83.52%	14 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2084   +/-   ##
=======================================
  Coverage   78.74%   78.75%           
=======================================
  Files         598      598           
  Lines      103656   103700   +44     
  Branches    14718    14732   +14     
=======================================
+ Hits        81628    81664   +36     
- Misses      21374    21384   +10     
+ Partials      654      652    -2

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

Define Keccak1600, FIPS202, and SHA3/SHAKE API layers Keccak1600 implements absorb and squeeze functionalities. It defines the lowest lever APIs for SHA3/SHAKE; Keccak1600 functions only process complete blocks; internal input/output buffers are handles by higher layer (FIPS202) APIs. FIPS202 APIs handle the internal input/output buffers to allow incremental function calls. FIPS202 layer is used by both SHA3 and SHAKE high level APIs. FIPS202 defines Reset, Init, Update, Finalize APIs. SHA3/SHAKE layer implements the SHA3 and SHAKE algorithms. SHA3 supports Init, Update and Final APIs since it produces a given length digest and should be Finalized in a single Final function call. SHAKE supports Init, Update, Finalize and Squeeze APIs since it can generate arbitrary length output in incremental way. SHAKE_Finalize processes padding and absorb of last input block and generates the first output value; Incremental XOF output generation is defined by SHAKE_Squeeze function which implements the squeeze phase (it does not finalize the absorb, SHAKE_Squeeze can be only called after SHAKE_Finalize

Note: symmetric-shake.c will be inlined, therefore, additional checks for |ctx->padded| flag will be omitted (SHAKE_Finalize should be called once to finalize absorb and initialize squeeze phase; SHAKE_Squeeze should be called to generate additional incremental XOF output).

Update blocksize/rate macro names

…on may not be completed (incremental XOF output request); however, the SHAKE_Finalize function completes the first requested output, thus, SHAKE has processed the first valid output value

Rename SHAKE_Update to SHAKE_Absorb; Define SHAKE_Final as a single-shot API; Defined SHAKE_Squeeze as an incremental (independent) API. It can can be called immediately after SHAKE_Absorb; SHAKE_Squeeze will finalize absorb phase and initiate squeeze phase; When called a signle time SHAKE_Squeeze has the same behavior as SHAKE_Final, SHAKE_Final cannot be called consecutive times; SHAKE_Squeeze can be called multiple times; SHAKE_Squeeze can be called after SHAKE_Final (to squeeze more bytes).

Allow KECCAK1600_STATE_ABSORB, KECCAK1600_STATE_SQUEEZE, KECCAK1600_STATE_FINAL state flag values to prevent incremental usage of SHAKE_Final or SHAKE_Squeeze after SHAKE_Final The cahnge is introduced for consistency reasons KECCAK1600_STATE_ABSORB corresponds to |ctx->padded| = 0 (NOT_PADDED), KECCAK1600_STATE_SQUEEZE corresponds to |ctx->padded| = 1 (PADDED), and KECCAK1600_STATE_FINAL blocks incremental Squeeze calls.

andrewhop · 2025-01-03T23:44:42Z

crypto/fipsmodule/sha/internal.h

-// SHA3_Absorb processes the largest multiple of |r| out of |len| bytes and
+// SHAKE_Init initializes |ctx| with specified |block_size|, returns 1 on
+// success and 0 on failure. Calls SHA3_Init under the hood.
+OPENSSL_EXPORT int SHAKE_Init(KECCAK1600_CTX *ctx, size_t block_size);


Why are you exporting these functions?

andrewhop · 2025-01-03T23:45:38Z

crypto/fipsmodule/sha/internal.h


-// SHA3_Update processes all data blocks that don't need pad through
-// |SHA3_Absorb| and returns 1 and 0 on failure.
+// SHA3_Update check |ctx| pointer and |len| value and calls FIPS202_Update.


Also why are these SHA functions exported here? As far as I can tell they are only used internally and not used in any test files that would need to be exported.

Agree. No need to have them exported. I will address that.

andrewhop · 2025-01-03T23:47:35Z

crypto/fipsmodule/sha/internal.h

-// SHAKE_Init initializes |ctx| with specified |block_size|, returns 1 on
-// success and 0 on failure. Calls SHA3_Init under the hood.
-OPENSSL_EXPORT int SHAKE_Init(KECCAK1600_CTX *ctx, size_t block_size);
+// FIPS202_Reset zeros the bitstate and the amount of processed input.


Can all of these FIPS202 APIs be made static and removed from this header file?

That is a good call. I will update them.

andrewhop · 2025-01-03T23:51:16Z

crypto/dilithium/pqcrystals_dilithium_ref_common/poly.c

-  unsigned int buflen = ML_DSA_POLY_UNIFORM_ETA_NBLOCKS_MAX * SHAKE256_RATE;
-  uint8_t buf[ML_DSA_POLY_UNIFORM_ETA_NBLOCKS_MAX * SHAKE256_RATE];
+  unsigned int buflen = ML_DSA_POLY_UNIFORM_ETA_NBLOCKS_MAX * SHAKE256_BLOCKSIZE;
+  uint8_t buf[ML_DSA_POLY_UNIFORM_ETA_NBLOCKS_MAX * SHAKE256_BLOCKSIZE];


Assuming this buf and buflen are related:

Suggested change

uint8_t buf[ML_DSA_POLY_UNIFORM_ETA_NBLOCKS_MAX * SHAKE256_BLOCKSIZE];

uint8_t buf[buflen];

andrewhop · 2025-01-03T23:52:50Z

crypto/dilithium/pqcrystals_dilithium_ref_common/poly.c

-  unsigned int buflen = POLY_UNIFORM_NBLOCKS*SHAKE128_RATE;
-  uint8_t buf[POLY_UNIFORM_NBLOCKS*SHAKE128_RATE + 2];
+  unsigned int buflen = POLY_UNIFORM_NBLOCKS*SHAKE128_BLOCKSIZE;
+  uint8_t buf[POLY_UNIFORM_NBLOCKS*SHAKE128_BLOCKSIZE + 2];


Why the + 2? Should that be reflected in buflen?

andrewhop · 2025-01-04T00:06:17Z

crypto/fipsmodule/sha/sha3.c

+int FIPS202_Update(KECCAK1600_CTX *ctx, const void *data, size_t len) {
  uint8_t *data_ptr_copy = (uint8_t *) data;
  size_t block_size = ctx->block_size;
  size_t num, rem;

-  if (len == 0) {
-    return 1;
-  }
-
+  // Case |len| equals 0 is checked in SHA3/SHAKE higher level APIs
  // Process intermediate buffer.
  num = ctx->buf_load;


If FIPS202_Update isn't static we can't know for sure all callers of it will pass in valid arguments so it also needs to check that ctx is not null.

Updated as static.

andrewhop · 2025-01-04T00:08:34Z

crypto/fipsmodule/sha/sha3.c

+int SHA3_Update(KECCAK1600_CTX *ctx, const void *data, size_t len) {
+  if (ctx == NULL) {
+    return 0;
+  }
+
+  if (len == 0) {
+    return 1;
+  }
+
+  return FIPS202_Update(ctx, data, len);
+}


I know this was the plan but it looks a little weird that SHA3_update and SHAKE_Absorb are the exact same code, will they ever not be the same?

They are, indeed, the same. That is the reason both - SHA3 and SHAKE were using the same SHA3_Update function. However, it is not consistent to execute SHAKE via SHAKE_Init, SHA3_Update, SHAKE_Final.
With the new design, FIPS202_ layer covers the repeated SHA3 and SHAKE functionalities.
I check the length and ctx pointer in the higher-level functions to avoid additional function calls if the input is not correct. However, I can move these to the lower-level FIPS202 as well if it simplifies the code.

andrewhop · 2025-01-04T00:10:56Z

crypto/fipsmodule/sha/sha3.c

+}
+
+// SHA3_Final should be called once to process final digest value
+// |ctx->padded| flag does not need to be updated


This is a change from before, why doesn't it need to be updated now?

The padded flag was introduced when the (block-wise) incremental XOF functionality was added. This flag should be only needed for SHAKE, since SHA3_Final should not be called multiple times.
I actually want to change the flag to state and add it to all SHA3 and SHAKE functions. ctx->state will be {KECCAK1600_STATE_ABSORB (not_padded), KECCAK1600_STATE_SQUEEZE (padded), KECCAK1600_STATE_FINAL(no_more_squeezes_allowed)}, where SHA3 will move from KECCAK1600_STATE_ABSORB directly to KECCAK1600_STATE_FINAL, preventing from incremental calls to SHA3_Final through EVP_DigestFinal.
The new flag will also allow to define EVP_DigestFinalXOF as a signle-shot API (similar to ossl) with no incremental squeezes after it. Instead, EVP_DigestSqueeze should be only used for incremental XOF.

andrewhop · 2025-01-04T00:22:15Z

crypto/fipsmodule/sha/sha3.c

      return 0;
    }
  }

-  SHA3_Squeeze(ctx->A, md, ctx->md_size, block_size, ctx->padded);
+  Keccak1600_Squeeze(ctx->A, md, len, ctx->block_size, ctx->padded);
  ctx->padded = 1;

  FIPS_service_indicator_update_state();


I don't think we want to add the service indicator to SHAKE_Squeeze right now, because you've updated ML-DSA/ML-KEM to use SHAKE_squeeze this will mark it as approved. We will need to lock/unlock in ML-DSA/KEM and then have it update the service indicator if it is operating in an approved manor.

Previously ML-DSA/KEM called SHA3_Update which doesn't update the indicator it was fine. In a follow up PR when you add the service indicator for SHAKE you'll need to add more tests to all users of SHAKE_Squeeze. I'm also a little surprised the existing service indicator tests for ML-KEM didn't catch this.

Actually, some of the CIs failed when I was not updating it at the end of the (at least) first call to SHAKE_Squeeze.
For the signle-shot SHAKE API I update it at the end of the SHAKE_Final call. For the streaming API SHAKE_Squeeze I can only update it once, when the first output chunk is generated (first call to SHAKE_Squeeze).
However, previously ML-KEM and ML-DSA were calling SHAKE_Final, which calls SHA3_Final, which was updating the service indicator.

Make FIPS202 functions static; Do not export SHA3 and SHAKE internal functions

manastasova closed this Dec 31, 2024

manastasova reopened this Dec 31, 2024

manastasova added 5 commits December 30, 2024 16:57

Add changes to ML-DSA based on SHA3/SHAKE new API Design

50cf7fa

Update blocksize/rate macro names

Update build files in generated-src

4b0b92e

Update service indicator in SHA3_Final

eb992ea

manastasova force-pushed the sha3_absorb_squeeze branch from 50e9841 to eb992ea Compare December 31, 2024 00:57

manastasova added 3 commits December 30, 2024 17:58

Initialize |ctx->padded| to 0 for SHAKE inside SHAKE_Init

d40fbec

Update service indicator at the end of SHAKE_Finalize; The XOF functi…

adb910d

…on may not be completed (incremental XOF output request); however, the SHAKE_Finalize function completes the first requested output, thus, SHAKE has processed the first valid output value

Fix conflicts with MLDSA parameters renaming

02b8085

manastasova marked this pull request as ready for review December 31, 2024 18:24

manastasova requested a review from a team as a code owner December 31, 2024 18:24

Merge branch 'main' into sha3_absorb_squeeze

e61be0d

manastasova requested a review from nebeid January 2, 2025 21:57

manastasova and others added 7 commits January 3, 2025 14:08

Merge branch 'aws:main' into sha3_absorb_squeeze

3008821

Update incremental block-wise SHAKE squeezes in MLKEM

c5d0afd

Update incremental block-wise SHAKE squeezes in MLDSA

b6a5590

Update MLKEM and MLDSA

7edb6c7

Update Keccak state flag in SHA3 functions

7386c1b

andrewhop reviewed Jan 4, 2025

View reviewed changes

manastasova added 2 commits January 3, 2025 18:35

Address code review comments

e424771

Make FIPS202 functions static; Do not export SHA3 and SHAKE internal functions

Add export macro to functions in the tests

6597af1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SHA3 and SHAKE - New API Design #2084

SHA3 and SHAKE - New API Design #2084

manastasova commented Dec 31, 2024

codecov-commenter commented Dec 31, 2024 •

edited

Loading

andrewhop Jan 3, 2025

andrewhop Jan 3, 2025

manastasova Jan 4, 2025

andrewhop Jan 3, 2025

manastasova Jan 4, 2025

andrewhop Jan 3, 2025

andrewhop Jan 3, 2025

andrewhop Jan 4, 2025

manastasova Jan 4, 2025

andrewhop Jan 4, 2025

manastasova Jan 4, 2025

andrewhop Jan 4, 2025

manastasova Jan 4, 2025

andrewhop Jan 4, 2025

manastasova Jan 4, 2025

	uint8_t buf[ML_DSA_POLY_UNIFORM_ETA_NBLOCKS_MAX * SHAKE256_BLOCKSIZE];
	uint8_t buf[buflen];

SHA3 and SHAKE - New API Design #2084

Are you sure you want to change the base?

SHA3 and SHAKE - New API Design #2084

Conversation

manastasova commented Dec 31, 2024

Issues:

Description of changes:

Call-outs:

Testing:

codecov-commenter commented Dec 31, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 31, 2024 •

edited

Loading