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

Remove support for BIKE, SIKE, and Kyber (Round 2) #3392

Merged
merged 9 commits into from
Jul 21, 2022

Conversation

alexw91
Copy link
Contributor

@alexw91 alexw91 commented Jul 13, 2022

Resolved issues:

N/A

Description of changes:

Now that the 3rd Round of the NIST PQ standardization process has closed, and NIST has selected Kyber for eventual standardization, we'd like to focus more of our efforts towards Kyber. As a result, this pull request removes support for the other PQ KEM algorithms that have been added to s2n throughout the NIST PQ standardization effort so far. Both BIKE and SIKE have progressed into Round 4 of the NIST standardization process, and either algorithm may still end up being selected for standardization at the end of Round 4. If that happens, we'll likely add support for them back to s2n in the future, but in the meantime, we'd like to reduce the number of PQ KEM algorithms that we are maintaining.

This PR removes support for the following NIST PQ KEM candidate algorithms in s2n:

  • SIKE Round {1, 2, 3}
  • BIKE Round {1, 2, 3}
  • Kyber Round 2. (Kyber Round 1 was never added to s2n.)

In general, this PR contains the following changes:

  • Deleted every PQ KEM candidate implementation in the pq-crypto directory except for Kyber Round 3.
  • Deleted any draft algorithm ID's for these PQ KEM candidates
  • If a TLS preference list used BIKE or SIKE, those algorithms were removed from the preference list, and the other algorithms left in place to allow for graceful fallback.
  • If a TLS preference list used Kyber Round 2, it was upgraded in place to use Kyber Round 3 to allow for a graceful upgrade. (All AWS services that launched Kyber Round 2 support, now also support Kyber Round 3).
  • Code and tests that depended on any removed PQ algorithm implementation details were deleted (Eg, draft PQ KEM algorithm ID's, custom BIKE/SIKE assembly flags, SAW tests, KAT tests, fuzz tests, etc)
  • Other tests that performed "generic" hybrid PQ TLS negotiations were upgraded to use Kyber Round 3 where possible.
  • Some tests that checked PQ ClientHelloRetry negotiation logic with PQ were disabled when compiled with older Openssl versions since a minimum of 2 PQ KEMs are needed to check fallback logic. (Both p256_Kyber and x25519_Kyber are required, but older Openssl versions do not have support for x25519.)
  • The OQS Openssl integration test was upgraded to use Kyber Round 3 from Kyber Round 2.
  • Fixed a bug in s2nc client debugging output that printed which KEM Group was attempted to be negotiated, rather than the one that was actually negotiated.

Call-outs:

I haven't run every test permutation locally.

Some of the older s2n TLS security policies that have "PQ" in their name have had all of their PQ KEM algorithms removed. After this change, those policies will gracefully downgrade to non-PQ algorithms during the TLS handshake rather than break anyone still using them. To avoid surprising or impacting customers with this behavior change, these older PQ security policies have been preemptively deprecated and removed from the AWS SDK's:

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

  • Unit Tests pass locally on my Ubuntu desktop against Openssl 1.1.1, Openssl 1.0.2, Openssl 1.0.2-FIPS, and AWS-LC.
  • Fuzz Tests pass locally against Openssl 1.1.1.
  • Both Make and CMake builds pass locally

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 marked this pull request as ready for review July 14, 2022 19:51
@alexw91 alexw91 requested a review from dougch as a code owner July 14, 2022 19:51
@alexw91 alexw91 changed the title [DRAFT] Remove support for BIKE, SIKE, and Kyber (Round 2) Remove support for BIKE, SIKE, and Kyber (Round 2) Jul 14, 2022
@alexw91 alexw91 requested a review from dkostic July 15, 2022 23:27
Copy link
Contributor

@dougch dougch left a comment

Choose a reason for hiding this comment

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

Just an observation, do we need more kyber tests (not as part of this, but in general) ?

Copy link
Contributor

@dkostic dkostic left a comment

Choose a reason for hiding this comment

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

lgtm. Nit: all but one test in tests/integration/s2n_pq_handshake_test.py are testing classical algorithms. Not sure it makes sense to keep those tests, but I won't complain about more tests :)

@dougch dougch merged commit df28694 into aws:main Jul 21, 2022
@alexw91 alexw91 mentioned this pull request Aug 4, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants