diff --git a/iocore/net/SSLSNIConfig.cc b/iocore/net/SSLSNIConfig.cc index 012f1b7acc8..8214d12cbfe 100644 --- a/iocore/net/SSLSNIConfig.cc +++ b/iocore/net/SSLSNIConfig.cc @@ -107,7 +107,7 @@ SNIConfigParams::get_property_config(const std::string &servername) const return nps; } -void +int SNIConfigParams::load_sni_config() { for (auto &item : yaml_sni.items) { @@ -153,13 +153,19 @@ SNIConfigParams::load_sni_config() nps->prop.client_key_file = Layout::get()->relative_to(params->clientKeyPathOnly, item.client_key.data()); } - params->getCTX(nps->prop.client_cert_file, nps->prop.client_key_file, params->clientCACertFilename, params->clientCACertPath); + auto ctx = params->getCTX(nps->prop.client_cert_file, nps->prop.client_key_file, params->clientCACertFilename, + params->clientCACertPath); + if (ctx.get() == nullptr) { + return 1; + } } nps->set_glob_name(item.fqdn); nps->prop.verify_server_policy = item.verify_server_policy; nps->prop.verify_server_properties = item.verify_server_properties; } // end for + + return 0; } std::pair @@ -213,7 +219,8 @@ SNIConfigParams::initialize() if (stat(sni_filename.c_str(), &sbuf) == -1 && errno == ENOENT) { Note("%s failed to load", sni_filename.c_str()); Warning("Loading SNI configuration - filename: %s doesn't exist", sni_filename.c_str()); - return 1; + + return 0; } YamlSNIConfig yaml_sni_tmp; @@ -230,10 +237,7 @@ SNIConfigParams::initialize() } yaml_sni = std::move(yaml_sni_tmp); - load_sni_config(); - Note("%s finished loading", sni_filename.c_str()); - - return 0; + return load_sni_config(); } SNIConfigParams::~SNIConfigParams() @@ -249,19 +253,34 @@ int SNIConfig::_configid = 0; void SNIConfig::startup() { - reconfigure(); + if (!reconfigure()) { + std::string sni_filename = RecConfigReadConfigPath("proxy.config.ssl.servername.filename"); + Fatal("failed to load %s", sni_filename.c_str()); + } } -void +int SNIConfig::reconfigure() { Debug("ssl", "Reload SNI file"); SNIConfigParams *params = new SNIConfigParams; - params->initialize(); - _configid = configProcessor.set(_configid, params); + int retStatus = params->initialize(); + if (!retStatus) { + _configid = configProcessor.set(_configid, params); + prewarmManager.reconfigure(); + } else { + delete params; + } + + std::string sni_filename = RecConfigReadConfigPath("proxy.config.ssl.servername.filename"); + if (!retStatus || TSSystemState::is_initializing()) { + Note("%s finished loading", sni_filename.c_str()); + } else { + Error("%s failed to load", sni_filename.c_str()); + } - prewarmManager.reconfigure(); + return !retStatus; } SNIConfigParams * diff --git a/iocore/net/SSLSNIConfig.h b/iocore/net/SSLSNIConfig.h index 123e66f5c14..70a051d61c7 100644 --- a/iocore/net/SSLSNIConfig.h +++ b/iocore/net/SSLSNIConfig.h @@ -86,7 +86,10 @@ struct SNIConfigParams : public ConfigInfo { const NextHopProperty *get_property_config(const std::string &servername) const; int initialize(); - void load_sni_config(); + /** Walk sni.yaml config and populate sni_action_list + @return 0 for success, 1 is failure + */ + int load_sni_config(); std::pair get(std::string_view servername) const; SNIList sni_action_list; @@ -100,7 +103,10 @@ class SNIConfig using scoped_config = ConfigProcessor::scoped_config; static void startup(); - static void reconfigure(); + /** Loads sni.yaml and swap into place if successful + @return 0 for success, 1 is failure + */ + static int reconfigure(); static SNIConfigParams *acquire(); static void release(SNIConfigParams *params); diff --git a/tests/gold_tests/tls/tls_sni_yaml_reload.test.py b/tests/gold_tests/tls/tls_sni_yaml_reload.test.py new file mode 100644 index 00000000000..d6d525df9f5 --- /dev/null +++ b/tests/gold_tests/tls/tls_sni_yaml_reload.test.py @@ -0,0 +1,131 @@ +# 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. + + +Test.Summary = ''' +Test reloading sni.yaml behaves as expected +''' + +sni_domain = 'example.com' + +ts = Test.MakeATSProcess("ts", command="traffic_manager", enable_tls=True) +server = Test.MakeOriginServer("server") +server2 = Test.MakeOriginServer("server3") +request_header = {"headers": f"GET / HTTP/1.1\r\nHost: {sni_domain}\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) + +ts.Disk.records_config.update({ + 'proxy.config.ssl.server.cert.path': ts.Variables.SSLDir, + 'proxy.config.ssl.server.private_key.path': ts.Variables.SSLDir, + 'proxy.config.ssl.CA.cert.filename': f'{ts.Variables.SSLDir}/signer.pem', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'ssl|http', + 'proxy.config.diags.output.debug': 'L', +}) + + +ts.addDefaultSSLFiles() +ts.addSSLfile("ssl/signed-foo.pem") +ts.addSSLfile("ssl/signed-foo.key") +ts.addSSLfile("ssl/signer.pem") + +ts.Disk.remap_config.AddLine( + f'map / http://127.0.0.1:{server.Variables.Port}' +) + +ts.Disk.ssl_multicert_config.AddLine( + 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' +) + + +ts.Disk.ssl_multicert_config.AddLine( + 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' +) + +ts.Disk.sni_yaml.AddLines( + f""" + sni: + - fqdn: {sni_domain} + http2: off + client_cert: {ts.Variables.SSLDir}/signed-foo.pem + client_key: {ts.Variables.SSLDir}/signed-foo.key + verify_client: STRICT + """.split('\n') +) + +tr = Test.AddTestRun(f'ensure we can connect for SNI {sni_domain}') +tr.Setup.Copy("ssl/signed-foo.pem") +tr.Setup.Copy("ssl/signed-foo.key") +tr.Processes.Default.StartBefore(Test.Processes.ts) +tr.Processes.Default.StartBefore(server) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.Processes.Default.Command = f"curl -q --tls-max 1.2 -s -v -k --cert ./signed-foo.pem --key ./signed-foo.key --resolve '{sni_domain}:{ts.Variables.ssl_port}:127.0.0.1' https://{sni_domain}:{ts.Variables.ssl_port}" + +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression("Could Not Connect", "Verify curl could successfully connect") +tr.Processes.Default.Streams.stderr = Testers.IncludesExpression(f"CN={sni_domain}", f"Verify curl used the {sni_domain} SNI") +ts.Disk.diags_log.Content = Testers.IncludesExpression( + "SSL negotiation finished successfully", + "Verify that the TLS handshake was successful") + +# This config reload should fail because it references non-existent TLS key files +trupd = Test.AddTestRun("Update config file") +# Update the configs - this will overwrite the sni.yaml file +sniyamlpath = ts.Disk.sni_yaml.AbsPath +trupd.Disk.File(sniyamlpath, id="sni_yaml", typename="ats:config") +trupd.Disk.sni_yaml.AddLines( + f""" + sni: + - fqdn: {sni_domain} + http2: on + client_cert: {ts.Variables.SSLDir}/signed-notexist.pem + client_key: {ts.Variables.SSLDir}/signed-notexist.key + verify_client: STRICT + """.split('\n') +) + +trupd.StillRunningAfter = ts +trupd.StillRunningAfter = server +trupd.Processes.Default.Command = 'echo Updated configs' +trupd.Processes.Default.Env = ts.Env +trupd.Processes.Default.ReturnCode = 0 + + +tr2reload = Test.AddTestRun("Reload config") +tr2reload.StillRunningAfter = ts +tr2reload.StillRunningAfter = server +tr2reload.Processes.Default.Command = 'traffic_ctl config reload' +tr2reload.Processes.Default.Env = ts.Env +tr2reload.Processes.Default.ReturnCode = 0 +ts.Disk.diags_log.Content = Testers.ContainsExpression( + 'sni.yaml failed to load', + 'reload should result in failure to load sni.yaml') + +tr3 = Test.AddTestRun(f"Make request again for {sni_domain} that should still work") +# Wait for the reload to complete +tr3.Setup.Copy("ssl/signed-bar.pem") +tr3.Setup.Copy("ssl/signed-bar.key") +tr3.Processes.Default.StartBefore(server2, ready=When.FileContains(ts.Disk.diags_log.Name, "signed-notexist.pem", 1)) +tr3.StillRunningAfter = ts +tr3.StillRunningAfter = server +tr3.Processes.Default.Command = f"curl -q --tls-max 1.2 -s -v -k --cert ./signed-bar.pem --key ./signed-bar.key --resolve '{sni_domain}:{ts.Variables.ssl_port}:127.0.0.1' https://{sni_domain}:{ts.Variables.ssl_port}" + +tr3.Processes.Default.ReturnCode = 0 +# since the 2nd config with http2 turned on should have failed and used the prior config, verify http2 was not used +tr3.Processes.Default.Streams.stderr = Testers.ExcludesExpression("GET / HTTP/2", "Confirm that HTTP2 is still not used")