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 a693beafea5371648f4def05af00fec63de189b0..c0a9a1814769ae973fb521b9c0d4ac85ca70cebb 100644 (file)
--- 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 b134f7e4bd0282915ab3f2087434193d2b71d131..5543355a8989aafc851cf8f3295471e18c759bdb 100644 (file)
--- 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 42ea0fdd24b1e6d83d2b1c429f31b897c2d9bb1c..a1577b91c27fda4c38ddba1558c4c4d373804a4b 100644 (file)
--- 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 c679835f72360c925f0c203040ae7c93f455bd55..90225e58d91bc249d6f3ef0573894c8eb58104d4 100644 (file)
--- 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 500ec04d729fb2fb1d378015668a6ba4f5539293..641ed300fd0d107e56b22e011cdd219440975f79 100644 (file)
--- 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 fec780d783a02dc117d1622df76d5a0bae4d0b0f..f2fbfcfd124deab401f8dd0c60ba14de8b2473df 100644 (file)
--- 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 f570ff7c3ee8bae5518eb58ca27bcea954c10652..4a84987832812ce498b4e846734ceca2ad938b4b 100644 (file)
--- 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 801bfa8ecffe4daaa2ed6d9deb4d740cff6a8bc8..32f58fa01781f194107cc7f73cf1ecc1ae28d5e8 100644 (file)
--- 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)