feat: TLS1.2 support for RSA-PSS certificates #4927
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lrstewart
force-pushed
the
rsapss
branch
5 times, most recently
from
7a70f20 to
7906e1c
Compare
lrstewart
force-pushed
the
rsapss
branch
2 times, most recently
from
cad840e to
4419928
Compare
goatgoose
reviewed
Dec 4, 2024
jouho
reviewed
Dec 4, 2024
goatgoose
approved these changes
Dec 5, 2024
jouho
approved these changes
Dec 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release Summary:
Added TLS1.2 support for RSA-PSS certificates. Previously, RSA-PSS certificates could only be used with TLS1.3.
Resolved issues:
resolves #4804
Description of changes:
Supporting RSA-PSS certificates with TLS1.2 turned out to be as simple as just removing the TLS1.3-only restrictions. Previously, we specifically restricted RSA-PSS-PSS to TLS1.3.
Call-outs:
Testing:
I added a new end-to-end test to clearly assert that we support all certificates where we expect to.
Initially, I also updated the integration tests to start testing TLS1.2 + RSA-PSS-PSS. However, the cross_compatibility test fails because the mainline version of s2n-tls can't handle the new cases because it still forbids TLS1.2 + RSA-PSS-PSS. Skipping just TLS1.2 + RSA-PSS-PSS for the cross_compatibility test would be pretty verbose (I'd have to add a hacky "ignore this" check to every test in the cross_compatibility file) so I'm thinking it'd be simpler just to split the change into two steps. If folks disagree, I can recombine the changes. Here is a working run of the integ tests, with cross_compatibility removed.
Here's the integ test changes: b2efd5c
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.