RajaMuhammadAwais commented on code in PR #13677:
URL: https://github.com/apache/trafficserver/pull/13677#discussion_r4017456184
##########
src/iocore/net/SSLUtils.cc:
##########
@@ -578,33 +578,21 @@ ssl_apply_sni_session_ticket_properties(SSL *ssl)
}
#endif
-static ssl_ticket_key_block *
-ssl_context_enable_tickets(SSL_CTX *ctx, const char *ticket_key_path)
+static bool
+ssl_context_enable_tickets(SSL_CTX *ctx)
{
#if TS_HAS_TLS_SESSION_TICKET
- ssl_ticket_key_block *keyblock = nullptr;
-
- keyblock = ssl_create_ticket_keyblock(ticket_key_path);
-
- // On the "first run" the metrics have not been initialized, so this has to
check it.
- if (ssl_rsb.total_ticket_keys_renewed) {
- Metrics::Counter::increment(ssl_rsb.total_ticket_keys_renewed);
- }
-
// Setting the callback can only fail if OpenSSL does not recognize the
// SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB constant. we set the callback first
- // so that we don't leave a ticket_key pointer attached if it fails.
+ // so that we don't leave a ticket-key callback attached if it fails.
if (!ssl_context_enable_ticket_callback(ctx)) {
- ticket_block_free(keyblock);
- return nullptr;
+ return false;
}
-
SSL_CTX_clear_options(ctx, SSL_OP_NO_TICKET);
- return keyblock;
-
+ return true;
#else /* !TS_HAS_TLS_SESSION_TICKET */
- (void)ticket_key_path;
- return nullptr;
+ (void)ctx;
+ return true;
Review Comment:
@JosiahWI Separate certificate contexts are stored per IP because ATS can
listen on multiple local addresses and select the certificate/TLS configuration
associated with the local endpoint. The lookup is also combined with SNI-based
selection, so ATS can select the appropriate SSL_CTX for the local IP and
requested server name.
The contexts can differ in substantially more than the session-ticket key
block. In particular, the selected context can carry a different certificate
chain, private key, CA/client-certificate configuration, OCSP response, TLS
policy, SNI-specific ticket enablement, and TLS 1.3 ticket count. The SSL_CTX
itself also contains the OpenSSL state configured for that certificate and
policy.
The session-ticket key block is different from those certificate-context
properties. It is cryptographic key material used by the ticket callback, and
in the current implementation the callback uses the context key block when one
is present; otherwise it falls back to
SSLTicketKeyConfig::default_global_keyblock.
Before this change, SSLUtils.cc created a fresh random key block while
constructing every certificate context. However, TSSslTicketKeyUpdate() and the
ticket-key configuration reload update the global key block, not those
independently stored context key blocks. That created two separate ownership
paths for ticket keys.
After this change, certificate contexts still have their own SSL_CTX and all
certificate/TLS configuration remains context-specific, but they no longer
receive an independent session-ticket key block by default. Consequently, the
ticket callback uses the globally managed key block consistently for all
default certificate contexts.
In the current source, the global key block is loaded or replaced through
SSLTicketKeyConfig::LoadTicket(), LoadTicketData(), and the public ticket-key
update path. The context key block was assigned during certificate-context
construction and copied when SSLCertContext objects were copied. There is no
intended per-context ticket-key rotation path corresponding to
TSSslTicketKeyUpdate(), which is the mismatch this change fixes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
