Add MLKEM768 Hybrid Groups to libssl #1849
Conversation
alexw91
force-pushed
the
mlkem-hybrid-tls
branch
from
bfe3544 to
45e24c9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1849 +/- ##
=======================================
Coverage 78.51% 78.52%
=======================================
Files 583 583
Lines 98809 98821 +12
Branches 14159 14160 +1
=======================================
+ Hits 77583 77598 +15
+ Misses 20598 20596 -2
+ Partials 628 627 -1 ☔ View full report in Codecov by Sentry. |
alexw91
force-pushed
the
mlkem-hybrid-tls
branch
from
45e24c9 to
313712a
Compare
alexw91
force-pushed
the
mlkem-hybrid-tls
branch
from
313712a to
91c3318
Compare
|
Hm, I'm getting 404'd on the description's linked draft RFC. |
| }, | ||
|
|
||
| { | ||
| SSL_GROUP_SECP256R1_MLKEM768, // group_id |
There was a problem hiding this comment.
where do we configure AWS-LC's group preference? is that done by the caller's configuration of SupportedGroups, or do we maintain an ordered list somewhere as with s2n security policies?
There was a problem hiding this comment.
If no supported group override preference list is configured for the SSL connection, the default supported group list is used here:
https://github.com/aws/aws-lc/blob/main/ssl/extensions.cc#L309-L320
There was a problem hiding this comment.
should we add the hybrid MLKEM groups to this list and prefer them over regular ECDH? that would enable PQ TLS by default for consumers who don't configure a preference list (i'm not sure how common this is among our libssl consumers).
Can you add this as an automated test to our CI to ensure we don't break in the future. Have you tested this with s2n-tls? |
If this was a small addition to an existing test, I'd be happy to, but creating a brand new AWS-LC to BoringSSL CI integration test seems out of scope for this PR. s2n-tls does not yet have support for either of these algorithms. |
Fixed: https://datatracker.ietf.org/doc/html/draft-kwiatkowski-tls-ecdhe-mlkem |
alexw91
force-pushed
the
mlkem-hybrid-tls
branch
from
d07d6fa to
890aca4
Compare
alexw91
force-pushed
the
mlkem-hybrid-tls
branch
from
890aca4 to
1fc854f
Compare
## What's Changed * More tweaks for Ruby integration by @samuel40791765 in #1852 * Implementation of EVP_PKEY_CTX_ctrl_str for various key types by @justsmth in #1850 * Add MLKEM768 Hybrid Groups to libssl by @alexw91 in #1849 * add support for PEM_write_bio_PrivateKey_traditional by @samuel40791765 in #1845 * Update s2n-bignum subtree by @torben-hansen in #1861 * Add asserts in testing to fix Coverity alert by @smittals2 in #1864 * Disable CRYPTO_is_AVX512IFMA_capable by @justsmth in #1858 **Full Changelog**: v1.35.0...v1.35.1 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.
Issues:
None
Description of changes:
Add support for
X25519MLKEM768andSecP256r1MLKEM768from https://datatracker.ietf.org/doc/html/draft-kwiatkowski-tls-ecdhe-mlkemCall-outs:
A bug in
BadHybridKeyShareAcceptTestandBadHybridKeyShareFinishTestwas fixed. The for loop to corrupt each half of the Hybrid key share did not correctly calculate the bounds index after the 1st iteration. This bug would only partially corrupt the buffer on later iterations, and not corrupt the buffer at all if the 2nd KeyShare was smaller than the first (which is true for X25519MLKEM768).Testing:
All existing unit tests for
X25519Kyber768Draft00andSecP256r1Kyber768Draft00were updated or copied, and equivalent tests added forX25519MLKEM768andSecP256r1MLKEM768.Interoperability with BoringSSL for
X25519MLKEM768has been confirmed to work with AWS-LC as both client and server. BoringSSL does not implementSecP256r1MLKEM768.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.