Skip to content

Conversation

@jakemas
Copy link
Collaborator

@jakemas jakemas commented Feb 3, 2025

Issues:

Resolves #CryptoAlg-2889

Description of changes:

The industry has decided upon a few conventions to assist with the usability of ML-DSA key sizes, one of which is to decrease the amount of bytes transferred when encoding ML-DSA private keys. As ML-DSA keys can be deterministically generated from a provided 32-byte seed, many standards bodies have selected to use private keys in seed format. For example, the IETF draft that standardizes the use of ML-DSA in X.509. https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/

This provides:

The functionality that provides key generation from seeds for ML-DSA was added in my previous PR #1999. This PR exposes the internal key generation functions within the PQDSA struct, so that a new function named PQDSA_KEY_set_raw_keypair_from_seed can directly call the method.

The function PQDSA_KEY_set_raw_keypair_from_seed simply performs:

  • a check that the provided seed is of the correct length
  • allocation of public/private key buffers
  • key generation from seed using pqdsa_keygen_internal

As such, pqdsa_priv_decode has been modified to call:

  • PQDSA_KEY_set_raw_private_key when the provided length of the key is pqdsa->private_key_len
  • PQDSA_KEY_set_raw_keypair_from_seed when the provided length of the key is pqdsa->keygen_seed_len

We implement this at the EVP layer by modifying EVP_PKEY_pqdsa_new_raw_private_key to:

  • call PQDSA_KEY_set_raw_private_key when the len provided is private_key_len
  • call PQDSA_KEY_set_raw_keypair_from_seed when the len provided is keygen_seed_len

Call-outs:

  • The API EVP_PKEY_pqdsa_new_raw_private_key will now populate both public and private keys when provided a seed.
  • The API EVP_PKEY_pqdsa_new_raw_public_key only sets the public key
  • The API PQDSA_KEY_set_raw_private_key will only set private key
  • The API PQDSA_KEY_set_raw_public_key will only set public key
  • The API EVP_parse_public_key sets the public key from DER encoded of public key
  • The API EVP_parse_private_key can accept DER encodings of either the full private key representation, or the 32-byte seed.
  • The API EVP_marshal_public_key sets the DER encoded public key from a EVP PKEY
  • The API EVP_marshal_private_key sets the DER encoded full private key representation (it cannot marshal the seed, as this cannot be derived from the full private key representation.

Testing:

To provide KAT for keys from seeds I have included the Appendix C. Examples from https://datatracker.ietf.org/doc/draft-ietf-lamps-dilithium-certificates/. For each provided ML-DSA parameter set, we generate the example private key provided from seed, and check that the corresponding public key matches that in the specification. These tests are in PQDSAParameterTest namely, ParsePrivateKey.

These examples show how much shorter ML-DSA-87 private keys are in seed format:

 -----BEGIN PRIVATE KEY-----
   MDICAQAwCwYJYIZIAWUDBAMTBCAAAQIDBAUGBwgJCgsMDQ4PEBESExQVFhcYGRob
   HB0eHw==
   -----END PRIVATE KEY-----

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.

@jakemas jakemas requested a review from a team as a code owner February 3, 2025 20:07
justsmth
justsmth previously approved these changes Feb 3, 2025
@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2025

Codecov Report

Attention: Patch coverage is 62.12121% with 25 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@bd19ef4). Learn more about missing BASE report.

Files with missing lines Patch % Lines
crypto/pqdsa/pqdsa.c 56.00% 11 Missing ⚠️
crypto/evp_extra/p_pqdsa_test.cc 66.66% 6 Missing and 2 partials ⚠️
crypto/evp_extra/p_pqdsa.c 50.00% 4 Missing ⚠️
crypto/evp_extra/p_pqdsa_asn1.c 77.77% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2157   +/-   ##
=======================================
  Coverage        ?   78.97%           
=======================================
  Files           ?      611           
  Lines           ?   105682           
  Branches        ?    14960           
=======================================
  Hits            ?    83463           
  Misses          ?    21566           
  Partials        ?      653           

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

@justsmth justsmth self-requested a review February 5, 2025 17:02
justsmth
justsmth previously approved these changes Feb 5, 2025
@jakemas jakemas dismissed stale reviews from WillChilds-Klein and justsmth via 1809d69 February 5, 2025 17:35
@justsmth justsmth enabled auto-merge (squash) February 5, 2025 19:40
@justsmth justsmth merged commit 54d5051 into aws:main Feb 5, 2025
122 of 126 checks passed
WillChilds-Klein added a commit to corretto/amazon-corretto-crypto-provider that referenced this pull request Feb 13, 2025
AWS-LC [PR #2157](aws/aws-lc#2157) newly
supports parsing ML-DSA private keys from seeds, as BouncyCastle does,
so we update our relevant interop test.
julian89j added a commit to julian89j/amazon-corretto-crypto-provider that referenced this pull request Sep 7, 2025
AWS-LC [PR #2157](aws/aws-lc#2157) newly
supports parsing ML-DSA private keys from seeds, as BouncyCastle does,
so we update our relevant interop test.
CryptoBee56 added a commit to CryptoBee56/amazon-corretto-crypto-provider that referenced this pull request Sep 16, 2025
AWS-LC [PR #2157](aws/aws-lc#2157) newly
supports parsing ML-DSA private keys from seeds, as BouncyCastle does,
so we update our relevant interop test.
ElonWills added a commit to ElonWills/amazon-corretto-crypto-provider that referenced this pull request Sep 17, 2025
AWS-LC [PR #2157](aws/aws-lc#2157) newly
supports parsing ML-DSA private keys from seeds, as BouncyCastle does,
so we update our relevant interop test.
jakemas added a commit that referenced this pull request Oct 6, 2025
### Issues:
Resolves #N/A
Addresses #aws/aws-lc-rs#799

### Description of changes: 
This PR adds support for the parsing of ML-KEM private keys encoded in
the ASN.1 seed format, as described in
https://datatracker.ietf.org/doc/draft-ietf-lamps-kyber-certificates/.

As such, 64 byte ASN.1 encoded private keys, such as this example
ml-kem-512 private key from the IETF draft can now be parsed in aws-lc
using `EVP_parse_private_key`:
```
   -----BEGIN PRIVATE KEY-----
   MFQCAQAwCwYJYIZIAWUDBAQBBEKAQAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZ
   GhscHR4fICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8=
   -----END PRIVATE KEY-----
```
The `EVP_PKEY` produced will have **both** the full private and public
ML-KEM key expanded material within.

This very much follows a similar PR for ML-DSA
#2157.

While there are a few ways to tackle the issue of getting the public key
when the private is provided, for example making use of the
`OneAsymmetricKey` structure as in
https://datatracker.ietf.org/doc/html/draft-ietf-lamps-kyber-certificates-11#section-6.
However as PKCS8 v2 isn't well supported, I feel encoding
`OneAsymmetricKey` won't be easy for those wanting to bring key material
into aws-lc/-rs. Python Cryptography doesn't support it today, for
example. OpenSSL encodes keys by default using the `BOTH` approach (seed
and expanded material), so importing from OpenSSL produced private seeds
will now be possible, and populate both pub and priv keys.

#### Background

The IETF draft standard defines three private key formats for ML-KEM
certificates:

1. __seed [0] OCTET STRING__ - 64-byte deterministic seed (added in this
PR)
2. __expandedKey OCTET STRING__ - Full private key material
(#2624)
3. __both SEQUENCE {seed, expandedKey}__ - Both formats together (future
work)

Previously, AWS-LC only supported the expandedKey format. This PR adds
support for the more compact seed format, which uses deterministic key
generation from a 64-byte seed.

How does this help aws/aws-lc-rs#799? 

When the private key is provided as a seed, both the expanded private
and secret key are populated within the EVP_PKEY structure, allowing,
for the first time, a fully populated key on import.

### Call-outs:
__Core Implementation:__

- __`crypto/evp_extra/p_kem_asn1.c`__: Enhanced `kem_priv_decode()` to
detect and parse seed format using ASN.1 context-specific tag `[0]`
(stubbed out BOTH format)
- __`crypto/fipsmodule/kem/kem.c`__: Added
`KEM_KEY_set_raw_keypair_from_seed()` function to generate keypairs from
seeds using deterministic ML-KEM functions
- __`crypto/fipsmodule/kem/internal.h`__: Added function declaration for
seed-based key generation

FIPS Questions:

Are we allowed to expose this functionality as part of the FIPS module?
Yes, see https://csrc.nist.gov/projects/post-quantum-cryptography/faqs
Question 1:
>Are cryptographic modules implementing FIPS 203 or FIPS 204 allowed to
use seeds as the default key format instead of expanded keys? For
example, in FIPS 203, can a cryptographic module store a 64-byte seed
(d, z) instead of the output (dk, ek) of Algorithm 19 (ML-KEM.KeyGen),
and compute (dk, ek) as needed via (dk, ek) <--
ML-KEM.KeyGen_internal(d, z)?
>
> **Response**:
For both FIPS 203 and FIPS 204, a KeyGen seed is considered an
acceptable alternative format for a key-pair, or for the private (i.e.,
decapsulation or signing) key. In particular, generating the seed in one
cryptographic module and then importing or exporting it into another
cryptographic module is allowed. The internal key generation functions
ML-KEM.KeyGen_Internal(d, z) and ML-DSA.KeyGen_internal(ξ) can be
accessed for this purpose.

### Testing:
- __`crypto/evp_extra/p_kem_test.cc`__: Added comprehensive
`ParsePrivateKeySeed` test using IETF standard test vectors
- Added test vectors for ML-KEM-512, ML-KEM-768, and ML-KEM-1024 seed
formats from Appendix C of the draft standard
- Tests verify that seed-generated keypairs match expected public keys
from the standard

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.

---------

Co-authored-by: Sean McGrail <[email protected]>
Co-authored-by: Justin W Smith <[email protected]>
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.

4 participants