From 05431f8aa7c0612cbc61eb1bc459b3d366390f45 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Wed, 24 Jun 2020 13:54:50 +0900 Subject: [PATCH] Remove dup code in QUICMultiCertConfigLoader The only difference between SSLMultiCertConfigLoader::_store_ssl_ctx and QUICMultiCertConfigLoader::_store_ssl_ctx was whether it says "QUIC" on log outputs. This commit removes the long dup code and introduces _debug_tag() so that derived classes can override the debug tag. --- iocore/net/P_SSLUtils.h | 3 +- iocore/net/QUICMultiCertConfigLoader.cc | 63 +++---------------------- iocore/net/QUICMultiCertConfigLoader.h | 2 +- iocore/net/SSLUtils.cc | 10 +++- 4 files changed, 17 insertions(+), 61 deletions(-) diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index ac43cf2d309..5f0e40250f2 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -89,7 +89,8 @@ class SSLMultiCertConfigLoader std::set &names); private: - virtual bool _store_ssl_ctx(SSLCertLookup *lookup, shared_SSLMultiCertConfigParams ssl_multi_cert_params); + virtual const char *_debug_tag() const; + bool _store_ssl_ctx(SSLCertLookup *lookup, shared_SSLMultiCertConfigParams ssl_multi_cert_params); virtual void _set_handshake_callbacks(SSL_CTX *ctx); }; diff --git a/iocore/net/QUICMultiCertConfigLoader.cc b/iocore/net/QUICMultiCertConfigLoader.cc index cf9c74f41e6..e2285f25f3b 100644 --- a/iocore/net/QUICMultiCertConfigLoader.cc +++ b/iocore/net/QUICMultiCertConfigLoader.cc @@ -164,63 +164,6 @@ QUICMultiCertConfigLoader::init_server_ssl_ctx(SSLMultiCertConfigLoader::CertLoa return nullptr; } -bool -QUICMultiCertConfigLoader::_store_ssl_ctx(SSLCertLookup *lookup, const shared_SSLMultiCertConfigParams multi_cert_params) -{ - bool retval = true; - std::vector cert_list; - SSLMultiCertConfigLoader::CertLoadData data; - std::set common_names; - std::unordered_map> unique_names; - const SSLConfigParams *params = this->_params; - this->load_certs_and_cross_reference_names(cert_list, data, params, multi_cert_params.get(), common_names, unique_names); - - for (size_t i = 0; i < cert_list.size(); i++) { - const char *current_cert_name = data.cert_names_list[i].c_str(); - if (0 > SSLMultiCertConfigLoader::check_server_cert_now(cert_list[i], current_cert_name)) { - /* At this point, we know cert is bad, and we've already printed a - descriptive reason as to why cert is bad to the log file */ - QUICConfDebug("Marking certificate as NOT VALID: %s", current_cert_name); - lookup->is_valid = false; - } - } - - shared_SSL_CTX ctx(this->init_server_ssl_ctx(data, multi_cert_params.get(), common_names), SSL_CTX_free); - - shared_ssl_ticket_key_block keyblock = nullptr; - - if (!ctx || !multi_cert_params || !this->_store_single_ssl_ctx(lookup, multi_cert_params, ctx, common_names)) { - retval = false; - std::string names; - for (auto name : data.cert_names_list) { - names.append(name); - names.append(" "); - } - Warning("QUIC: Failed to insert SSL_CTX for certificate %s entries for names already made", names.c_str()); - } - - for (auto iter = unique_names.begin(); retval && iter != unique_names.end(); ++iter) { - size_t i = iter->first; - - SSLMultiCertConfigLoader::CertLoadData single_data; - single_data.cert_names_list.push_back(data.cert_names_list[i]); - single_data.key_list.push_back(i < data.key_list.size() ? data.key_list[i] : ""); - single_data.ca_list.push_back(i < data.ca_list.size() ? data.ca_list[i] : ""); - single_data.ocsp_list.push_back(i < data.ocsp_list.size() ? data.ocsp_list[i] : ""); - - shared_SSL_CTX unique_ctx(this->init_server_ssl_ctx(single_data, multi_cert_params.get(), iter->second), SSL_CTX_free); - if (!unique_ctx || !this->_store_single_ssl_ctx(lookup, multi_cert_params, unique_ctx, iter->second)) { - retval = false; - } - } - - for (auto &i : cert_list) { - X509_free(i); - } - - return retval; -} - void QUICMultiCertConfigLoader::_set_handshake_callbacks(SSL_CTX *ssl_ctx) { @@ -303,3 +246,9 @@ QUICMultiCertConfigLoader::ssl_cert_cb(SSL *ssl, void * /*arg*/) return 1; } + +const char * +QUICMultiCertConfigLoader::_debug_tag() const +{ + return "quic"; +} diff --git a/iocore/net/QUICMultiCertConfigLoader.h b/iocore/net/QUICMultiCertConfigLoader.h index 796c083e161..01f8e0bebdd 100644 --- a/iocore/net/QUICMultiCertConfigLoader.h +++ b/iocore/net/QUICMultiCertConfigLoader.h @@ -51,7 +51,7 @@ class QUICMultiCertConfigLoader : public SSLMultiCertConfigLoader const SSLMultiCertConfigParams *sslMultCertSettings, std::set &names) override; private: - bool _store_ssl_ctx(SSLCertLookup *lookup, shared_SSLMultiCertConfigParams ssl_multi_cert_params) override; + const char *_debug_tag() const override; virtual void _set_handshake_callbacks(SSL_CTX *ssl_ctx) override; static int ssl_select_next_protocol(SSL *ssl, const unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned inlen, void *); diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 56fae1d4db8..e8519ab6549 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -1406,7 +1406,7 @@ SSLMultiCertConfigLoader::_store_ssl_ctx(SSLCertLookup *lookup, const shared_SSL if (0 > SSLMultiCertConfigLoader::check_server_cert_now(cert, current_cert_name)) { /* At this point, we know cert is bad, and we've already printed a descriptive reason as to why cert is bad to the log file */ - Debug("ssl", "Marking certificate as NOT VALID: %s", current_cert_name); + Debug(this->_debug_tag(), "Marking certificate as NOT VALID: %s", current_cert_name); lookup->is_valid = false; } i++; @@ -1421,7 +1421,7 @@ SSLMultiCertConfigLoader::_store_ssl_ctx(SSLCertLookup *lookup, const shared_SSL names.append(name); names.append(" "); } - Warning("Failed to insert SSL_CTX for certificate %s entries for names already made", names.c_str()); + Warning("(%s) Failed to insert SSL_CTX for certificate %s entries for names already made", this->_debug_tag(), names.c_str()); } for (auto iter = unique_names.begin(); retval && iter != unique_names.end(); ++iter) { @@ -2238,6 +2238,12 @@ SSLMultiCertConfigLoader::set_session_id_context(SSL_CTX *ctx, const SSLConfigPa return result; } +const char * +SSLMultiCertConfigLoader::_debug_tag() const +{ + return "ssl"; +} + /** Clear password in SSL_CTX @static