ML-KEM: Minimal Support for Expanded Keys

[FAlbertDev]

Why?

Until now, we only supported ML-KEM private keys encoded as seeds (see #3893 (comment)). We hoped that there would be a consensus in crypto libraries to only use seeds. Sadly, this consensus was not reached (e.g., see Request to Extend IETF WGLC for ML-KEM and ML-DSA private key format Specifications).

We already have some internal requests for exporting ML-KEM keys in an expanded format for use on external hardware devices.

How

This PR adds some minimal (and hopefully not too much committing) support for expanded ML-KEM private keys. It does not break the API of previous versions. The logic is the following:

Depending on the byte length, the ML-KEM private key constructor recognizes if a seed or an expanded key was passed. Keys imported as expanded are serialized as expanded, while keys imported as seeds are serialized as seeds. Additionally, keys can be explicitly serialized as expanded and/or seeds using the (unstable API) methods: _expanded_private_key_bits() and _seed_private_key_bits(). This requires a dynamic cast, but it is better than nothing.

[coveralls]

coverage: 91.311% (-0.003%) from 91.314%
when pulling e5182ce on Rohde-Schwarz:expanded-ml-kem
into 3bdfe65 on randombit:master.

[reneme]

This PR adds some minimal (and hopefully not too much committing) support for expanded ML-KEM private keys.

I think it is worth adding that this explicitly omitting to adopt the PKCS #8 format discussed in the email thread also linked above. It didn't seem like consensus was reached there just yet. But the extension in this pull request should not hinder the adoption.

[reneme]

I think we should just get rid of the Kyber_Keypair_Codec base class and its children altogether.

Also: I'm somewhat on the fence whether the private key getters shouldn't actually be public and supported. I.e. remove the underscore prefix from them. After all this is the way how users can explicitly access the raw expansion bytes or the seed.

The story for PKCS#8 is more complicated, especially when we decide to implement the container variants proposed: i.e. "seed-only", "expansion-only", or "both". Perhaps #4694 might come in handy for this, to provide a high level API like so:

auto pkcs8 = mlkem_decaps_key.serialize()
                             .with_encoding_options(WithSeed | WithExpansion)
                             .with_cipher("AES-128/GCM")
                           //. ...
                             .as_pem();

[Copilot]

Copilot reviewed 11 out of 13 changed files in this pull request and generated no comments.

Files not reviewed (2)

• doc/api_ref/pubkey.rst: Language not supported
• src/tests/data/pubkey/kyber_encodings.vec: Language not supported

Comments suppressed due to low confidence (2)

src/tests/test_kyber.cpp:257

• Ensure that the public key object 'pkr' is used to retrieve the public key bits, as this change correctly distinguishes between sk and pk encoding. Verify that pkr is properly initialized and represents the intended public key.

result.test_eq("pk's encoding of pk", pkr->public_key_bits(), pk_raw);

src/lib/pubkey/kyber/kyber_common/kyber_constants.h:113

• [nitpick] Consider changing 'an private key' to 'a private key' to fix the grammatical error in the comment.

/// byte length of an private key with expanded encoding as defined in FIPS 203

[randombit]

Definitely not happy that this is required, but that's not something we can do about in the immediate sense. One suggestion of an alternate approach for the public API which might have some cross-algorithm utility.