This is an automated email from the ASF dual-hosted git repository. zwoop pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 69944c4656a020c6db95d03b20ba0dbd2a2a591b Author: Susan Hinrichs <[email protected]> AuthorDate: Mon Nov 30 14:03:52 2020 -0600 Make reloading client certificate configuration more reliable (#7313) (cherry picked from commit 495a3a13dc0dc3195d725e3ff03d9aa634884335) --- iocore/net/Makefile.am | 2 + iocore/net/P_SSLClientCoordinator.h | 34 ++++++++++++++ iocore/net/SSLClientCoordinator.cc | 54 ++++++++++++++++++++++ iocore/net/SSLConfig.cc | 10 ++-- iocore/net/SSLNetProcessor.cc | 5 +- iocore/net/SSLSNIConfig.cc | 5 +- tests/gold_tests/tls/gold/proxycert-accesslog.gold | 4 ++ tests/gold_tests/tls/tls_client_cert.test.py | 2 +- 8 files changed, 102 insertions(+), 14 deletions(-) diff --git a/iocore/net/Makefile.am b/iocore/net/Makefile.am index 6af18e9..fa60587 100644 --- a/iocore/net/Makefile.am +++ b/iocore/net/Makefile.am @@ -120,6 +120,7 @@ libinknet_a_SOURCES = \ P_SSLNextProtocolSet.h \ P_SSLSNI.h \ P_SSLUtils.h \ + P_SSLClientCoordinator.h \ P_SSLClientUtils.h \ P_OCSPStapling.h \ P_UDPConnection.h \ @@ -137,6 +138,7 @@ libinknet_a_SOURCES = \ ProxyProtocol.cc \ Socks.cc \ SSLCertLookup.cc \ + SSLClientCoordinator.cc \ SSLClientUtils.cc \ SSLConfig.cc \ SSLDiags.cc \ diff --git a/iocore/net/P_SSLClientCoordinator.h b/iocore/net/P_SSLClientCoordinator.h new file mode 100644 index 0000000..779653e --- /dev/null +++ b/iocore/net/P_SSLClientCoordinator.h @@ -0,0 +1,34 @@ +/** @file + + P_SSLClientCoordinator.h - coordinate the loading of SSL related configs + + @section license License + + 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. + */ + +#include "ProxyConfig.h" +#include <memory> + +// A class to pass the ConfigUpdateHandler, so both SSLConfig and SNIConfig get updated +// when the relevant files/configs get updated. +class SSLClientCoordinator +{ +public: + static void startup(); + static void reconfigure(); +}; diff --git a/iocore/net/SSLClientCoordinator.cc b/iocore/net/SSLClientCoordinator.cc new file mode 100644 index 0000000..c58ccd1 --- /dev/null +++ b/iocore/net/SSLClientCoordinator.cc @@ -0,0 +1,54 @@ +/** @file + + SSLClientCoordinator.cc - Coordinate the loading of SSL related configs + + @section license License + + 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. + */ + +#include "P_SSLClientCoordinator.h" +#include "P_SSLConfig.h" +#include "P_SSLSNI.h" + +std::unique_ptr<ConfigUpdateHandler<SSLClientCoordinator>> sslClientUpdate; + +void +SSLClientCoordinator::reconfigure() +{ + // The SSLConfig must have its configuration loaded before the SNIConfig. + // The SSLConfig owns the client cert context storage and the SNIConfig will load + // into it. + SSLConfig::reconfigure(); + SNIConfig::reconfigure(); +} + +void +SSLClientCoordinator::startup() +{ + // The SSLConfig must have its configuration loaded before the SNIConfig. + // The SSLConfig owns the client cert context storage and the SNIConfig will load + // into it. + sslClientUpdate.reset(new ConfigUpdateHandler<SSLClientCoordinator>()); + sslClientUpdate->attach("proxy.config.ssl.client.cert.path"); + sslClientUpdate->attach("proxy.config.ssl.client.cert.filename"); + sslClientUpdate->attach("proxy.config.ssl.client.private_key.path"); + sslClientUpdate->attach("proxy.config.ssl.client.private_key.filename"); + SSLConfig::startup(); + sslClientUpdate->attach("proxy.config.ssl.servername.filename"); + SNIConfig::startup(); +} diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 7c05881..c685329 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -43,6 +43,7 @@ #include "P_Net.h" #include "P_SSLClientUtils.h" #include "P_SSLCertLookup.h" +#include "P_SSLSNI.h" #include "SSLDiags.h" #include "SSLSessionCache.h" #include "SSLSessionTicket.h" @@ -75,7 +76,6 @@ int SSLConfigParams::async_handshake_enabled = 0; char *SSLConfigParams::engine_conf_file = nullptr; static std::unique_ptr<ConfigUpdateHandler<SSLCertificateConfig>> sslCertUpdate; -static std::unique_ptr<ConfigUpdateHandler<SSLConfig>> sslConfigUpdate; static std::unique_ptr<ConfigUpdateHandler<SSLTicketKeyConfig>> sslTicketKey; SSLConfigParams::SSLConfigParams() @@ -427,17 +427,13 @@ SSLConfigParams::getClientSSL_CTX() const void SSLConfig::startup() { - sslConfigUpdate.reset(new ConfigUpdateHandler<SSLConfig>()); - sslConfigUpdate->attach("proxy.config.ssl.client.cert.path"); - sslConfigUpdate->attach("proxy.config.ssl.client.cert.filename"); - sslConfigUpdate->attach("proxy.config.ssl.client.private_key.path"); - sslConfigUpdate->attach("proxy.config.ssl.client.private_key.filename"); reconfigure(); } void SSLConfig::reconfigure() { + Debug("ssl", "Reload SSLConfig"); SSLConfigParams *params; params = new SSLConfigParams; params->initialize(); // re-read configuration @@ -656,6 +652,8 @@ SSLConfigParams::getCTX(const char *client_cert, const char *key_file, const cha ts::bwprint(top_level_key, "{}:{}", ca_bundle_file, ca_bundle_path); ts::bwprint(ctx_key, "{}:{}", client_cert, key_file); + Debug("ssl", "Load client cert %s %s", client_cert, key_file); + auto ctx_map_iter = top_level_ctx_map.find(top_level_key); if (ctx_map_iter != top_level_ctx_map.end()) { auto ctx_iter = ctx_map_iter->second.find(ctx_key); diff --git a/iocore/net/SSLNetProcessor.cc b/iocore/net/SSLNetProcessor.cc index 0707cc3..7318886 100644 --- a/iocore/net/SSLNetProcessor.cc +++ b/iocore/net/SSLNetProcessor.cc @@ -26,8 +26,8 @@ #include "records/I_RecHttp.h" #include "P_SSLUtils.h" #include "P_OCSPStapling.h" -#include "P_SSLSNI.h" #include "SSLStats.h" +#include "P_SSLClientCoordinator.h" // // Global Data @@ -60,9 +60,8 @@ SSLNetProcessor::start(int, size_t stacksize) { // This initialization order matters ... SSLInitializeLibrary(); - SSLConfig::startup(); + SSLClientCoordinator::startup(); SSLPostConfigInitialize(); - SNIConfig::startup(); if (!SSLCertificateConfig::startup()) { return -1; diff --git a/iocore/net/SSLSNIConfig.cc b/iocore/net/SSLSNIConfig.cc index ec10fc6..d1de514 100644 --- a/iocore/net/SSLSNIConfig.cc +++ b/iocore/net/SSLSNIConfig.cc @@ -32,14 +32,12 @@ #include "P_SSLSNI.h" #include "tscore/Diags.h" #include "tscore/SimpleTokenizer.h" -#include "P_SSLConfig.h" #include "tscore/ink_memory.h" #include "tscpp/util/TextView.h" #include "tscore/I_Layout.h" #include <sstream> #include <pcre.h> -static ConfigUpdateHandler<SNIConfig> *sniConfigUpdate; static constexpr int OVECSIZE{30}; const NextHopProperty * @@ -191,14 +189,13 @@ SNIConfigParams::~SNIConfigParams() void SNIConfig::startup() { - sniConfigUpdate = new ConfigUpdateHandler<SNIConfig>(); - sniConfigUpdate->attach("proxy.config.ssl.servername.filename"); reconfigure(); } void SNIConfig::reconfigure() { + Debug("ssl", "Reload SNI file"); SNIConfigParams *params = new SNIConfigParams; params->Initialize(); diff --git a/tests/gold_tests/tls/gold/proxycert-accesslog.gold b/tests/gold_tests/tls/gold/proxycert-accesslog.gold index 130f1ac..d100cd0 100644 --- a/tests/gold_tests/tls/gold/proxycert-accesslog.gold +++ b/tests/gold_tests/tls/gold/proxycert-accesslog.gold @@ -6,3 +6,7 @@ 502 https://127.0.0.1:``/ 2 0 404 https://127.0.0.1:``/ 2 0 502 https://127.0.0.1:``/ 2 0 +404 https://127.0.0.1:``/ 2 0 +502 https://127.0.0.1:``/ 2 0 +200 https://127.0.0.1:``/ 2 0 +502 https://127.0.0.1:``/ 2 0 diff --git a/tests/gold_tests/tls/tls_client_cert.test.py b/tests/gold_tests/tls/tls_client_cert.test.py index ca9e5a2..194cc94 100644 --- a/tests/gold_tests/tls/tls_client_cert.test.py +++ b/tests/gold_tests/tls/tls_client_cert.test.py @@ -301,7 +301,7 @@ tr4fail.Processes.Default.ReturnCode = 0 tr4fail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response") tr = Test.AddTestRun("Wait for the access log to write out") -tr.Processes.Default.StartBefore(server4, ready=When.FileExists(ts.Disk.squid_log)) +tr.Processes.Default.StartBefore(server4, ready=When.FileContains(ts.Disk.squid_log.Name, 'https', 12)) tr.StillRunningAfter = ts tr.Processes.Default.Command = 'echo "log file exists"' tr.Processes.Default.ReturnCode = 0
