Skip to content

fix: do not use "digest and sign" for ML-DSA in FIPS mode #5348

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

Merged
merged 2 commits into from
Jun 3, 2025

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Jun 3, 2025

Release Summary:

Resolved issues:

resolves #5343

Description of changes:

The error from #5343 is actually related to signing, not to supporting / loading ML-DSA certificates. It just occurs during certificate loading because we perform a sign/verify operation to test the certificates.

The issue is that to support FIPS for RSA and ECDSA, we have to support a rather hacky "digest_and_sign" variation of signing. The same variation isn't required for ML-DSA, and won't actually work, hence the signing error in FIPS mode. The error didn't occur with the current validated version of AWS-LC-FIPS because that version of AWS-LC doesn't support ML-DSA at all.

The fix is fairly simple: don't use the "digest_and_sign" signing method when using ML-DSA + FIPS. Stick to the more standard "digest_then_sign" method.

Testing:

Our CI does not currently support a version of AWS-LC-FIPS that supports ML-DSA. @dougch is working on that.
In the meantime, I tested locally with the mainline version of AWS-LC built in FIPS mode (./codebuild/bin/install_awslc_fips.sh "$(mktemp -d)" test-deps/awslc-fips-next next from our build scripts).

Without this fix, s2n_handshake_test and s2n_mldsa_test failed with:

Error Message: 'error signing data'
 Debug String: 'Error encountered in /home/ubuntu/s2n-tls/crypto/s2n_pkey_evp.c:140'

which is the same error the customer observed in #5343.

With this fix, all tests pass.

Edit: @dougch also got s2n_handshake_test and s2n_mldsa_test passing in a Codebuild job (link) with the new version of awslc-fips and this fix. The build is failing because s2n_build_test needs to be updated to account for the new S2N_LIBCRYPTO value, but from the output you can see that all other tests pass.

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

@github-actions github-actions bot added the s2n-core team label Jun 3, 2025
@lrstewart lrstewart marked this pull request as ready for review June 3, 2025 05:08
@dougch dougch requested a review from jakemas June 3, 2025 16:25
@jakemas
Copy link
Contributor

jakemas commented Jun 3, 2025

Looks good to me. For background, this is a consequence of how NIST has been standardizing pre-hash mode of late.

@lrstewart lrstewart added this pull request to the merge queue Jun 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 3, 2025
@lrstewart lrstewart added this pull request to the merge queue Jun 3, 2025
Merged via the queue into aws:main with commit a772605 Jun 3, 2025
47 checks passed
@lrstewart lrstewart deleted the mldsa_fips_fix branch June 3, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ML-DSA unsupported with FIPS build of mainline AWS-LC
4 participants