From e3dab5e24b460a876082ae99d24ccfa1de5771e1 Mon Sep 17 00:00:00 2001 From: Persia Aziz Date: Tue, 18 Oct 2016 14:53:59 -0500 Subject: [PATCH] TS-4978: illegal memory access with ticket_key.filename --- iocore/net/P_SSLConfig.h | 1 + iocore/net/SSLCertLookup.cc | 12 ++++---- iocore/net/SSLConfig.cc | 57 ++++++++++++++++--------------------- 3 files changed, 31 insertions(+), 39 deletions(-) diff --git a/iocore/net/P_SSLConfig.h b/iocore/net/P_SSLConfig.h index 6adc2b55606..621f5703918 100644 --- a/iocore/net/P_SSLConfig.h +++ b/iocore/net/P_SSLConfig.h @@ -115,6 +115,7 @@ struct SSLConfigParams : public ConfigInfo { void initialize(); void cleanup(); + void reset(); }; ///////////////////////////////////////////////////////////// diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc index b6547cdf9cd..1e4fb09d94d 100644 --- a/iocore/net/SSLCertLookup.cc +++ b/iocore/net/SSLCertLookup.cc @@ -171,7 +171,7 @@ ticket_block_alloc(unsigned count) ssl_ticket_key_block * ticket_block_create(char *ticket_key_data, int ticket_key_len) { - ssl_ticket_key_block *keyblock = NULL; + ssl_ticket_key_block *keyblock = nullptr; unsigned num_ticket_keys = ticket_key_len / sizeof(ssl_ticket_key_t); if (num_ticket_keys == 0) { Error("SSL session ticket key is too short (>= 48 bytes are required)"); @@ -195,7 +195,7 @@ ticket_block_create(char *ticket_key_data, int ticket_key_len) fail: ticket_block_free(keyblock); - return NULL; + return nullptr; } ssl_ticket_key_block * @@ -204,9 +204,9 @@ ssl_create_ticket_keyblock(const char *ticket_key_path) #if HAVE_OPENSSL_SESSION_TICKETS ats_scoped_str ticket_key_data; int ticket_key_len; - ssl_ticket_key_block *keyblock = NULL; + ssl_ticket_key_block *keyblock = nullptr; - if (ticket_key_path != NULL) { + if (ticket_key_path != nullptr) { ticket_key_data = readIntoBuffer(ticket_key_path, __func__, &ticket_key_len); if (!ticket_key_data) { Error("failed to read SSL session ticket key from %s", (const char *)ticket_key_path); @@ -224,11 +224,11 @@ ssl_create_ticket_keyblock(const char *ticket_key_path) fail: ticket_block_free(keyblock); - return NULL; + return nullptr; #else /* !HAVE_OPENSSL_SESSION_TICKETS */ (void)ticket_key_path; - return NULL; + return nullptr; #endif /* HAVE_OPENSSL_SESSION_TICKETS */ } void diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index 898fdbbbc36..bada78d8795 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -73,30 +73,27 @@ static ConfigUpdateHandler *sslCertUpdate; SSLConfigParams::SSLConfigParams() { - serverCertPathOnly = nullptr; - serverCertChainFilename = nullptr; - configFilePath = nullptr; - serverCACertFilename = nullptr; - serverCACertPath = nullptr; - clientCertPath = nullptr; - - clientKeyPath = nullptr; - clientCACertFilename = nullptr; - clientCACertPath = nullptr; - cipherSuite = nullptr; - client_cipherSuite = nullptr; - dhparamsFile = nullptr; - serverKeyPathOnly = nullptr; - - ticket_key_filename = nullptr; - default_global_keyblock = nullptr; + reset(); +} - clientCertLevel = client_verify_depth = verify_depth = clientVerify = 0; +SSLConfigParams::~SSLConfigParams() +{ + cleanup(); +} + +void +SSLConfigParams::reset() +{ + serverCertPathOnly = serverCertChainFilename = configFilePath = serverCACertFilename = serverCACertPath = clientCertPath = + clientKeyPath = clientCACertFilename = clientCACertPath = cipherSuite = client_cipherSuite = dhparamsFile = serverKeyPathOnly = + ticket_key_filename = nullptr; + default_global_keyblock = nullptr; - ssl_ctx_options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; - ssl_client_ctx_protocols = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; - ssl_session_cache = SSL_SESSION_CACHE_MODE_SERVER_ATS_IMPL; - ssl_session_cache_size = 1024 * 100; + clientCertLevel = client_verify_depth = verify_depth = clientVerify = 0; + ssl_ctx_options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; + ssl_client_ctx_protocols = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; + ssl_session_cache = SSL_SESSION_CACHE_MODE_SERVER_ATS_IMPL; + ssl_session_cache_size = 1024 * 100; ssl_session_cache_num_buckets = 1024; // Sessions per bucket is ceil(ssl_session_cache_size / ssl_session_cache_num_buckets) ssl_session_cache_skip_on_contention = 0; ssl_session_cache_timeout = 0; @@ -104,11 +101,6 @@ SSLConfigParams::SSLConfigParams() configExitOnLoadError = 0; } -SSLConfigParams::~SSLConfigParams() -{ - cleanup(); -} - void SSLConfigParams::cleanup() { @@ -128,9 +120,7 @@ SSLConfigParams::cleanup() ssl_wire_trace_ip = (IpAddr *)ats_free_null(ssl_wire_trace_ip); ticket_key_filename = (char *)ats_free_null(ticket_key_filename); ticket_block_free(default_global_keyblock); - default_global_keyblock = NULL; - - clientCertLevel = client_verify_depth = verify_depth = clientVerify = 0; + reset(); } /** set_paths_helper @@ -268,12 +258,13 @@ SSLConfigParams::initialize() ats_free(CACertRelativePath); #if HAVE_OPENSSL_SESSION_TICKETS - REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename"); - if (this->ticket_key_filename != NULL) { + + if (REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename") == REC_ERR_OKAY && + this->ticket_key_filename != nullptr) { ats_scoped_str ticket_key_path(Layout::relative_to(this->serverCertPathOnly, this->ticket_key_filename)); default_global_keyblock = ssl_create_ticket_keyblock(ticket_key_path); } else { - default_global_keyblock = ssl_create_ticket_keyblock(NULL); + default_global_keyblock = ssl_create_ticket_keyblock(nullptr); } #endif