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

Account for cipher auth with multiple cert slots #1956

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

samuel40791765
Copy link
Contributor

@samuel40791765 samuel40791765 commented Oct 30, 2024

Issues:

Addresses CryptoAlg-2699

Description of changes:

Ruby has a dependency on the multiple certificate slot mechanisms that OpenSSL allows for. We've already done the work to support this, but another Ruby 3.1 test has exposed a gap in our support. We were only depending on the negotiated signature algorithms to retrieve the right certificate for the server to send back, but the cipher authentication scheme was also checked in OpenSSL as well. Ruby's tests happen to only depend on configuring the authentication scheme which brought this to light.
We happen to do only do this when checking private keys for TLS 1.0/1.1 , this also fixes it to check the cipher against the initial public key. This change also introduces all of the mentioned cipher authenticated check behavior for TLS 1.2.

Call-outs:

N/A

Testing:

New unit test that allows all possible sigalgs, with the cipher authentication suite being the only restriction.

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.

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 98.24561% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.88%. Comparing base (796202b) to head (19d003e).

Files with missing lines Patch % Lines
ssl/ssl_test.cc 96.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1956      +/-   ##
==========================================
+ Coverage   78.86%   78.88%   +0.01%     
==========================================
  Files         593      593              
  Lines      101933   101961      +28     
  Branches    14450    14454       +4     
==========================================
+ Hits        80386    80427      +41     
+ Misses      20903    20890      -13     
  Partials      644      644              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@samuel40791765 samuel40791765 force-pushed the multiple-certs-cipher branch 3 times, most recently from 35a1680 to 3f53a7a Compare November 1, 2024 20:21
@samuel40791765 samuel40791765 marked this pull request as ready for review November 1, 2024 20:25
@samuel40791765 samuel40791765 requested a review from a team as a code owner November 1, 2024 20:25
ssl/ssl_privkey.cc Outdated Show resolved Hide resolved
ssl/ssl_privkey.cc Outdated Show resolved Hide resolved
skmcgrail
skmcgrail previously approved these changes Nov 12, 2024
ssl/ssl_privkey.cc Outdated Show resolved Hide resolved
ssl/ssl_privkey.cc Outdated Show resolved Hide resolved
ssl/ssl_test.cc Outdated Show resolved Hide resolved
@samuel40791765 samuel40791765 merged commit 3aa32cc into aws:main Nov 13, 2024
114 of 116 checks passed
@samuel40791765 samuel40791765 deleted the multiple-certs-cipher branch November 13, 2024 20:58
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.

4 participants