Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| SSL_CTX_set_cert_cb( | ||
| client_ctx_.get(), | ||
| [](SSL *ssl, void *arg) -> int { | ||
| const uint16_t *sigalgs; |
Contributor
There was a problem hiding this comment.
warning: variable 'sigalgs' is not initialized [cppcoreguidelines-init-variables]
ssl/ssl_ciphers_test.cc:1530:
- ;
+ = nullptr;
Contributor
|
🔒 Security Review — View Report Please review before merging. |
SSL_set_SSL_CTX() only swaps the certificate configuration. Since 1.47.0 an SSL keeps the cipher lists that SSL_new() copied from the initial SSL_CTX (the peer-verification sigalgs have always worked this way), so a server that switches to a stricter SSL_CTX per SNI, as is common with OpenSSL, silently keeps the original policy. This was not documented. Document what SSL_set_SSL_CTX() does and does not change, how and when to apply per-connection cipher and verify-sigalg policy from the handshake callbacks, and how earlier releases differ. Also deprecate SSL_set_SSL_CTX() in the docs in favor of configuring the connection directly. It is not marked OPENSSL_DEPRECATED because nginx builds with -Werror and calls it. No behavior change. Add SSLContextSwitchTest, covering TLS 1.2 and 1.3 with the servername and select-certificate callbacks: a switch keeps the cipher and verify-sigalg policy, and per-connection setters change negotiation, including rejecting a client that offers only excluded suites. Related to aws#3614.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3633 +/- ##
==========================================
+ Coverage 78.46% 78.50% +0.03%
==========================================
Files 705 705
Lines 130645 130767 +122
Branches 17912 17916 +4
==========================================
+ Hits 102516 102654 +138
+ Misses 27150 27137 -13
+ Partials 979 976 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
nhatnghiho
reviewed
Oct 8, 2026
Comment on lines
3360
to
+3398
| // SSL_set_SSL_CTX changes |ssl|'s |SSL_CTX|. |ssl| will use the | ||
| // certificate-related settings from |ctx|, and |SSL_get_SSL_CTX| will report | ||
| // |ctx|. This function may be used during the callbacks registered by | ||
| // |SSL_CTX_set_select_certificate_cb|, | ||
| // |SSL_CTX_set_tlsext_servername_callback|, and |SSL_CTX_set_cert_cb| or when | ||
| // the handshake is paused from them. It is typically used to switch | ||
| // certificates based on SNI. | ||
| // | ||
| // This function is deprecated because it changes only part of |ssl|'s | ||
| // configuration. New code should instead configure |ssl| directly from those | ||
| // callbacks, setting the certificate with |SSL_set_chain_and_key| or | ||
| // |SSL_use_cert_and_key|. Existing callers remain supported. | ||
| // | ||
| // It replaces the certificate-related settings with |ctx|'s: the certificates, | ||
| // private keys and private key method, signing algorithm preferences, OCSP | ||
| // response, SCT list, delegated credential, certificate callback, session ID | ||
| // context, and the store set by |SSL_CTX_set1_verify_cert_store|. Any of these | ||
| // configured on |ssl| are discarded. |ssl| also takes |ctx|'s early data | ||
| // setting. Settings read from the current |SSL_CTX| when used, such as the ALPN | ||
| // selection callback and the certificate store (|SSL_CTX_set_cert_store|), | ||
| // come from |ctx| afterwards. | ||
| // | ||
| // It does not change settings that |SSL_new| copied from the initial |SSL_CTX|, | ||
| // such as the cipher suites, peer-verification signature algorithm preferences, | ||
| // supported groups, protocol versions, options, verify mode, and certificate | ||
| // verification parameters. To use |ctx|'s values, set them on |ssl| after this | ||
| // call, for example with |SSL_set_cipher_list|, |SSL_set_ciphersuites|, and | ||
| // |SSL_set_verify_algorithm_prefs|. A server selects its cipher suite after the | ||
| // callbacks above return, but fixes its protocol version range when the | ||
| // |SSL_CTX_set_select_certificate_cb| callback returns, before the other two | ||
| // run. For differences from OpenSSL and earlier AWS-LC releases, see | ||
| // https://github.com/aws/aws-lc/blob/main/PORTING.md#switching-ssl_ctx-during-the-handshake | ||
| // | ||
| // Note the session cache and related settings will continue to use the initial | ||
| // |SSL_CTX|. Callers should use |SSL_CTX_set_session_id_context| to partition | ||
| // the session cache between different domains. | ||
| // | ||
| // TODO(davidben): Should other settings change after this call? | ||
| // TODO (CryptoAlg-2398): Add |OPENSSL_DEPRECATED|. nginx defines -Werror and | ||
| // depends on this. |
Contributor
There was a problem hiding this comment.
This blob is a bit confusing for me overall. Consider something like below?
Also, the link https://github.com/aws/aws-lc/blob/main/PORTING.md#switching-ssl_ctx-during-the-handshake doesn't lead to anywhere. PORTING.md doesn't have a switching-ssl_ctx-during-the-handshake section
Suggested change
| // depends on this. | |
| // SSL_set_SSL_CTX changes |ssl|'s |SSL_CTX|. |ssl| will use the | |
| // certificate-related settings from |ctx|, and |SSL_get_SSL_CTX| will report | |
| // |ctx|. This function may be used during the callbacks registered by | |
| // |SSL_CTX_set_select_certificate_cb|, | |
| // |SSL_CTX_set_tlsext_servername_callback|, and |SSL_CTX_set_cert_cb| or when | |
| // the handshake is paused from them. It is typically used to switch | |
| // certificates based on SNI. | |
| // | |
| // Deprecated: switching contexts only partly reconfigures |ssl|, which is easy | |
| // to get wrong. Prefer setting what you need on |ssl| from the same callback, | |
| // e.g. the certificate with |SSL_set_chain_and_key| or |SSL_use_cert_and_key|. | |
| // This function will keep working for existing code. | |
| // | |
| // |ssl| has its own copy of every setting that |SSL_new| copied from the | |
| // initial |SSL_CTX| (including cipher suites, peer-verification signature | |
| // algorithms, groups, and protocol versions) or that was set on |ssl| directly. | |
| // This function does not change those original copies, with the exception | |
| // of certificate-related settings as noted above (e.g., certificates, keys, | |
| // signing preferences, OCSP response, SCT list, and certificate callback). | |
| // Any settings that |ssl| looks up in |SSL_CTX| is now looked up from the | |
| // replaced |SSL_CTX|. | |
| // | |
| // To apply other |ctx| settings, set them on |ssl| after this call, e.g. with | |
| // |SSL_set_cipher_list|, |SSL_set_ciphersuites|, and | |
| // |SSL_set_verify_algorithm_prefs|. | |
| // | |
| // Note the session cache and related settings will continue to use the initial | |
| // |SSL_CTX|. Callers should use |SSL_CTX_set_session_id_context| to partition | |
| // the session cache between different domains. | |
| // | |
| // TODO (CryptoAlg-2398): Add |OPENSSL_DEPRECATED|. nginx defines -Werror and | |
| // depends on this. |
justsmth
force-pushed
the
ssl-set-ssl-ctx-docs
branch
from
October 8, 2026 18:52
a62006b to
473e786
Compare
This branch has not been deployed
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
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.
Related issues
Related to #3614, haproxy/haproxy#3500 and P514234649.
Context and motivation
SSL_set_SSL_CTX()only swaps certificate configuration; since 1.47.0 the connection keeps the cipher listsSSL_new()copied (verify sigalgs always worked this way). Servers that switch to a stricterSSL_CTXper SNI, as with OpenSSL, silently keep the original policy, and this wasn't documented.Description of changes
Documents what
SSL_set_SSL_CTX()does and doesn't change (including cipher suites), how and when to apply per-connection policy from handshake callbacks, and how older releases differ. Also deprecates it in the docs in favor of configuring the connection directly; it isn't markedOPENSSL_DEPRECATEDbecause nginx builds with-Werrorand calls it. No behavior change.Testing
New
SSLContextSwitchTest(TLS 1.2/1.3, servername and select-certificate callbacks) checks that a switch keeps the cipher and verify-sigalg policy and that per-connection setters change negotiation, including rejecting clients that offer only excluded suites.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.