diff --git a/src/iocore/net/P_SSLCertLookup.h b/src/iocore/net/P_SSLCertLookup.h index 5d13a6300f4..6367818ece3 100644 --- a/src/iocore/net/P_SSLCertLookup.h +++ b/src/iocore/net/P_SSLCertLookup.h @@ -26,6 +26,8 @@ #include "iocore/eventsystem/ConfigProcessor.h" #include "iocore/net/SSLTypes.h" #include "records/RecCore.h" +#include "tsutil/Bravo.h" +#include #include #include @@ -134,8 +136,7 @@ struct SSLCertLookup : public ConfigInfo { std::unique_ptr ssl_storage; std::unique_ptr ec_storage; - shared_SSL_CTX ssl_default; - bool is_valid = true; + bool is_valid = true; int insert(const char *name, SSLCertContext const &cc); int insert(const IpEndpoint &address, SSLCertContext const &cc); @@ -146,6 +147,7 @@ struct SSLCertLookup : public ConfigInfo { @return @c A pointer to the matched context, @c nullptr if no match is found. */ SSLCertContext *find(const IpEndpoint &address) const; + SSLCertContext *find(const IpEndpoint &address, [[maybe_unused]] SSLCertContextType ctxType) const; /** Find certificate context by name (FQDN). Exact matches have priority, then wildcards. Only destination based matches are checked. @@ -154,10 +156,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(); + ts::bravo::shared_lock 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 +180,9 @@ struct SSLCertLookup : public ConfigInfo { ~SSLCertLookup() override; private: + mutable ts::bravo::shared_mutex default_ctx_mutex; + mutable shared_SSL_CTX ssl_default; + // 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/SSLCertLookup.cc b/src/iocore/net/SSLCertLookup.cc index 4b51a21579e..bf33d0294f6 100644 --- a/src/iocore/net/SSLCertLookup.cc +++ b/src/iocore/net/SSLCertLookup.cc @@ -273,8 +273,8 @@ SSLCertContext::setCtx(shared_SSL_CTX sc) SSLCertLookup::SSLCertLookup() : ssl_storage(std::make_unique()), ec_storage(std::make_unique()), - ssl_default(nullptr), - is_valid(true) + is_valid(true), + ssl_default(nullptr) { } @@ -299,36 +299,48 @@ SSLCertLookup::find(const std::string &address, [[maybe_unused]] SSLCertContextT SSLCertContext * SSLCertLookup::find(const IpEndpoint &address) const { - SSLCertContext *cc; - SSLAddressLookupKey key(address); - #ifdef OPENSSL_IS_BORINGSSL // If the context is EC supportable, try finding that first. - if ((cc = this->ec_storage->lookup(key.get()))) { + if (auto *cc = this->find(address, SSLCertContextType::EC)) { return cc; } +#endif - // If that failed, try the address without the port. - if (address.network_order_port()) { - key.split(); - if ((cc = this->ec_storage->lookup(key.get()))) { - return cc; - } - } + return this->find(address, SSLCertContextType::RSA); +} + +SSLCertContext * +SSLCertLookup::find(const IpEndpoint &address, [[maybe_unused]] SSLCertContextType ctxType) const +{ + SSLAddressLookupKey key(address); - // reset for search across RSA - key = SSLAddressLookupKey(address); +#ifdef OPENSSL_IS_BORINGSSL + SSLContextStorage *storage = nullptr; + switch (ctxType) { + case SSLCertContextType::GENERIC: + case SSLCertContextType::RSA: + storage = ssl_storage.get(); + break; + case SSLCertContextType::EC: + storage = ec_storage.get(); + break; + default: + ink_assert(false); + return nullptr; + } +#else + SSLContextStorage *storage = ssl_storage.get(); #endif // First try the full address. - if ((cc = this->ssl_storage->lookup(key.get()))) { + if (auto *cc = storage->lookup(key.get())) { return cc; } // If that failed, try the address without the port. if (address.network_order_port()) { key.split(); - return this->ssl_storage->lookup(key.get()); + return storage->lookup(key.get()); } return nullptr; 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..635fac9be22 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -1733,6 +1733,27 @@ SSLMultiCertConfigLoader::update_ssl_ctx(const std::string &secret_name) if (!ctx) { retval = false; } else { + if ((*policy_iter)->addr) { + SSLCertContext *cc = nullptr; + if (strcmp((*policy_iter)->addr, "*") == 0) { + this->_set_handshake_callbacks(ctx.get()); + cc = lookup->find("*", loadingctx.ctx_type); + } else { + IpEndpoint ep; + if (ats_ip_pton((*policy_iter)->addr, &ep) == 0) { + cc = lookup->find(ep, loadingctx.ctx_type); + } else { + Error("'%s' is not a valid IPv4 or IPv6 address", (const char *)(*policy_iter)->addr); + retval = false; + } + } + if (cc && cc->userconfig.get() == policy_iter->get()) { + cc->setCtx(ctx); + if (strcmp((*policy_iter)->addr, "*") == 0) { + lookup->setDefaultContext(ctx); + } + } + } for (auto const &name : common_names) { SSLCertContext *cc = lookup->find(name, loadingctx.ctx_type); if (cc && cc->userconfig.get() == policy_iter->get()) { @@ -1787,8 +1808,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 +1911,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..ae530c70ad8 --- /dev/null +++ b/tests/gold_tests/tls/tls_secret_update_default.test.py @@ -0,0 +1,234 @@ +''' +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 +import re + +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: str = 'signed-bar.pem' + updated_cert: str = 'signed2-bar.pem' + key_file: str = 'signed-bar.key' + shadowed_cert: str = 'signed-foo.pem' + updated_shadowed_cert: str = 'signed2-foo.pem' + shadowed_key_file: str = 'signed-foo.key' + plugin: str = 'ssl_secret_load_test.so' + + def __init__(self) -> None: + '''Configure the ATS process, origin server, and test runs.''' + self._configure_server() + self._configure_traffic_server() + self._add_start_run() + self._add_config_touch_run() + self._add_config_reload_run() + self._add_config_reload_wait_run() + self._add_initial_certificate_run() + self._add_mtime_delay_run() + self._add_shadowed_certificate_update_run() + self._add_secret_update_wait_run(self.shadowed_cert) + self._add_unchanged_default_certificate_run() + self._add_default_certificate_update_run() + self._add_secret_update_wait_run(self.initial_cert) + self._add_updated_certificate_run() + + def _configure_server(self) -> 'Process': + '''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) -> 'Process': + '''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}') + ts.addSSLfile(f'ssl/{self.shadowed_cert}') + ts.addSSLfile(f'ssl/{self.updated_shadowed_cert}') + ts.addSSLfile(f'ssl/{self.shadowed_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', + 'proxy.config.ssl.server.cert.path': f'{ts.Variables.SSLDir}/../', + 'proxy.config.ssl.server.private_key.path': f'{ts.Variables.SSLDir}/../', + 'proxy.config.ssl.server.multicert.concurrency': 1, + '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}', + ' - dest_ip: "*"', + f' ssl_cert_name: {self.shadowed_cert}', + f' ssl_key_name: {self.shadowed_key_file}', + ]) + ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{self._server.Variables.Port}') + return ts + + def _add_start_run(self) -> 'TestRun': + '''Start ATS before reloading the multicert configuration serially.''' + tr = Test.AddTestRun('Start ATS') + tr.Processes.Default.Command = 'echo starting ATS' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + self._keep_processes_running(tr) + return tr + + def _add_config_reload_run(self) -> 'TestRun': + '''Reload the multicert configuration with deterministic ordering.''' + tr = Test.AddTestRun('Reload the multicert configuration serially') + tr.Processes.Default.Command = 'traffic_ctl config reload' + tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.ReturnCode = 0 + self._keep_processes_running(tr) + return tr + + def _add_config_touch_run(self) -> 'TestRun': + '''Mark the multicert configuration for reload.''' + tr = Test.AddTestRun('Mark the multicert configuration for reload') + tr.Processes.Default.Command = f'touch {self._ts.Disk.ssl_multicert_yaml.AbsPath}' + tr.Processes.Default.ReturnCode = 0 + self._keep_processes_running(tr) + return tr + + def _add_config_reload_wait_run(self) -> 'TestRun': + '''Wait for the serial multicert configuration reload to finish.''' + tr = Test.AddTestRun('Wait for the multicert configuration reload') + tr.Processes.Default.Command = 'echo awaiting multicert reload' + tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.ReturnCode = 0 + await_reload = tr.Processes.Process('await_reload', 'sleep 30') + await_reload.Ready = When.FileContains(self._ts.Disk.diags_log.Name, 'ssl_multicert.yaml finished loading', 2) + tr.Processes.Default.StartBefore(await_reload) + self._keep_processes_running(tr) + return tr + + def _add_initial_certificate_run(self) -> 'TestRun': + '''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.Streams.All += Testers.ContainsExpression(r'subject:.*CN=bar\.com', 'Default cert is bar.com.') + tr.Processes.Default.Streams.All += Testers.ExcludesExpression(r'subject:.*CN=foo\.com', 'Shadowed cert is not served.') + return tr + + def _add_mtime_delay_run(self) -> 'TestRun': + '''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_shadowed_certificate_update_run(self) -> 'TestRun': + '''Replace the certificate for the shadowed wildcard policy.''' + tr = Test.AddTestRun('Update the shadowed wildcard certificate') + tr.Setup.CopyAs(f'ssl/{self.updated_shadowed_cert}', '.', f'{self._ts.Variables.SSLDir}/{self.shadowed_cert}') + tr.Processes.Default.Command = f'touch {self._ts.Variables.SSLDir}/{self.shadowed_cert}' + tr.Processes.Default.ReturnCode = 0 + self._keep_processes_running(tr) + return tr + + def _add_unchanged_default_certificate_run(self) -> 'TestRun': + '''Verify that refreshing a shadowed wildcard does not replace the default.''' + tr = self._add_curl_run( + 'Default certificate unchanged after shadowed wildcard update', + expected_cn='signer.yahoo.com', + unexpected_cn='signer2.yahoo.com', + expected_description='Default cert still uses signer.', + unexpected_description='Shadowed wildcard cert did not become the default.') + tr.Processes.Default.Streams.All += Testers.ContainsExpression(r'subject:.*CN=bar\.com', 'Default cert is still bar.com.') + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + r'subject:.*CN=foo\.com', 'Shadowed foo.com cert is not served.') + return tr + + def _add_default_certificate_update_run(self) -> 'TestRun': + '''Replace the plugin backing default certificate file.''' + tr = Test.AddTestRun('Update the default 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, secret_name: str) -> 'TestRun': + '''Wait for the test plugin to update the certificate secret.''' + tr = Test.AddTestRun(f'Wait for the {secret_name} 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, f'updated cert for secret .*{re.escape(secret_name)}') + tr.Processes.Default.StartBefore(await_update) + self._keep_processes_running(tr) + return tr + + def _add_updated_certificate_run(self) -> 'TestRun': + '''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: str, expected_cn: str, unexpected_cn: str, expected_description: str, + unexpected_description: str) -> 'TestRun': + '''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={re.escape(expected_cn)}', expected_description) + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + f'issuer:.*CN={re.escape(unexpected_cn)}', unexpected_description) + return tr + + def _keep_processes_running(self, tr: 'TestRun') -> None: + '''Keep ATS and the origin server running after a TestRun.''' + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + + +TestDefaultSecretUpdate() diff --git a/tests/tools/plugins/ssl_secret_load_test.cc b/tests/tools/plugins/ssl_secret_load_test.cc index 54c26eeb0a4..b9f1dfc6560 100644 --- a/tests/tools/plugins/ssl_secret_load_test.cc +++ b/tests/tools/plugins/ssl_secret_load_test.cc @@ -159,8 +159,12 @@ CB_Update_Secret(TSCont cont, TSEvent /* event ATS_UNUSED */, void * /* edata AT } } for (auto name : updates) { - Dbg(dbg_ctl, "update cert for secret %s", name.c_str()); - TSSslSecretUpdate(name.c_str(), name.length()); + TSReturnCode result = TSSslSecretUpdate(name.c_str(), name.length()); + if (result == TS_SUCCESS) { + Dbg(dbg_ctl, "updated cert for secret %s", name.c_str()); + } else { + TSError("[%s] failed to update cert for secret %s", PN, name.c_str()); + } } TSContScheduleOnPool(cont, 3000, TS_THREAD_POOL_TASK); return TS_SUCCESS;