Conversation
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def private_bytes( |
There was a problem hiding this comment.
FLAG: This method only returns the seed - never the expanded key.
| (params, pub_key_der) | ||
| } | ||
| #[cfg(CRYPTOGRAPHY_IS_AWSLC)] | ||
| id if id == openssl::pkey::Id::from_raw(cryptography_openssl::mldsa::NID_PQDSA) => { |
There was a problem hiding this comment.
FLAG: I did not find any better constant here, so I had to have a second check based on the key size.
| // We call ml_dsa_65_sign/verify directly instead of going through | ||
| // EVP_DigestSign/EVP_DigestVerify because the EVP PQDSA path hardcodes | ||
| // context to (NULL, 0), so we'd lose context string support. | ||
| fn ml_dsa_65_sign( |
There was a problem hiding this comment.
FLAG: I think it's here, but now that I'm re-reading the comment above, I think I may have not understand it.
I will double-check this.
| // Note: private_key_to_pkcs8() (i2d_PKCS8PrivateKey_bio) must be used | ||
| // instead of private_key_to_der() (i2d_PrivateKey), because AWS-LC's | ||
| // i2d_PrivateKey doesn't support PQDSA keys. | ||
| let pkcs8_der = self.pkey.private_key_to_pkcs8()?; |
There was a problem hiding this comment.
FLAG: This is not elegant here - but moving this to another layer (like the cryptography-openssl crate would induce a new dependency on asn1)
|
Can you take a look at the missing coverage before I review? (If there's questions about idioms for covering certain things, let me know) |
DarkaMaul
force-pushed
the
dm/mldsa65-aws-lc
branch
from
49220b3 to
4f4bc0b
Compare
DarkaMaul
force-pushed
the
dm/mldsa65-aws-lc
branch
from
4f4bc0b to
65ab941
Compare
| let pkcs8_der = self.pkey.private_key_to_pkcs8()?; | ||
| let pki = | ||
| asn1::parse_single::<cryptography_key_parsing::pkcs8::PrivateKeyInfo<'_>>(&pkcs8_der) | ||
| .unwrap(); |
There was a problem hiding this comment.
FLAG: I used unwrap here because I think we can trust the output of the parse_single here (and it was really challenging to be able to have coverage for the map_err branch)
2 participants
This PR adds ML-DSA-65 support when using the AWS-LC backend.
I know this PR is quite massive (+1000 lines), but I've already excluded the documentation from it. I can split it even further by:
mldsa_supported() -> bool = Falseso the test suite continue to pass (would be about 500 lines)I could also try to remove the x509/pkcs8 support, but in my tests, that led to a weird API state where some functions would not work.
I'm also happy to also split it in any other potential better ways if you see any.