Refresh default TLS context on secret update

src/iocore/net/P_SSLCertLookup.h

@@ -134,8 +134,8 @@ struct SSLCertLookup : public ConfigInfo {
   std::unique_ptr<SSLContextStorage> ssl_storage;
   std::unique_ptr<SSLContextStorage> 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<std::mutex> lock(default_ctx_mutex);
+    return ssl_default;
+  }
+
+  void
+  setDefaultContext(shared_SSL_CTX ctx) const
+  {
+    std::lock_guard<std::mutex> 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<std::string, std::vector<std::string>> cert_secret_registry;
 };

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,

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);

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);
 

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)) {

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()