From bac9ce386360f0c7bfe394c924fabc7f712838c2 Mon Sep 17 00:00:00 2001 From: bneradt Date: Fri, 26 Jun 2026 13:30:15 -0500 Subject: [PATCH] Refresh default TLS context on secret update 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 --- src/iocore/net/P_SSLCertLookup.h | 18 ++- src/iocore/net/QUICPacketHandler.cc | 3 +- src/iocore/net/SSLNetVConnection.cc | 9 +- src/iocore/net/SSLStats.cc | 5 +- src/iocore/net/SSLUtils.cc | 18 ++- .../tls/tls_secret_update_default.test.py | 151 ++++++++++++++++++ 6 files changed, 190 insertions(+), 14 deletions(-) create mode 100644 tests/gold_tests/tls/tls_secret_update_default.test.py diff --git a/src/iocore/net/P_SSLCertLookup.h b/src/iocore/net/P_SSLCertLookup.h index 5d13a6300f4..f10b1437674 100644 --- a/src/iocore/net/P_SSLCertLookup.h +++ b/src/iocore/net/P_SSLCertLookup.h @@ -134,8 +134,8 @@ struct SSLCertLookup : public ConfigInfo { std::unique_ptr ssl_storage; std::unique_ptr ec_storage; - shared_SSL_CTX ssl_default; - bool is_valid = true; + mutable shared_SSL_CTX ssl_default; + bool is_valid = true; int insert(const char *name, SSLCertContext const &cc); int insert(const IpEndpoint &address, SSLCertContext const &cc); @@ -154,10 +154,18 @@ struct SSLCertLookup : public ConfigInfo { SSLCertContext *find(const std::string &name, SSLCertContextType ctxType = SSLCertContextType::GENERIC) const; // Return the last-resort default TLS context if there is no name or address match. - SSL_CTX * + shared_SSL_CTX defaultContext() const { - return ssl_default.get(); + std::lock_guard lock(default_ctx_mutex); + return ssl_default; + } + + void + setDefaultContext(shared_SSL_CTX ctx) const + { + std::lock_guard lock(default_ctx_mutex); + ssl_default = std::move(ctx); } unsigned count(SSLCertContextType ctxType = SSLCertContextType::GENERIC) const; @@ -170,6 +178,8 @@ struct SSLCertLookup : public ConfigInfo { ~SSLCertLookup() override; private: + mutable std::mutex default_ctx_mutex; + // Map cert_secret name to lookup keys std::unordered_map> cert_secret_registry; }; diff --git a/src/iocore/net/QUICPacketHandler.cc b/src/iocore/net/QUICPacketHandler.cc index d8650c858d0..406c9ef7204 100644 --- a/src/iocore/net/QUICPacketHandler.cc +++ b/src/iocore/net/QUICPacketHandler.cc @@ -281,7 +281,8 @@ QUICPacketHandlerIn::_recv_packet(int /* event ATS_UNUSED */, UDPPacket *udp_pac QUICConnectionId new_cid; QUICCertConfig::scoped_config server_cert; - SSL *ssl = SSL_new(server_cert->defaultContext()); + auto default_ctx = server_cert->defaultContext(); + SSL *ssl = SSL_new(default_ctx.get()); quiche_conn *quiche_con = quiche_conn_new_with_tls( new_cid, new_cid.length(), retry_token.original_dcid(), retry_token.original_dcid().length(), &udp_packet->to.sa, diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index 95b7e57f11a..b86f987e7e1 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -1108,7 +1108,8 @@ SSLNetVConnection::_sslStartHandShake(int event, int &err) } ats_ip_nptop(&dst, ipb1, sizeof(ipb1)); ats_ip_nptop(&src, ipb2, sizeof(ipb2)); - DbgPrint(dbg_ctl_ssl, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, lookup->defaultContext()); + auto default_ctx = lookup->defaultContext(); + DbgPrint(dbg_ctl_ssl, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, default_ctx.get()); } // Escape if this is marked to be a tunnel. @@ -1130,7 +1131,7 @@ SSLNetVConnection::_sslStartHandShake(int event, int &err) // Attach the default SSL_CTX to this SSL session. The default context is never going to be able // to negotiate a SSL session, but it's enough to trampoline us into the SNI callback where we // can select the right server certificate. - this->_make_ssl_connection(lookup->defaultContext()); + this->_make_ssl_connection(lookup->defaultContext().get()); } if (this->ssl == nullptr) { @@ -2005,8 +2006,8 @@ SSLNetVConnection::_lookupContextByIP() return nullptr; } ats_ip_nptop(&src, ipb2, sizeof(ipb2)); - DbgPrint(dbg_ctl_proxyprotocol, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, - lookup->defaultContext()); + auto default_ctx = lookup->defaultContext(); + DbgPrint(dbg_ctl_proxyprotocol, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, default_ctx.get()); } } else if (0 == safe_getsockname(this->get_socket(), &ip.sa, &namelen)) { cc = lookup->find(ip); diff --git a/src/iocore/net/SSLStats.cc b/src/iocore/net/SSLStats.cc index 11d82d2b247..5bbe3dfbcf4 100644 --- a/src/iocore/net/SSLStats.cc +++ b/src/iocore/net/SSLStats.cc @@ -273,12 +273,13 @@ SSLInitializeStatistics() // Acquire the loaded SSL certificate configuration to enumerate ciphers and groups. // This must be called AFTER SSLCertificateConfig::startup(). SSLCertificateConfig::scoped_config lookup; - if (!lookup || !lookup->ssl_default) { + auto default_ctx = lookup ? lookup->defaultContext() : nullptr; + if (!default_ctx) { Dbg(dbg_ctl_ssl, "No SSL configuration, skipping cipher/group statistics initialization"); return; } - SSL_CTX *ctx = lookup->ssl_default.get(); + SSL_CTX *ctx = default_ctx.get(); SSL *ssl = SSL_new(ctx); STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl); diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index 407dfd4b87b..f4f2e20c61d 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -1733,6 +1733,18 @@ SSLMultiCertConfigLoader::update_ssl_ctx(const std::string &secret_name) if (!ctx) { retval = false; } else { + // Address and default-context lookups are owned by the same policy, but + // are not part of the certificate CN/SAN name sets below. + for (unsigned i = 0; i < lookup->count(loadingctx.ctx_type); ++i) { + SSLCertContext *cc = lookup->get(i, loadingctx.ctx_type); + if (cc && cc->ctx_type == loadingctx.ctx_type && cc->userconfig.get() == policy_iter->get()) { + cc->setCtx(ctx); + } + } + if ((*policy_iter)->addr && strcmp((*policy_iter)->addr, "*") == 0) { + lookup->setDefaultContext(ctx); + this->_set_handshake_callbacks(ctx.get()); + } for (auto const &name : common_names) { SSLCertContext *cc = lookup->find(name, loadingctx.ctx_type); if (cc && cc->userconfig.get() == policy_iter->get()) { @@ -1787,8 +1799,8 @@ SSLMultiCertConfigLoader::_store_single_ssl_ctx(SSLCertLookup *lookup, const sha if (strcmp(sslMultCertSettings->addr, "*") == 0) { Dbg(dbg_ctl_ssl_load, "Addr is '*'; setting %p to default", ctx.get()); if (lookup->insert(sslMultCertSettings->addr, SSLCertContext(ctx, ctx_type, sslMultCertSettings, keyblock)) >= 0) { - inserted = true; - lookup->ssl_default = ctx; + inserted = true; + lookup->setDefaultContext(ctx); this->_set_handshake_callbacks(ctx.get()); } } else { @@ -1890,7 +1902,7 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup, bool firstLoad) // We *must* have a default context even if it can't possibly work. The default context is used to // bootstrap the SSL handshake so that we can subsequently do the SNI lookup to switch to the real // context. - if (lookup->ssl_default == nullptr) { + if (lookup->defaultContext() == nullptr) { shared_SSLMultiCertConfigParams sslMultiCertSettings(new SSLMultiCertConfigParams); sslMultiCertSettings->addr = ats_strdup("*"); if (!this->_store_ssl_ctx(lookup, sslMultiCertSettings)) { diff --git a/tests/gold_tests/tls/tls_secret_update_default.test.py b/tests/gold_tests/tls/tls_secret_update_default.test.py new file mode 100644 index 00000000000..3c47dc9ccf4 --- /dev/null +++ b/tests/gold_tests/tls/tls_secret_update_default.test.py @@ -0,0 +1,151 @@ +''' +Test default server certificate updates through TSSslSecretSet/TSSslSecretUpdate. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +Test.Summary = ''' +Verify that secret updates refresh the default server SSL_CTX used without SNI. +''' + + +class TestDefaultSecretUpdate: + '''Verify default server certificate updates through the secret API.''' + + initial_cert = 'signed-bar.pem' + updated_cert = 'signed2-bar.pem' + key_file = 'signed-bar.key' + plugin = 'ssl_secret_load_test.so' + + def __init__(self): + '''Configure the ATS process, origin server, and test runs.''' + self._configure_server() + self._configure_traffic_server() + self._add_initial_certificate_run() + self._add_mtime_delay_run() + self._add_certificate_update_run() + self._add_secret_update_wait_run() + self._add_updated_certificate_run() + + def _configure_server(self): + '''Configure the origin server.''' + server = Test.MakeOriginServer('server') + self._server = server + + request_header = {'headers': 'GET / HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n', 'timestamp': '1469733493.993', 'body': ''} + response_header = {'headers': 'HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n', 'timestamp': '1469733493.993', 'body': ''} + server.addResponse('sessionlog.json', request_header, response_header) + return server + + def _configure_traffic_server(self): + '''Configure the Traffic Server process.''' + ts = Test.MakeATSProcess('ts', enable_tls=True) + self._ts = ts + + ts.addSSLfile(f'ssl/{self.initial_cert}') + ts.addSSLfile(f'ssl/{self.updated_cert}') + ts.addSSLfile(f'ssl/{self.key_file}') + Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, self.plugin), ts) + + ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'ssl_secret_load_test|ssl|ssl_config_updateCTX', + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}/../', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}/../', + 'proxy.config.exec_thread.autoconfig.scale': 1.0, + 'proxy.config.url_remap.pristine_host_hdr': 1, + }) + + ts.Disk.ssl_multicert_yaml.AddLines( + [ + 'ssl_multicert:', + ' - dest_ip: "*"', + f' ssl_cert_name: {self.initial_cert}', + f' ssl_key_name: {self.key_file}', + ]) + ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{self._server.Variables.Port}') + return ts + + def _add_initial_certificate_run(self): + '''Verify the original default certificate before the secret update.''' + tr = self._add_curl_run( + 'Initial default certificate', + expected_cn='signer.yahoo.com', + unexpected_cn='signer2.yahoo.com', + expected_description='Initial cert uses signer.', + unexpected_description='Initial cert is not updated yet.') + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + return tr + + def _add_mtime_delay_run(self): + '''Make the replacement certificate mtime differ from the original.''' + tr = Test.AddTestRun('Make the cert mtime differ') + tr.Processes.Default.Command = 'sleep 2' + tr.Processes.Default.ReturnCode = 0 + self._keep_processes_running(tr) + return tr + + def _add_certificate_update_run(self): + '''Replace the plugin backing certificate file.''' + tr = Test.AddTestRun('Update the plugin backing certificate') + tr.Setup.CopyAs(f'ssl/{self.updated_cert}', '.', f'{self._ts.Variables.SSLDir}/{self.initial_cert}') + tr.Processes.Default.Command = f'touch {self._ts.Variables.SSLDir}/{self.initial_cert}' + tr.Processes.Default.ReturnCode = 0 + self._keep_processes_running(tr) + return tr + + def _add_secret_update_wait_run(self): + '''Wait for the test plugin to update the certificate secret.''' + tr = Test.AddTestRun('Wait for the secret update') + tr.Processes.Default.Command = 'echo awaiting secret update' + tr.Processes.Default.ReturnCode = 0 + await_update = tr.Processes.Process('await_update', 'sleep 30') + await_update.Ready = When.FileContains(self._ts.Disk.traffic_out.Name, 'update cert for secret') + tr.Processes.Default.StartBefore(await_update) + self._keep_processes_running(tr) + return tr + + def _add_updated_certificate_run(self): + '''Verify the updated default certificate after the secret update.''' + return self._add_curl_run( + 'Updated default certificate', + expected_cn='signer2.yahoo.com', + unexpected_cn='signer.yahoo.com', + expected_description='Updated cert uses signer2.', + unexpected_description='Updated cert no longer uses signer.') + + def _add_curl_run(self, name, expected_cn, unexpected_cn, expected_description, unexpected_description): + '''Add a curl run against ATS without SNI.''' + tr = Test.AddTestRun(name) + self._keep_processes_running(tr) + tr.MakeCurlCommand( + f"-k -v --http1.1 -H 'host: doesnotmatter' https://127.0.0.1:{self._ts.Variables.ssl_port}/", ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Streams.All = Testers.ContainsExpression(f'issuer:.*CN={expected_cn}', expected_description) + tr.Processes.Default.Streams.All += Testers.ExcludesExpression(f'issuer:.*CN={unexpected_cn}', unexpected_description) + return tr + + def _keep_processes_running(self, tr): + '''Keep ATS and the origin server running after a TestRun.''' + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + + +TestDefaultSecretUpdate()