Refresh default TLS context on secret update - #13342
Merged
Merged
Conversation
moonchen
requested changes
Jul 2, 2026
moonchen
left a comment
moonchen
left a comment
Contributor
There was a problem hiding this comment.
👍 to covering the default/no-SNI context on secret updates. A couple of issues with the new refresh loop, plus some smalls.
|
|
||
| QUICCertConfig::scoped_config server_cert; | ||
| SSL *ssl = SSL_new(server_cert->defaultContext()); | ||
| auto default_ctx = server_cert->defaultContext(); |
Contributor
There was a problem hiding this comment.
Not for this PR, but worth a note in the description or a follow-up issue: QUICCertConfig builds its own SSLCertLookup that only a full reload rebuilds, so secret updates still leave QUIC serving the stale default cert.
Contributor
Author
There was a problem hiding this comment.
Agreed. This change updates the SSLConfig lookup used by TCP TLS; QUICCertConfig maintains a separate lookup and still requires a full reload. I’m treating QUIC secret-update support as follow-up work rather than expanding this fix into that separate lifecycle.
bneradt
force-pushed
the
fix-cert-secret-default-refresh
branch
from
July 13, 2026 15:30
bac9ce3 to
aee87a7
Compare
bneradt
force-pushed
the
fix-cert-secret-default-refresh
branch
from
July 13, 2026 15:56
aee87a7 to
7dc7d88
Compare
bneradt
force-pushed
the
fix-cert-secret-default-refresh
branch
from
July 13, 2026 16:26
7dc7d88 to
1b58230
Compare
bneradt
force-pushed
the
fix-cert-secret-default-refresh
branch
from
July 13, 2026 17:33
1b58230 to
f9c7d12
Compare
Default server certificate secret updates could rebuild the TLS contexts for CN/SAN lookups while leaving the default/no-SNI context pointing at the old SSL_CTX. Operators could update cert material on disk and through the secret API, but new handshakes without a more specific match could still serve the stale certificate. This updates runtime context refresh to cover address/default lookup entries owned by the same ssl_multicert policy and retains the default context while callers create new TLS sessions. This also adds an AuTest that updates a plugin-loaded default certificate and verifies the next no-SNI handshake sees the new certificate. Fixes: apache#9562
bneradt
force-pushed
the
fix-cert-secret-default-refresh
branch
from
July 13, 2026 17:51
f9c7d12 to
319e655
Compare
moonchen
approved these changes
Jul 13, 2026
moonchen
left a comment
moonchen
left a comment
Contributor
There was a problem hiding this comment.
Verified that previous findings are addressed. CI is green.
Contributor
|
Cherry-picked to 10.2.x |
cmcfarlen
pushed a commit
that referenced
this pull request
Jul 29, 2026
Default server certificate secret updates could rebuild the TLS contexts for CN/SAN lookups while leaving the default/no-SNI context pointing at the old SSL_CTX. Operators could update cert material on disk and through the secret API, but new handshakes without a more specific match could still serve the stale certificate. This updates runtime context refresh to cover address/default lookup entries owned by the same ssl_multicert policy and retains the default context while callers create new TLS sessions. This also adds an AuTest that updates a plugin-loaded default certificate and verifies the next no-SNI handshake sees the new certificate. Fixes: #9562 (cherry picked from commit 01ed487)
cmcfarlen
pushed a commit
to cmcfarlen/trafficserver
that referenced
this pull request
Jul 29, 2026
Default server certificate secret updates could rebuild the TLS contexts for CN/SAN lookups while leaving the default/no-SNI context pointing at the old SSL_CTX. Operators could update cert material on disk and through the secret API, but new handshakes without a more specific match could still serve the stale certificate. This updates runtime context refresh to cover address/default lookup entries owned by the same ssl_multicert policy and retains the default context while callers create new TLS sessions. This also adds an AuTest that updates a plugin-loaded default certificate and verifies the next no-SNI handshake sees the new certificate. Fixes: apache#9562
cmcfarlen
added a commit
to cmcfarlen/trafficserver
that referenced
this pull request
Jul 29, 2026
http2_max_active_streams and tls_secret_update_default used the ssl_multicert.yaml Disk API, which the 10.2.x autest harness does not provide -- it registers ssl_multicert.config -- so both tests threw AttributeError at load time. Convert them to the flat ssl_multicert.config form already used by other tests on the branch. For tls_secret_update_default this also updates the reload touch target and the diags.log wait condition, which looked for "ssl_multicert.yaml finished loading"; on 10.2.x the SSL loader logs the actual filename (ssl_multicert.config), so the reload wait would otherwise never become ready. These tests were backported (via apache#13386 and apache#13342) without adapting their config-file format to the release branch.
cmcfarlen
added a commit
that referenced
this pull request
Jul 29, 2026
http2_max_active_streams and tls_secret_update_default used the ssl_multicert.yaml Disk API, which the 10.2.x autest harness does not provide -- it registers ssl_multicert.config -- so both tests threw AttributeError at load time. Convert them to the flat ssl_multicert.config form already used by other tests on the branch. For tls_secret_update_default this also updates the reload touch target and the diags.log wait condition, which looked for "ssl_multicert.yaml finished loading"; on 10.2.x the SSL loader logs the actual filename (ssl_multicert.config), so the reload wait would otherwise never become ready. These tests were backported (via #13386 and #13342) without adapting their config-file format to the release branch.
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
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.
Default server certificate secret updates could rebuild the TLS
contexts for CN/SAN lookups while leaving the default/no-SNI context
pointing at the old SSL_CTX. Operators could update cert material on
disk and through the secret API, but new handshakes without a more
specific match could still serve the stale certificate.
This updates runtime context refresh to cover address/default lookup
entries owned by the same ssl_multicert policy and retains the
default context while callers create new TLS sessions. This also adds
an AuTest that updates a plugin-loaded default certificate and
verifies the next no-SNI handshake sees the new certificate.
Fixes: #9562