From: Amos Jeffries Date: Mon, 19 Sep 2016 01:29:59 +0000 (+1200) Subject: Cleanup: remove most Security::ContextPtr uses from libsecurity X-Git-Tag: SQUID_4_0_15~35 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=64769c7950d4f6f16ca0084c074693dc3fb7caad;p=thirdparty%2Fsquid.git Cleanup: remove most Security::ContextPtr uses from libsecurity Replace with Security::ContextPointer. The total change to remove Security::ContextPtr entirely is big and at times may require logic redesign. So doing this as smaller steps. --- diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc index a693beafea..c0a9a18147 100644 --- a/src/security/PeerOptions.cc +++ b/src/security/PeerOptions.cc @@ -215,36 +215,38 @@ Security::PeerOptions::updateTlsVersionLimits() } } -Security::ContextPtr +Security::ContextPointer Security::PeerOptions::createBlankContext() const { - Security::ContextPtr t = nullptr; - + Security::ContextPointer ctx; #if USE_OPENSSL Ssl::Initialize(); #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) - t = SSL_CTX_new(TLS_client_method()); + SSL_CTX *t = SSL_CTX_new(TLS_client_method()); #else - t = SSL_CTX_new(SSLv23_client_method()); + SSL_CTX *t = SSL_CTX_new(SSLv23_client_method()); #endif if (!t) { const auto x = ERR_error_string(ERR_get_error(), nullptr); fatalf("Failed to allocate TLS client context: %s\n", x); } + ctx.resetWithoutLocking(t); #elif USE_GNUTLS // Initialize for X.509 certificate exchange + gnutls_certificate_credentials_t t; if (const int x = gnutls_certificate_allocate_credentials(&t)) { fatalf("Failed to allocate TLS client context: error=%d\n", x); } + ctx.resetWithoutLocking(t); #else debugs(83, 1, "WARNING: Failed to allocate TLS client context: No TLS library"); #endif - return t; + return ctx; } Security::ContextPtr @@ -252,18 +254,18 @@ Security::PeerOptions::createClientContext(bool setOptions) { updateTlsVersionLimits(); - Security::ContextPtr t = createBlankContext(); + Security::ContextPointer t = createBlankContext(); if (t) { #if USE_OPENSSL // XXX: temporary performance regression. c_str() data copies and prevents this being a const method Ssl::InitClientContext(t, *this, (setOptions ? parsedOptions : 0), parsedFlags); #endif updateContextNpn(t); - updateContextCa(t); - updateContextCrl(t); + updateContextCa(t.get()); + updateContextCrl(t.get()); } - return t; + return t.release(); } /// set of options we can parse and what they map to @@ -554,13 +556,13 @@ ssl_next_proto_cb(SSL *s, unsigned char **out, unsigned char *outlen, const unsi #endif void -Security::PeerOptions::updateContextNpn(Security::ContextPtr &ctx) +Security::PeerOptions::updateContextNpn(Security::ContextPointer &ctx) { if (!flags.tlsNpn) return; #if USE_OPENSSL && defined(TLSEXT_TYPE_next_proto_neg) - SSL_CTX_set_next_proto_select_cb(ctx, &ssl_next_proto_cb, nullptr); + SSL_CTX_set_next_proto_select_cb(ctx.get(), &ssl_next_proto_cb, nullptr); #endif // NOTE: GnuTLS does not support the obsolete NPN extension. @@ -584,7 +586,7 @@ loadSystemTrustedCa(Security::ContextPtr &ctx) } void -Security::PeerOptions::updateContextCa(Security::ContextPtr &ctx) +Security::PeerOptions::updateContextCa(Security::ContextPtr ctx) { debugs(83, 8, "Setting CA certificate locations."); #if USE_OPENSSL @@ -612,7 +614,7 @@ Security::PeerOptions::updateContextCa(Security::ContextPtr &ctx) } void -Security::PeerOptions::updateContextCrl(Security::ContextPtr &ctx) +Security::PeerOptions::updateContextCrl(Security::ContextPtr ctx) { #if USE_OPENSSL bool verifyCrl = false; diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h index b134f7e4bd..5543355a89 100644 --- a/src/security/PeerOptions.h +++ b/src/security/PeerOptions.h @@ -33,7 +33,7 @@ public: virtual void clear() {*this = PeerOptions();} /// generate an unset security context object - virtual Security::ContextPtr createBlankContext() const; + virtual Security::ContextPointer createBlankContext() const; /// generate a security client-context from these configured options Security::ContextPtr createClientContext(bool setOptions); @@ -42,13 +42,13 @@ public: void updateTlsVersionLimits(); /// setup the NPN extension details for the given context - void updateContextNpn(Security::ContextPtr &); + void updateContextNpn(Security::ContextPointer &); /// setup the CA details for the given context - void updateContextCa(Security::ContextPtr &); + void updateContextCa(Security::ContextPtr); /// setup the CRL details for the given context - void updateContextCrl(Security::ContextPtr &); + void updateContextCrl(Security::ContextPtr); /// output squid.conf syntax with 'pfx' prefix on parameters for the stored settings virtual void dumpCfg(Packable *, const char *pfx) const; diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index 42ea0fdd24..a1577b91c2 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -85,36 +85,38 @@ Security::ServerOptions::dumpCfg(Packable *p, const char *pfx) const p->appendf(" %sdh=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(dh)); } -Security::ContextPtr +Security::ContextPointer Security::ServerOptions::createBlankContext() const { - Security::ContextPtr t = nullptr; - + Security::ContextPointer ctx; #if USE_OPENSSL Ssl::Initialize(); #if (OPENSSL_VERSION_NUMBER >= 0x10100000L) - t = SSL_CTX_new(TLS_server_method()); + SSL_CTX *t = SSL_CTX_new(TLS_server_method()); #else - t = SSL_CTX_new(SSLv23_server_method()); + SSL_CTX *t = SSL_CTX_new(SSLv23_server_method()); #endif if (!t) { const auto x = ERR_error_string(ERR_get_error(), nullptr); debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: " << x); } + ctx.resetWithoutLocking(t); #elif USE_GNUTLS // Initialize for X.509 certificate exchange + gnutls_certificate_credentials_t t; if (const int x = gnutls_certificate_allocate_credentials(&t)) { debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: error=" << x); } + ctx.resetWithoutLocking(t); #else debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: No TLS library"); #endif - return t; + return ctx; } bool diff --git a/src/security/ServerOptions.h b/src/security/ServerOptions.h index c679835f72..90225e58d9 100644 --- a/src/security/ServerOptions.h +++ b/src/security/ServerOptions.h @@ -29,7 +29,7 @@ public: /* Security::PeerOptions API */ virtual void parse(const char *); virtual void clear() {*this = ServerOptions();} - virtual Security::ContextPtr createBlankContext() const; + virtual Security::ContextPointer createBlankContext() const; virtual void dumpCfg(Packable *, const char *pfx) const; /// generate a security server-context from these configured options diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 500ec04d72..641ed300fd 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -622,22 +622,22 @@ Ssl::InitServerContext(const Security::ContextPointer &ctx, AnyP::PortCfg &port) } bool -Ssl::InitClientContext(Security::ContextPtr &sslContext, Security::PeerOptions &peer, long options, long fl) +Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &peer, long options, long fl) { - if (!sslContext) + if (!ctx) return false; - SSL_CTX_set_options(sslContext, options); + SSL_CTX_set_options(ctx.get(), options); #if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) - SSL_CTX_set_info_callback(sslContext, ssl_info_cb); + SSL_CTX_set_info_callback(ctx.get(), ssl_info_cb); #endif if (!peer.sslCipher.isEmpty()) { debugs(83, 5, "Using chiper suite " << peer.sslCipher << "."); const char *cipher = peer.sslCipher.c_str(); - if (!SSL_CTX_set_cipher_list(sslContext, cipher)) { + if (!SSL_CTX_set_cipher_list(ctx.get(), cipher)) { const int ssl_error = ERR_get_error(); fatalf("Failed to set SSL cipher suite '%s': %s\n", cipher, ERR_error_string(ssl_error, NULL)); @@ -651,7 +651,7 @@ Ssl::InitClientContext(Security::ContextPtr &sslContext, Security::PeerOptions & debugs(83, DBG_IMPORTANT, "Using certificate in " << keys.certFile); const char *certfile = keys.certFile.c_str(); - if (!SSL_CTX_use_certificate_chain_file(sslContext, certfile)) { + if (!SSL_CTX_use_certificate_chain_file(ctx.get(), certfile)) { const int ssl_error = ERR_get_error(); fatalf("Failed to acquire SSL certificate '%s': %s\n", certfile, ERR_error_string(ssl_error, NULL)); @@ -659,9 +659,9 @@ Ssl::InitClientContext(Security::ContextPtr &sslContext, Security::PeerOptions & debugs(83, DBG_IMPORTANT, "Using private key in " << keys.privateKeyFile); const char *keyfile = keys.privateKeyFile.c_str(); - ssl_ask_password(sslContext, keyfile); + ssl_ask_password(ctx.get(), keyfile); - if (!SSL_CTX_use_PrivateKey_file(sslContext, keyfile, SSL_FILETYPE_PEM)) { + if (!SSL_CTX_use_PrivateKey_file(ctx.get(), keyfile, SSL_FILETYPE_PEM)) { const int ssl_error = ERR_get_error(); fatalf("Failed to acquire SSL private key '%s': %s\n", keyfile, ERR_error_string(ssl_error, NULL)); @@ -669,7 +669,7 @@ Ssl::InitClientContext(Security::ContextPtr &sslContext, Security::PeerOptions & debugs(83, 5, "Comparing private and public SSL keys."); - if (!SSL_CTX_check_private_key(sslContext)) { + if (!SSL_CTX_check_private_key(ctx.get())) { const int ssl_error = ERR_get_error(); fatalf("SSL private key '%s' does not match public key '%s': %s\n", certfile, keyfile, ERR_error_string(ssl_error, NULL)); @@ -678,14 +678,14 @@ Ssl::InitClientContext(Security::ContextPtr &sslContext, Security::PeerOptions & } debugs(83, 9, "Setting RSA key generation callback."); - SSL_CTX_set_tmp_rsa_callback(sslContext, ssl_temp_rsa_cb); + SSL_CTX_set_tmp_rsa_callback(ctx.get(), ssl_temp_rsa_cb); if (fl & SSL_FLAG_DONT_VERIFY_PEER) { debugs(83, 2, "NOTICE: Peer certificates are not verified for validity!"); - SSL_CTX_set_verify(sslContext, SSL_VERIFY_NONE, NULL); + SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_NONE, NULL); } else { debugs(83, 9, "Setting certificate verification callback."); - SSL_CTX_set_verify(sslContext, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ssl_verify_cb); + SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ssl_verify_cb); } return true; diff --git a/src/ssl/support.h b/src/ssl/support.h index fec780d783..f2fbfcfd12 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -89,7 +89,7 @@ extern const char *SessionCacheName; bool InitServerContext(const Security::ContextPointer &, AnyP::PortCfg &); /// initialize a TLS client context with OpenSSL specific settings -bool InitClientContext(Security::ContextPtr &, Security::PeerOptions &, long options, long flags); +bool InitClientContext(Security::ContextPointer &, Security::PeerOptions &, long options, long flags); } //namespace Ssl diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index f570ff7c3e..4a84987832 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -71,9 +71,9 @@ Security::PeerOptions Security::ProxyOutgoingConfig; void Security::PeerOptions::parse(char const*) STUB Security::ContextPtr Security::PeerOptions::createClientContext(bool) STUB_RETVAL(NULL) void Security::PeerOptions::updateTlsVersionLimits() STUB -Security::ContextPtr Security::PeerOptions::createBlankContext() const STUB -void Security::PeerOptions::updateContextCa(Security::ContextPtr &) STUB -void Security::PeerOptions::updateContextCrl(Security::ContextPtr &) STUB +Security::ContextPointer Security::PeerOptions::createBlankContext() const STUB_RETVAL(Security::ContextPointer()) +void Security::PeerOptions::updateContextCa(Security::ContextPtr) STUB +void Security::PeerOptions::updateContextCrl(Security::ContextPtr) STUB void Security::PeerOptions::dumpCfg(Packable*, char const*) const STUB long Security::PeerOptions::parseOptions() STUB_RETVAL(0) long Security::PeerOptions::parseFlags() STUB_RETVAL(0) @@ -83,7 +83,7 @@ void parse_securePeerOptions(Security::PeerOptions *) STUB //Security::ServerOptions::ServerOptions(const Security::ServerOptions &) STUB void Security::ServerOptions::parse(const char *) STUB void Security::ServerOptions::dumpCfg(Packable *, const char *) const STUB -Security::ContextPtr Security::ServerOptions::createBlankContext() const STUB +Security::ContextPointer Security::ServerOptions::createBlankContext() const STUB_RETVAL(Security::ContextPointer()) bool Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) STUB_RETVAL(false) void Security::ServerOptions::updateContextEecdh(Security::ContextPtr &) STUB diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index 801bfa8ecf..32f58fa017 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -51,7 +51,7 @@ const String & Ssl::ErrorDetail::toString() const STUB_RETSTATREF(String) namespace Ssl { bool InitServerContext(const Security::ContextPointer &, AnyP::PortCfg &) STUB_RETVAL(false) -bool InitClientContext(Security::ContextPtr &, Security::PeerOptions &, long, const char *) STUB_RETVAL(false) +bool InitClientContext(Security::ContextPointer &, Security::PeerOptions &, long, const char *) STUB_RETVAL(false) } // namespace Ssl int ssl_read_method(int, char *, int) STUB_RETVAL(0) int ssl_write_method(int, const char *, int) STUB_RETVAL(0)