Skip to content
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

Add initial support for MLKEM768 (without any new Security Policies) #4816

Merged
merged 6 commits into from
Oct 11, 2024

Conversation

alexw91
Copy link
Contributor

@alexw91 alexw91 commented Oct 1, 2024

Resolved issues:

None

Note: PR #4823 should be reviewed and merged before this PR.

Description of changes:

Part 3 in a multi-part series to add X25519MLKEM768 support to s2n.

This PR includes the following changes:

  • Add support for new send_kem_first boolean parameter for Hybrid PQ KeyShares (needed for X25519MLKEM768).
  • Add support for 1 new KEM: MLKEM768
  • Add support for 2 new Hybrid KemGroups: X25519MLKEM768 and SecP256r1MLKEM768
  • Add new self-talk unit tests to negotiate X25519MLKEM768 and confirm shared secret calculation is correct.

Call-outs:

No new s2n-tls security policies with X25519MLKEM768 have been added (other than test_all Policy). These new MLKEM-768 security policies will be coming in a later PR.

Previous PR's must be merged first:

Testing:

  • s2n self-talk unit tests with X25519MLKEM768
  • Unit tests confirm that s2n-tls hybrid shared secret calculation matches 2nd independent Python implementation.

Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@alexw91 alexw91 changed the title Add initial support for MLKEM768 (without any new Security Policies) [Part 2] Add initial support for MLKEM768 (without any new Security Policies) Oct 1, 2024
@alexw91 alexw91 force-pushed the mlkem-part-2 branch 2 times, most recently from 316df93 to 19fcae8 Compare October 1, 2024 23:51
@alexw91 alexw91 changed the title [Part 2] Add initial support for MLKEM768 (without any new Security Policies) Add initial support for MLKEM768 (without any new Security Policies) Oct 2, 2024
@alexw91 alexw91 force-pushed the mlkem-part-2 branch 3 times, most recently from a4cf925 to d07f333 Compare October 2, 2024 23:42
tls/s2n_kem.h Show resolved Hide resolved
tls/s2n_kem_preferences.c Outdated Show resolved Hide resolved
tls/s2n_kem.h Show resolved Hide resolved
crypto/s2n_pq.c Outdated Show resolved Hide resolved
tls/s2n_kem.h Outdated Show resolved Hide resolved
tls/extensions/s2n_client_key_share.c Outdated Show resolved Hide resolved
tests/unit/s2n_kem_preferences_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_pq_kem_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_security_policies_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_server_key_share_extension_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_mlkem_test.c Outdated Show resolved Hide resolved
crypto/s2n_libcrypto.h Outdated Show resolved Hide resolved
tls/extensions/s2n_server_key_share.c Show resolved Hide resolved
tls/extensions/s2n_server_key_share.c Show resolved Hide resolved
tls/extensions/s2n_server_key_share.c Outdated Show resolved Hide resolved
tests/unit/s2n_kem_preferences_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_tls13_pq_handshake_test.c Show resolved Hide resolved
@alexw91 alexw91 force-pushed the mlkem-part-2 branch 3 times, most recently from 8e227ab to 128df67 Compare October 8, 2024 16:22
@alexw91 alexw91 requested review from goatgoose and lrstewart October 8, 2024 21:10
tls/s2n_kem.h Outdated Show resolved Hide resolved
Comment on lines 16 to 20
#include "crypto/s2n_libcrypto.h"
#include "s2n_test.h"
#include "testlib/s2n_testlib.h"

bool s2n_libcrypto_supports_flag_no_check_time();
uint64_t s2n_libcrypto_awslc_api_version(void);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this change related to the rest of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not, this change is a cleanup that probably should have been included in the previous PR. This is the only other use of s2n_libcrypto_awslc_api_version() in the s2n unit tests. Now that this API was added to s2n_libcrypto.h I updated this test to include the header instead of declaring the API itself.

tls/extensions/s2n_client_key_share.c Outdated Show resolved Hide resolved
tls/extensions/s2n_client_key_share.c Show resolved Hide resolved
@alexw91 alexw91 removed the request for review from camshaft October 10, 2024 22:09
@goatgoose goatgoose enabled auto-merge (squash) October 11, 2024 17:50
@goatgoose goatgoose merged commit 5a2fdf6 into aws:main Oct 11, 2024
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants