-
Notifications
You must be signed in to change notification settings - Fork 3k
Ingela/ssl/public key/quantum security/otp 19552 #10004
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
base: maint
Are you sure you want to change the base?
Ingela/ssl/public key/quantum security/otp 19552 #10004
Conversation
CT Test Results 4 files 75 suites 28m 50s ⏱️ Results for commit 9311afc. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
e807c5b
to
81acc7c
Compare
48969f9
to
f64504a
Compare
f64504a
to
5b09406
Compare
5b09406
to
4c89af9
Compare
… test data function So the we can generate ML-DSA cert chains from pre-generated keys. ML-DSA private keys do no explicitly include the public keys.
4c89af9
to
9311afc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for ML-DSA signature schemes (mldsa44, mldsa65, mldsa87) and ML-KEM key-exchange groups (mlkem512, mlkem768, mlkem1024) across SSL/TLS libraries and tests.
- Introduces new test fixtures and groups for ML-DSA and ML-KEM in
ssl_test_lib
,ssl_cert_tests
, and multiple suite modules. - Extends core TLS logic (
tls_v1
,tls_server_connection_1_3
,tls_handshake_1_3
, etc.) to recognize and process the new algorithms and groups. - Updates runtime dependencies and configuration (
ssl_app.src
,ssl_config
, etc.) and includes new public_key test data.
Reviewed Changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
lib/ssl/test/ssl_test_lib.erl | Adjusted OpenSSL client flags, updated skip messages, added ML-DSA signature helper |
lib/ssl/test/ssl_cert_SUITE.erl | Added mldsa and mlkem group tests and initialization logic |
lib/ssl/src/tls_v1.erl | Extended signature_schemes and default_signature_schemes for ML-DSA |
lib/ssl/src/tls_server_connection_1_3.erl | Added generate_server_share override for ML-KEM |
lib/ssl/src/ssl_app.src | Updated crypto dependency version placeholder |
Comments suppressed due to low confidence (2)
lib/ssl/test/ssl_test_lib.erl:409
- The skip message concatenates 'for' and the group name without a space; consider adding a space after 'for ' to improve readability (e.g., "Missing OpenSSL support for ").
{skip, "Missing OpenSSL support for" ++ atom_to_list(GroupName)}
lib/ssl/test/ssl_test_lib.erl:2414
- The
-verify 2
argument is removed from the OpenSSL client invocation; verify that equivalent certificate verification is covered by tests to catch any regressions.
["s_client",
case [] =/= crypto:supports(kems) of | ||
true -> | ||
Config; | ||
false -> | ||
{skip, "Missing support for mlkem in OpenSSL"} | ||
end; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The init_per_testcase/2
handling for mlkem_groups is duplicated across multiple test modules; consider extracting a common helper or macro to reduce repetition.
case [] =/= crypto:supports(kems) of | |
true -> | |
Config; | |
false -> | |
{skip, "Missing support for mlkem in OpenSSL"} | |
end; | |
handle_mlkem_groups(Config); |
Copilot uses AI. Check for mistakes.
@@ -308,6 +317,43 @@ init_per_group(GroupName, Config) -> | |||
do_init_per_group(GroupName, Config) | |||
end. | |||
|
|||
do_init_per_group(Group, Config) when Group == mldsa -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The do_init_per_group/2
logic for mldsa is repeated in multiple SSL test suites; consider refactoring into a shared helper function to avoid duplicated code.
do_init_per_group(Group, Config) when Group == mldsa -> | |
do_init_per_group(Group, Config) when Group == mldsa -> | |
do_init_mldsa(Group, Config); | |
do_init_per_group(Group, Config0) when Group == rsa; | |
do_init_mldsa(Group, Config) -> |
Copilot uses AI. Check for mistakes.
Add support for ML-DSA and ML-KEM