From 69544aff86d56d288a8708c6aae34750fcd54b16 Mon Sep 17 00:00:00 2001 From: Persia Aziz Date: Mon, 23 Jan 2017 15:05:36 -0600 Subject: [PATCH] TS-5022: reduce memory allocation in clientcert lookup --- iocore/net/SSLConfig.cc | 12 +------ mgmt/RecordsConfig.cc | 4 +-- .../experimental/ts_lua/ts_lua_http_config.c | 1 - proxy/InkAPI.cc | 19 +++-------- proxy/InkAPITest.cc | 3 +- proxy/http/HttpConfig.cc | 6 ++-- proxy/http/HttpConfig.h | 6 ++-- proxy/http/HttpSM.cc | 32 ++++++++----------- 8 files changed, 26 insertions(+), 57 deletions(-) diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 4cd5f76246a..0533d84c68d 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -159,8 +159,6 @@ SSLConfigParams::initialize() char *serverCertRelativePath = nullptr; char *ssl_server_private_key_path = nullptr; char *CACertRelativePath = nullptr; - char *ssl_client_cert_filename = nullptr; - char *ssl_client_cert_path = nullptr; char *ssl_client_private_key_filename = nullptr; char *ssl_client_private_key_path = nullptr; char *clientCACertRelativePath = nullptr; @@ -303,15 +301,7 @@ SSLConfigParams::initialize() client_verify_depth = 7; REC_ReadConfigInt32(clientVerify, "proxy.config.ssl.client.verify.server"); - ssl_client_cert_filename = nullptr; - ssl_client_cert_path = nullptr; - REC_ReadConfigStringAlloc(ssl_client_cert_filename, "proxy.config.ssl.client.cert.filename"); - REC_ReadConfigStringAlloc(ssl_client_cert_path, "proxy.config.ssl.client.cert.path"); - if (ssl_client_cert_filename && ssl_client_cert_path) { - set_paths_helper(ssl_client_cert_path, ssl_client_cert_filename, nullptr, &clientCertPath); - } - ats_free_null(ssl_client_cert_filename); - ats_free_null(ssl_client_cert_path); + REC_ReadConfigStringAlloc(clientCertPath, "proxy.config.ssl.client.cert.file"); REC_ReadConfigStringAlloc(ssl_client_private_key_filename, "proxy.config.ssl.client.private_key.filename"); REC_ReadConfigStringAlloc(ssl_client_private_key_path, "proxy.config.ssl.client.private_key.path"); diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index acae4816cc2..b3968c14d8e 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1259,9 +1259,7 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.ssl.client.verify.server", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} , - {RECT_CONFIG, "proxy.config.ssl.client.cert.filename", RECD_STRING, nullptr, RECU_RESTART_TS, RR_NULL, RECC_STR, "^[^[:space:]]*$", RECA_NULL} - , - {RECT_CONFIG, "proxy.config.ssl.client.cert.path", RECD_STRING, TS_BUILD_SYSCONFDIR, RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL} + {RECT_CONFIG, "proxy.config.ssl.client.cert.file", RECD_STRING, nullptr, RECU_RESTART_TS, RR_NULL, RECC_STR, "^[^[:space:]]*$", RECA_NULL} , {RECT_CONFIG, "proxy.config.ssl.client.private_key.filename", RECD_STRING, nullptr, RECU_RESTART_TS, RR_NULL, RECC_STR, "^[^[:space:]]*$", RECA_NULL} , diff --git a/plugins/experimental/ts_lua/ts_lua_http_config.c b/plugins/experimental/ts_lua/ts_lua_http_config.c index de0ca0efa15..e321274066c 100644 --- a/plugins/experimental/ts_lua/ts_lua_http_config.c +++ b/plugins/experimental/ts_lua/ts_lua_http_config.c @@ -122,7 +122,6 @@ typedef enum { TS_LUA_CONFIG_SRV_ENABLED = TS_CONFIG_SRV_ENABLED, TS_LUA_CONFIG_HTTP_FORWARD_CONNECT_METHOD = TS_CONFIG_HTTP_FORWARD_CONNECT_METHOD, TS_LUA_CONFIG_SSL_CERT_FILENAME = TS_CONFIG_SSL_CERT_FILENAME, - TS_LUA_CONFIG_SSL_CERT_FILEPATH = TS_CONFIG_SSL_CERT_FILEPATH, } TSLuaOverridableConfigKey; typedef enum { diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc index e33c2d5a70e..7d8b29734d3 100644 --- a/proxy/InkAPI.cc +++ b/proxy/InkAPI.cc @@ -8143,11 +8143,7 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr break; case TS_CONFIG_SSL_CERT_FILENAME: typ = OVERRIDABLE_TYPE_STRING; - ret = &overridableHttpConfig->client_cert_filename; - break; - case TS_CONFIG_SSL_CERT_FILEPATH: - typ = OVERRIDABLE_TYPE_STRING; - ret = &overridableHttpConfig->client_cert_filepath; + ret = &overridableHttpConfig->client_cert_file; break; // This helps avoiding compiler warnings, yet detect unhandled enum members. case TS_CONFIG_NULL: @@ -8310,13 +8306,9 @@ TSHttpTxnConfigStringSet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char break; case TS_CONFIG_SSL_CERT_FILENAME: if (value && length > 0) { - s->t_state.txn_conf->client_cert_filename = const_cast(value); + s->t_state.txn_conf->client_cert_file = const_cast(value); } break; - case TS_CONFIG_SSL_CERT_FILEPATH: - if (value && length > 0) { - s->t_state.txn_conf->client_cert_filepath = const_cast(value); - } default: return TS_ERROR; break; @@ -8396,8 +8388,8 @@ TSHttpTxnConfigFind(const char *name, int length, TSOverridableConfigKey *conf, case 33: if (!strncmp(name, "proxy.config.http.cache.fuzz.time", length)) { cnf = TS_CONFIG_HTTP_CACHE_FUZZ_TIME; - } else if (!strncmp(name, "proxy.config.ssl.client.cert.path", length)) { - cnf = TS_CONFIG_SSL_CERT_FILEPATH; + } else if (!strncmp(name, "proxy.config.ssl.client.cert.file", length)) { + cnf = TS_CONFIG_SSL_CERT_FILENAME; typ = TS_RECORDDATATYPE_STRING; } break; @@ -8461,9 +8453,6 @@ TSHttpTxnConfigFind(const char *name, int length, TSOverridableConfigKey *conf, cnf = TS_CONFIG_HTTP_CACHE_FUZZ_MIN_TIME; } else if (!strncmp(name, "proxy.config.http.default_buffer_size", length)) { cnf = TS_CONFIG_HTTP_DEFAULT_BUFFER_SIZE; - } else if (!strncmp(name, "proxy.config.ssl.client.cert.filename", length)) { - cnf = TS_CONFIG_SSL_CERT_FILENAME; - typ = TS_RECORDDATATYPE_STRING; } break; diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc index 5f0120fe820..4f539f30837 100644 --- a/proxy/InkAPITest.cc +++ b/proxy/InkAPITest.cc @@ -7618,8 +7618,7 @@ const char *SDK_Overridable_Configs[TS_CONFIG_LAST_ENTRY] = { "proxy.config.http.transaction_active_timeout_in", "proxy.config.srv_enabled", "proxy.config.http.forward_connect_method", - "proxy.config.ssl.client.cert.filename", - "proxy.config.ssl.client.cert.path", + "proxy.config.ssl.client.cert.file", }; REGRESSION_TEST(SDK_API_OVERRIDABLE_CONFIGS)(RegressionTest *test, int /* atype ATS_UNUSED */, int *pstatus) diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index 9c69e0afeda..8c51b77bb5a 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -915,8 +915,7 @@ HttpConfig::startup() HttpEstablishStaticConfigByte(c.oride.insert_response_via_string, "proxy.config.http.insert_response_via_str"); HttpEstablishStaticConfigLongLong(c.oride.proxy_response_hsts_max_age, "proxy.config.ssl.hsts_max_age"); HttpEstablishStaticConfigByte(c.oride.proxy_response_hsts_include_subdomains, "proxy.config.ssl.hsts_include_subdomains"); - HttpEstablishStaticConfigStringAlloc(c.oride.client_cert_filename, "proxy.config.ssl.client.cert.filename"); - HttpEstablishStaticConfigStringAlloc(c.oride.client_cert_filepath, "proxy.config.ssl.client.cert.path"); + HttpEstablishStaticConfigStringAlloc(c.oride.client_cert_file, "proxy.config.ssl.client.cert.file"); HttpEstablishStaticConfigStringAlloc(c.proxy_request_via_string, "proxy.config.http.request_via_str"); c.proxy_request_via_string_len = -1; @@ -1404,8 +1403,7 @@ HttpConfig::reconfigure() params->redirection_host_no_port = INT_TO_BOOL(m_master.redirection_host_no_port); params->oride.number_of_redirections = m_master.oride.number_of_redirections; params->post_copy_size = m_master.post_copy_size; - params->oride.client_cert_filename = ats_strdup(m_master.oride.client_cert_filename); - params->oride.client_cert_filepath = ats_strdup(m_master.oride.client_cert_filepath); + params->oride.client_cert_file = ats_strdup(m_master.oride.client_cert_file); // Local Manager params->synthetic_port = m_master.synthetic_port; diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index cc75710384f..785ddea1c33 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -470,8 +470,7 @@ struct OverridableHttpConfigParams { cache_heuristic_lm_factor(0.10), freshness_fuzz_prob(0.005), background_fill_threshold(0.5), - client_cert_filename(NULL), - client_cert_filepath(NULL) + client_cert_file(NULL) { } @@ -683,7 +682,7 @@ struct OverridableHttpConfigParams { MgmtFloat cache_heuristic_lm_factor; MgmtFloat freshness_fuzz_prob; MgmtFloat background_fill_threshold; - char *client_cert_filename; + char *client_cert_file; char *client_cert_filepath; }; @@ -931,6 +930,7 @@ inline HttpConfigParams::~HttpConfigParams() ats_free(oride.body_factory_template_base); ats_free(oride.proxy_response_server_string); ats_free(oride.global_user_agent_header); + ats_free(oride.client_cert_file); ats_free(cache_vary_default_text); ats_free(cache_vary_default_images); ats_free(cache_vary_default_other); diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 6d611b93f63..b0615cddb36 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -4019,7 +4019,7 @@ HttpSM::do_remap_request(bool run_inline) { DebugSM("http_seq", "[HttpSM::do_remap_request] Remapping request"); DebugSM("url_rewrite", "Starting a possible remapping for request [%" PRId64 "]", sm_id); - SSLConfig::scoped_config params; + bool ret = false; if (t_state.cop_test_page == false) { ret = remapProcessor.setup_for_remap(&t_state); @@ -4060,20 +4060,6 @@ HttpSM::do_remap_request(bool run_inline) pending_action = remap_action_handle; } - // check if the overridden client cert filename is already attached to an existing ssl context - if (t_state.txn_conf->client_cert_filepath && t_state.txn_conf->client_cert_filename) { - ats_scoped_str clientCert(Layout::relative_to(t_state.txn_conf->client_cert_filepath, t_state.txn_conf->client_cert_filename)); - if (clientCert != nullptr) { - auto tCTX = params->getCTX(clientCert); - - if (tCTX == nullptr) { - // make new client ctx and add it to the ctx list - auto tctx = params->getNewCTX(clientCert); - params->InsertCTX(clientCert, tctx); - } - } - } - return; } @@ -5051,9 +5037,19 @@ HttpSM::do_http_server_open(bool raw) opt.set_sni_servername(host, len); } - ats_scoped_str clientCert( - (Layout::relative_to(t_state.txn_conf->client_cert_filepath, t_state.txn_conf->client_cert_filename))); - opt.set_client_certname(clientCert); + SSLConfig::scoped_config params; + // check if the overridden client cert filename is already attached to an existing ssl context + if (t_state.txn_conf->client_cert_file != nullptr) { + opt.set_client_certname(t_state.txn_conf->client_cert_file); + auto tCTX = params->getCTX(t_state.txn_conf->client_cert_file); + + if (tCTX == nullptr) { + // make new client ctx and add it to the ctx list + auto tctx = params->getNewCTX(t_state.txn_conf->client_cert_file); + params->InsertCTX(t_state.txn_conf->client_cert_file, tctx); + } + } + connect_action_handle = sslNetProcessor.connect_re(this, // state machine &t_state.current.server->dst_addr.sa, // addr + port &opt);