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

ML-DSA private keys from seeds #2157

Merged
merged 13 commits into from
Feb 5, 2025
Merged

Conversation

jakemas
Copy link
Contributor

@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
crypto/pqdsa/pqdsa.c Outdated Show resolved Hide resolved
@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.

crypto/pqdsa/pqdsa.c Outdated Show resolved Hide resolved
crypto/evp_extra/p_pqdsa.c Outdated Show resolved Hide resolved
crypto/evp_extra/p_pqdsa_asn1.c Outdated Show resolved Hide resolved
@justsmth justsmth self-requested a review February 5, 2025 17:02
justsmth
justsmth previously approved these changes Feb 5, 2025
crypto/evp_extra/p_pqdsa_asn1.c Outdated Show resolved Hide resolved
crypto/evp_extra/p_pqdsa_asn1.c Outdated Show resolved Hide resolved
@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
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