Allow SSL_CTX to use the new multiple certificate slots internally
#1100
Conversation
This file contains hidden or 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
samuel40791765
force-pushed
the
multiple-certs-work
branch
from
0a0a0aa to
39df237
Compare
justsmth
reviewed
Jul 19, 2023
samuel40791765
force-pushed
the
multiple-certs-work
branch
from
39df237 to
664b706
Compare
samuel40791765
force-pushed
the
multiple-certs-work
branch
from
664b706 to
95d7137
Compare
samuel40791765
force-pushed
the
multiple-certs-work
branch
2 times, most recently
from
ad52aa9 to
e132ffd
Compare
samuel40791765
force-pushed
the
multiple-certs-work
branch
from
e132ffd to
3b029b9
Compare
skmcgrail
previously approved these changes
Jul 26, 2023
justsmth
reviewed
Jul 26, 2023
|
While working on Justin's PR comments, I noticed that |
samuel40791765
force-pushed
the
multiple-certs-work
branch
from
841c071 to
487dbcf
Compare
justsmth
approved these changes
Jul 27, 2023
skmcgrail
approved these changes
Jul 28, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Issues:
Addresses
CryptoAlg-1844Description of changes:
This builds upon the new framework in #1086 to use the new multiple certificate slots available. The SSL connection should use the corresponding index of the certificate and pkey type assigned. As of now, the connection will directly use the last certificate that was set for it's connection. This corresponds with our original behavior before we reintroduced the multiple certificate slots framework, where we only maintained one
x509_leafand onepkey perCERT.A subsequent PR will be introduced to allow the user to manually select which certificate slots they wish to use.
Call-outs:
These functions change the certificate slot index:
SSL_CTX_use_certificate_ASN1/SSL_use_certificate_ASN1/SSL_CTX_use_certificate/SSL_use_certificate(functions that callssl_set_certinternally)SSL_use_*_PrivateKey_*(functions that callssl_set_pkeyinternally)The naming for the list of functions are pretty straightforward. Users setting these functions would have expected
SSL_CTXto use the new set certificate/pkey, so it's natural that we switch over to that slot. Despite it seeming straightforward, definately let me know if I should add more documentation regarding this anywaysSSL_CTX_select_current_certis a bit more interesting since it selects an existing cert inSSL_CTXto use. This will be implemented in one of the next PRs.Testing:
I've added a new test that sets up a connection with different certificate types and checks if the correct slot is being used.
ED25519is incompatible with older versions of TLS, so those are skipped.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.