SHA3 and SHAKE - New API Design

[manastasova]

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 new API layers - FIPS202, SHA3 and SHAKE.

  • Keccak1600 layer (Keccak1600_Squeeze/Absorb Layer (rename) #2097) 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).
    • FIPS202 layer implements all common behavior between SHA3 and SHAKE algorithms.
    • FIPS202 layer checks/updates the |ctx->state| flag when handling a common behavior between SHA3 and SHAKE algorithms. |ctx->state| is updated in the higher level SHA3_ SHAKE_ API layer when the behavior of both algorithms diverges (SHAKE can allow incremental squeezes).
  • 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 update internal |ctx->state| flag to prevent any sequential calls.
  • SHAKE layer implements XOF SHAKE algorithm, therefore, offers Init, Absorb, Squeeze, and Final functionalities;
    • SHAKE layer implements Init, and Absorb, Squeeze with incremental call support for absorb (byte-wise) and squeeze (block-wise).
    • SHAKE layer implements a single-call SHAKE_Final function that generates an arbitrary length output and finalizes SHAKE. Incremental XOF output generation is handled by |SHAKE_Squeeze|. |SHAKE_Squeeze| can be called multiple times. SHAKE_Final should be called only once.

• KECCAK600_CTX struct updates:

  • Remove |padded| field
  • Introduce |state| field
    • |state| can be |KECCAK1600_STATE_ABSORB|, |KECCAK1600_STATE_SQUEEZE|, |KECCAK1600_STATE_FINAL|
    • |KECCAK1600_STATE_ABSORB| - allows incremental absorbs until the state is changed
    • |KECCAK1600_STATE_SQUEEZE| - allows incremental squeezes for |SHAKE_Squeeze|
    • |KECCAK1600_STATE_Final| - prevents from incremental squeezes via |SHAKE_Final| and prevents from consecutive calls to |SHA3_Final| (Final functions are single-shot functions).

SHA3 vs SHAKE algorithms (APIs usage):

• SHA3 digest generation: SHA3_Init; SHA3_Update; SHA3_Final;
• SHAKE (single-shot-output) output generation: SHAKE_Init; SHAKE_Absorb; SHAKE_Final;
• SHAKE (incremental) output generation: SHAKE_Init; SHAKE_Absorb; 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);
• Inside SHAKE_Final (Single-Shot XOF Final output generation as previously in AWS-LC);
• Inside SHAKE_Squeeze (Streaming XOF Squeezes output generation updates the service indicator after each extendable output update);

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

• ML-KEM (SHA3/SHAKE calls will be inlined later)
• 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]

Codecov Report

Attention: Patch coverage is 78.18182% with 24 lines in your changes missing coverage. Please review.

Project coverage is 78.96%. Comparing base (cc9c9f0) to head (873f017).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crypto/fipsmodule/sha/sha3.c	75.00%	21 Missing ⚠️
crypto/ml_dsa/ml_dsa_ref/poly.c	76.92%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2098      +/-   ##
==========================================
- Coverage   78.97%   78.96%   -0.02%     
==========================================
  Files         611      611              
  Lines      105552   105599      +47     
  Branches    14950    14963      +13     
==========================================
+ Hits        83361    83382      +21     
- Misses      21538    21564      +26     
  Partials      653      653              

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

[justsmth]

Do we need to represent a failure state if one of the operations were to fail (or only partially succeed). Is it possible to get a KECCAK1600_CTX into a no longer usable state?

[manastasova]

I believe this should be covered by the returns of the functions. I don't think an additional value for |state| is needed since the purpose is to restrict the number/sequence of: absorb (only before padding is performed), squeeze (incremental squeezes are performed with no limitation), final (one call to final is allowed) function. Once a squeeze is called, no further absorb should be allowed, once final is called, no further absorb, squeeze, or final is allowed.

[justsmth]

Verify that data != NULL

[justsmth]

Should this instead use OPENSSL_memset? (Here and other places...)

[manastasova]

Thanks, Justin. Updated it.

[justsmth]

Should this instead use OPENSSL_memcpy? (Here and other places...)

[justsmth]

Verify that md != NULL and ctx != NULL.

[manastasova]

Added these check in all functions! Thank you!
They are checked in the higher EVP_ layer functions, however, since SHA3/SHAKE functions are also internally called, it is worth checking the values as well.

[justsmth]

I agree that the checks here should not be needed in this "layer" if we're confident the parameters are checked on all logic paths going into these functions. What I noticed was that there was some inconsistency between the functions, where some were checking certain parameters and others were not, and it wasn't clear to me why they weren't all performing the same checks.

[justsmth]

This comment feels like it's in the wrong place.

[manastasova]

Thanks! Fixed it.

[nebeid]

If I understood correctly,

Suggested change

	// KECCAK1600_STATE_FINAL can be called once. SHAKE_Squeeze cannot be called after SHAKE_Final.
	// SHAKE_Squeeze should be called for streaming XOF output.
	// KECCAK1600_STATE_FINAL is set once |SHAKE_Final| is called so that |SHAKE_Squeeze| cannot be called anymore.

[manastasova]

Thanks!

[nebeid]

Does it not do what it used to do: "processes all full blocks of the input data"

[manastasova]

SHA3_Update technically does the same: "processes all data blocks that don't need pad through". However, now it is performed via call to FIPS202_Update, which internally "processes all data blocks that don't need pad through".

[nebeid]

Suggested change

	// It processes the data through |Keccak1600_Squeeze| and returns 1 on success
	// It then calls |Keccak1600_Squeeze| and returns 1 on success

[nebeid]

Is there a historical reason why we chose to input bitlength to SHA3_Init and block_size to SHAKE_Init?

[manastasova]

That's a good observation!
I see it as more intuitive to consider the digest length as an identifier for the security level of the hashing algs rather than the block size (as it is for SHA2)
The block size for SHAKE servers as the SL identifier based on the lack of digest lengths.
I can change the SHA3_Init to take the block size as a parameter. However, the reverse, changing SHAKE_Init won't be possible, so I think it may be more confusing.

[nebeid]

I think it's fine to leave the bitlength in SHA3_Init because there isn't an equivalent function for SHA2 that we need to align with (e.g. using byte length). If we used bitlength with SHAKE_Init, it would identify 128 or 256. It's minor anyway and if it makes more sense to you this way, that's good.

[nebeid]

Would you like to put a comment here like
'''
// KECCAK1600_STATE_SQUEEZE is set when |SHAKE_Squeeze| is called.
// It remains set while |SHAKE_Squeeze| is called repeatedly to output chunks of the XOF output.
'''

[manastasova]

Thanks, Nevine. Added!

[nebeid]

The state is updated within the function, though.

[manastasova]

Yes, my confusion here!
The flag does not need to be updated inside SHA3_Final incorrect use will be prevented in the EVP_ high level functions (i.e., EVP_DigestFinal will cleanse the |ctx->digest| value. Later on, if EVP_DigestUpdate or EVP_DigestFinal is/are called incorrectly after EVP_DigestFinal, the check on ctx->digest| == NULL will prevent from executing EVP_DigestUpdate or EVP_DigestFinal)
Therefore, EVP_DigestFinal cannot be called incorrectly.
However, if we want to prevent the incorrect use of the low level internal APIs SHA3_Final, then the flag update will prevent calling SHA3_Final more then once via state check in FIPS202_Finalize. The flag update here would also prevent calling SHA3_Update after SHA3_Final via state check in FIPS202_Update.
I would consider your recommendation here. For now I will leave the flag update and will remove the comment.

[nebeid]

Suggested change

	// SHAKE_Final is a single-shot API and can be called once to finalize absorb and squeeze phases
	// SHAKE_Final is to be called once to finalize absorb and squeeze phases

I think "single-shot" here is not the right term because it's used with non-streaming APIs such as SHAKE_256()

[manastasova]

I agree. Thank you!

[nebeid]

Suggested change

	// SHAKE_Squeeze can be called multiple times
	// SHAKE_Squeeze should be called for incremental XOF output
	// SHAKE_Squeeze can be called multiple time for incremental XOF output

[nebeid]

Maybe move this comment one line up because it's talking about skipping this if statement (if I understood correctly).

[nebeid]

Do we have unit tests for SHAKE_Squeeze or we're relying on ml_dsa and ml_kem?

[manastasova]

I am adding multiple tests for SHAKE_Squeeze in #2155. Currently, the SHAKE_Squeeze is not exposed through the EVP functions, therefore, I only tested it through ML-KEM.
Actually, I noticed that ML-DSA test vectors never enter in the Squeeze loop (there is always enough mod q samples to generate the entire matrix of polys). I noted that in the call-outs in the PR description since I though it is an interesting observation.

[nebeid]

I think it's fine to leave the bitlength in SHA3_Init because there isn't an equivalent function for SHA2 that we need to align with (e.g. using byte length). If we used bitlength with SHAKE_Init, it would identify 128 or 256. It's minor anyway and if it makes more sense to you this way, that's good.