Conversation
# Conflicts: # src/cryptography/hazmat/primitives/asymmetric/types.py # src/rust/cryptography-key-parsing/src/pkcs8.rs # src/rust/cryptography-key-parsing/src/spki.rs
| return rust_openssl.mlkem.from_mlkem768_public_bytes(data) | ||
|
|
||
| @abc.abstractmethod | ||
| def encapsulate(self) -> tuple[bytes, bytes]: |
| // ML-KEM returns the seed rather than a PKey because AWS-LC's | ||
| // kem_priv_encode writes the expanded key (2,400 bytes) into PKCS#8, | ||
| // not the seed. |
There was a problem hiding this comment.
I've updated it to clarify it.
I wanted to say that we can't use OpenSSL PKey construct here, so we need a second element in our ParsedPrivateKey enum.
| } | ||
|
|
||
| pub fn serialize_private_key(key: &ParsedPrivateKey) -> crate::KeySerializationResult<Vec<u8>> { | ||
| let ParsedPrivateKey::Pkey(pkey) = key; |
There was a problem hiding this comment.
Can you refactor this out into its own PR for readability
| } | ||
|
|
||
| extern "C" { | ||
| fn EVP_PKEY_kem_new_raw_public_key( |
There was a problem hiding this comment.
I don't think you need to declare these, they should just be available in ffi::
There was a problem hiding this comment.
Correct, only EVP_PKEY_keygen_deterministic is needed because it is still in experimental/ in aws-lc.
IIUC, this is Algorithm 16 in FIPS 203.
Using this function allows us to use the seed form of the private key.
| format: crate::serialization::PrivateFormat, | ||
| encryption_algorithm: &pyo3::Bound<'p, pyo3::PyAny>, | ||
| ) -> CryptographyResult<pyo3::Bound<'p, pyo3::types::PyBytes>> { | ||
| if !encryption_algorithm.is_instance(&types::KEY_SERIALIZATION_ENCRYPTION.get(py)?)? { |
There was a problem hiding this comment.
Why did we end up needing to re-implement a ton of the utility function whereas that wasn't required for MLDSA?
There was a problem hiding this comment.
The problem here is that we can't represent the private key as an openssl PKey (AWS-LC's kem_priv_encode writes the expanded key into PKCS#8, not the seed), so we can't use pkey_private_bytes here.
There was a problem hiding this comment.
We have another solution by refactoring pkey_private_bytes to:
- accept the new enum ParsedPrivateKey as argument
// Replace this line with a match {}
let raw_bytes = pkey.raw_private_key()?;
return Ok(pyo3::types::PyBytes::new(py, &raw_bytes));- Revert
let parsed = cryptography_key_parsing::ParsedPrivateKey::Pkey(pkey.to_owned());- Gate before
if format == PrivateFormat::TraditionalOpenSSL {to ensure we're not reaching this for MLKem keys
The main downside is that the refactor modifies every caller of the function from &slf.borrow().pkey, to ParsedPrivateKey::Pkey(slf.borrow().pkey.to_owned())
That would avoid the deduplication here (and the need for extensive tests for each variant of the encryption part)
There was a problem hiding this comment.
If the aws-lc change to store the seed on the pkey lands, we'll need neither of these changes, right?
There was a problem hiding this comment.
Correct! Once we can represent MLKEM's key as PKey, all this complexity goes away
# Conflicts: # src/rust/cryptography-key-parsing/src/pkcs8.rs
| pub enum ParsedPrivateKey { | ||
| Pkey(openssl::pkey::PKey<openssl::pkey::Private>), | ||
| #[cfg(CRYPTOGRAPHY_IS_AWSLC)] | ||
| MlKem(cryptography_openssl::mlkem::MlKemVariant, [u8; 64]), |
There was a problem hiding this comment.
Flag: this has changed from MlKem768 to MlKem to not have an overcomplex enum when we'll add the other variants.
| || (encryption_algorithm.is_instance(&types::ENCRYPTION_BUILDER.get(py)?)? | ||
| && encryption_algorithm | ||
| .getattr(pyo3::intern!(py, "_format"))? | ||
| .extract::<crate::serialization::PrivateFormat>()? | ||
| == format) | ||
| { |
There was a problem hiding this comment.
looks like this branch currently isn't covered
2 participants
Add the support for ML-KEM using AWS-LC as the backend.
The PR is massive (sorry), about 1000 lines, but I could not reduce it more.
It follows closely the pattern of MLDSA, with the latest changes (class cases, variant enum)
A following PR with the documentation will be opened once this one get merged.
(Note: until aws/aws-lc#3140 is fixed, we have to store the ML-KEM seed on our side)