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

CAST and PCT for ML-DSA #2148

Merged
merged 18 commits into from
Feb 6, 2025
Merged

CAST and PCT for ML-DSA #2148

merged 18 commits into from
Feb 6, 2025

Conversation

jakemas
Copy link
Contributor

@jakemas jakemas commented Jan 29, 2025

Issues:

Resolves #CryptoAlg-2886

Related to #1846 and #1969

Description of changes:

As part of the Implementation Guidance for FIPS 140-3 and the Cryptographic Module Validation Program validation there is requirement that states:

Pairwise Consistency Tests (PCT)

Per the I.G guidance a PCT shall be conducted for every generated public and private key pair for the applicable approved algorithm.

a pairwise consistency test (PCT) shall be conducted for every generated public and private key pair for the applicable approved algorithm. For approved algorithms from FIPS 186-5, SP 800-56Arev3 or SP 800-56Brev2, or FIPS 204, if at the time a PCT on a key pair is performed it is known whether the keys will be used in a key agreement scheme, digital signature algorithm or to perform a key transport, then the PCT shall be performed consistent with the intended use of the keys (i.e., TE10.35.02 for signatures)

As such, we implement VE10.35.02 of ISO/IEC 24759:2017. A simple test that generates an ML-DSA keypair, signs a message, and verifies the signature.

static int ml_dsa_keypair_pct(ml_dsa_params *params,
                              uint8_t *pk,
                              uint8_t *sk) {
  uint8_t signature[MLDSA87_SIGNATURE_BYTES];
  ml_dsa_sign(params, signature, &params->bytes, NULL, 0, NULL, 0, sk);
  return ml_dsa_verify(params, signature, params->bytes, NULL, 0, NULL, 0, pk) == 0;
}

In which we:

  • assign the maximum signature size of bytes (params->bytes has the exact size but we can't dynamically allocate and we want to spent as little time in this function as possible -- allocate max KEM size here
  • sign NULL (quick and works)
  • return verify result

This is called within the internal key generation function if defined AWSLC_FIPS

#if defined(AWSLC_FIPS)
  // Abort in case of PCT failure.
  if (!ml_dsa_keypair_pct(params, pk, sk)) {
    BORINGSSL_FIPS_abort();
  }
#endif
  return 0;
}

"This effectively triples the runtime of key generation."
-Dusan Kostic #1969

Cryptographic Algorithm Self Test CAST)

Per the I.G guidance a CAST using a KAT is required for ML-DSA key generation

if the module implements ML-DSA key generation, the module shall have an ML-DSA key generation
CAST. The ML-DSA key generation takes no inputs and outputs a public key (pk) and private key
(sk). The CAST shall use the ML-DSA key generation algorithm (i.e., Algorithm 1 in FIPS 204),
and for a KAT, using a fixed/predetermined random seed (ξ) value, to compare the resulting outputs
to the pre-computed values of pk and sk.

This is implemented by the new addition to the self_check.c test suite with test boringssl_self_test_ml_dsa which performs the following:

  if (!ml_dsa_44_keypair_internal_no_self_test(public_key, private_key, kMLDSAKeyGenSeed) ||
      !check_test(kMLDSAKeyGenPublicKey, public_key, sizeof(public_key), "ML-DSA keyGen public") ||
      !check_test(kMLDSAKeyGenPrivateKey, private_key, sizeof(private_key), "ML-DSA keyGen private")) {
    fprintf(stderr, "ML-DSA-44 KeyGen failed.\n");
    goto err;
  }
  • Key generation using the new name for the core internal key generation from seed function (our wrapper to FIPS 204: Algorithm 6 ML-DSA.KeyGen_internal) ml_dsa_44_keypair_internal_no_self_test with provided seed kMLDSAKeyGenSeed which is from the NIST ACVP KAT Keygen Seed tgId = 1 tcId = 1
  • A check_test on the output public_key from that function, with public key matching that of the expected output at public key tgId = 1 tcId = 1
  • A check_test on the output private_key from that function, with private key matching that of the expected output at private key tgId = 1 tcId = 1

Per the I.G guidance a CAST using a KAT is required for ML-DSA signature generation

if the module implements digital signature generation, the module shall have an ML-DSA digital
signature generation CAST for either the “hedged” or “deterministic” algorithm (per FIPS 204
Section 5.2 and Algorithm 2) . ML-DSA signature generation algorithm takes as input a private key
(sk) and a message (M), and it outputs a signature (σ). The CAST shall use the ML-DSA signature
generation algorithm (i.e., Algorithm 2 in FIPS 204), and for a KAT, using fixed/predetermined sk
and M values, to compare the resulting outputs to the pre-computed value (σ). This requires the
randomization parameter rnd be fixed for the “hedged” algorithms (e.g., all zeros).

  if (!ml_dsa_44_sign_internal_no_self_test(private_key, signature, &sig_len, kMLDSASignPlaintext,
                                            mlen_int, NULL, 0, kMLDSASigGenSeed) ||
      !check_test(kMLDSASignSignature, signature, sizeof(signature), "ML-DSA SigGen signature")) {
    fprintf(stderr, "ML-DSA-44 sign failed.\n");
    goto err;
  }

  • Signature generation using the newly named implementation of FIPS 204: Algorithm 7 ML-DSA.Sign_internal ml_dsa_44_sign_internal_no_self_test. The I.G states
  // ISO/IEC 19790:2012 Section 7.10.3.3 15. page 84 Requires the randomization
  // parameter rnd be fixed for the “hedged” algorithms (e.g., all zeros).

We implement "hedged" in aws-lc, rather than deterministic signatures (hedged short for "hedging your bets" and using both an RNG and hashing input from the message).

  • The per message signing seed is kMLDSASigGenSeed is all zeros.
  • The plaintext is 32 bytes of all zeros.
  • The signature produced is checked against the precomputed signature kMLDSASignSignature for that message and private key. We are not able to use the same NIST ACVP vector for this, as NIST do not provide vectors for the same public private key pairs between keygen and sigGen. We could load an additional public/private key pair from a known NIST CAVP vector, then we would have a known signature, but would have to store 2 public/private key pairs in the file (which feels obtuse). FIPS 140-3 doesn't state that the KAT has to be from CAVP in the implementation guidance, but if this isn't critical to performance, I can happily add a seed/pub/sig vector from the NIST ACVP KATs.

Per the I.G guidance a CAST using a KAT is required for ML-DSA signature verification

• if the module implements digital signature verification, the module shall have an ML-DSA digital
signature verification CAST. The ML-DSA signature verification algorithm takes as input a public
key (pk), a message (M), and a signature (σ), and outputs a Boolean value (i.e., a value that is true if
the signature is valid with respect to the message and public key, and false if the signature is invalid).
The CAST shall use the ML-DSA signature verification algorithm (i.e., Algorithm 3 in FIPS 204),
and for a KAT, using a fixed/predetermined pk, M, and σ values, to verify the signature (i.e., returns
true).

  if (!ml_dsa_44_verify_internal_no_self_test(public_key, kMLDSASignSignature, sig_len, kMLDSASignPlaintext,
                                              mlen_int, NULL, 0)) {
    fprintf(stderr, "ML-DSA-44 verify failed.\n");
    goto err;
    }
  • verify is simple and just makes sure the provided signature kMLDSASignSignature successfully verifies the provided plaintext kMLDSASignPlaintext and public_key.

Test Configuration

For the above tests, only one ML-DSA parameter set variant needs to be tested:

Note26: The above CASTs shall be performed on at least one of the following parameter-sets for MLDSA that are implemented in the approved mode: ML-DSA-44, ML-DSA-65, ML-DSA-87.

Also note that only one variant of pre-hash (ExternalMu-ML-DSA) and pure mode needs to be tested:

Note 25: If both ML-DSA signature verification and pre-hash ML-DSA signature verification
(Section 5.4 and Algorithm 5 in FIPS 204) are implemented and they both share the same underlying
cryptographic implementations, a CAST on either one of them is sufficient to meet the CAST
requirements for both. Note33 of this IG would apply to ensure all the underlying approved
algorithms used within both algorithms (e.g., the hash or XOF function used to compute the message
digest) are covered by a CAST.

Call-outs:

For reviewers:

  • I noticed in the ML-KEM PCT we use the external encap/decap functions that generate randomness then call internals, it would be quicker to use the empty string as a seed and call the internal functions -- but is this compliant with the spec? ISO/IEC don't make the distinction.
  • I tested the CASTs fail gracefuly by flipping some bits in kMLDSA*, and also by putting in the seed from the IETF standard and verifying the key pair

Testing:

The ML-DSA self-tests have been added to --gtest_filter=SelfTests.

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
Copy link

codecov-commenter commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 91.04478% with 6 lines in your changes missing coverage. Please review.

Project coverage is 78.96%. Comparing base (e7bd073) to head (f5d26cc).

Files with missing lines Patch % Lines
crypto/fipsmodule/self_check/self_check.c 77.77% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2148      +/-   ##
==========================================
- Coverage   78.97%   78.96%   -0.02%     
==========================================
  Files         611      611              
  Lines      105748   105812      +64     
  Branches    14973    14975       +2     
==========================================
+ Hits        83511    83550      +39     
- Misses      21583    21608      +25     
  Partials      654      654              

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

@jakemas jakemas marked this pull request as ready for review January 29, 2025 22:05
@jakemas jakemas requested a review from a team as a code owner January 29, 2025 22:05
Copy link
Contributor

@andrewhop andrewhop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks like a CAST for ML-DSA which lazily runs once on first use and on demand calling the self test like we did for ML-KEM #1846. But I don't see it performing a PCT for every keypair it generates like #1969. I think we need both, just the PR description looks a little wrong.

@jakemas
Copy link
Contributor Author

jakemas commented Jan 30, 2025

Thanks Andrew! Yup clocked my mistake there! Thank you! I'll add PCT elements tonight.

@jakemas jakemas changed the title Pairwise Consistency Tests for ML-DSA CAST for ML-DSA Jan 30, 2025
@jakemas jakemas changed the title CAST for ML-DSA CAST and PCT for ML-DSA Jan 30, 2025
@WillChilds-Klein
Copy link
Contributor

What's the performance impact of this change for key generation?

@skmcgrail skmcgrail self-requested a review January 30, 2025 18:11
skmcgrail
skmcgrail previously approved these changes Jan 30, 2025
@jakemas
Copy link
Contributor Author

jakemas commented Jan 30, 2025

What's the performance impact of this change for key generation?

Sign is about 4x slower than keygen across ML-DSA-44/65/87, verify is approx. equivalent to keygen. Since these are performed sequentially, we get ~6x slow down on keygen with PCT.

nebeid
nebeid previously approved these changes Jan 30, 2025
@jakemas jakemas dismissed stale reviews from skmcgrail and nebeid via a03ddc3 February 4, 2025 17:54
@andrewhop andrewhop enabled auto-merge (squash) February 6, 2025 18:48
@nebeid nebeid disabled auto-merge February 6, 2025 18:50
@nebeid nebeid enabled auto-merge (squash) February 6, 2025 18:50
@nebeid nebeid merged commit 4e40ef6 into aws:main Feb 6, 2025
121 of 126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants