Merged from trunk rev.14625

diff --git a/src/base/TidyPointer.h b/src/base/TidyPointer.h
index 50b21e2a09760fd304c196243f370e1f7c4ac9a8..7d2e1e3ab1b9f914d66e859bf6dfdea3454eb178 100644 (file)
--- a/src/base/TidyPointer.h
+++ b/src/base/TidyPointer.h
@@ -29,7 +29,7 @@ public:
     T *get() const { return raw; }
 
     /// Reset raw pointer - delete last one and save new one.
-    void reset(T *t) {
+    virtual void reset(T *t) {
         deletePointer();
         raw = t;
     }
@@ -41,7 +41,7 @@ public:
         return ret;
     }
     /// Deallocate raw pointer.
-    ~TidyPointer() {
+    virtual ~TidyPointer() {
         deletePointer();
     }
 private:

diff --git a/src/client_side.cc b/src/client_side.cc
index 165ae8607f4bf313a419c7c842cf4f59950a4f28..6b4ba7a774dfbdf1eefe0edb331daa499d140078 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2885,9 +2885,9 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
     // fake certificate adaptation requires bump-server-first mode
     if (!sslServerBump) {
         assert(port->signingCert.get());
-        certProperties.signWithX509.resetAndLock(port->signingCert.get());
+        certProperties.signWithX509.reset(port->signingCert.get());
         if (port->signPkey.get())
-            certProperties.signWithPkey.resetAndLock(port->signPkey.get());
+            certProperties.signWithPkey.reset(port->signPkey.get());
         certProperties.signAlgorithm = Ssl::algSignTrusted;
         return;
     }
@@ -2899,7 +2899,7 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
     assert(sslServerBump->entry);
     if (sslServerBump->entry->isEmpty()) {
         if (X509 *mimicCert = sslServerBump->serverCert.get())
-            certProperties.mimicCert.resetAndLock(mimicCert);
+            certProperties.mimicCert.reset(mimicCert);
 
         ACLFilledChecklist checklist(NULL, sslServerBump->request.getRaw(),
                                      clientConnection != NULL ? clientConnection->rfc931 : dash_str);
@@ -2952,14 +2952,14 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
 
     if (certProperties.signAlgorithm == Ssl::algSignUntrusted) {
         assert(port->untrustedSigningCert.get());
-        certProperties.signWithX509.resetAndLock(port->untrustedSigningCert.get());
-        certProperties.signWithPkey.resetAndLock(port->untrustedSignPkey.get());
+        certProperties.signWithX509.reset(port->untrustedSigningCert.get());
+        certProperties.signWithPkey.reset(port->untrustedSignPkey.get());
     } else {
         assert(port->signingCert.get());
-        certProperties.signWithX509.resetAndLock(port->signingCert.get());
+        certProperties.signWithX509.reset(port->signingCert.get());
 
         if (port->signPkey.get())
-            certProperties.signWithPkey.resetAndLock(port->signPkey.get());
+            certProperties.signWithPkey.reset(port->signPkey.get());
     }
     signAlgorithm = certProperties.signAlgorithm;
 

diff --git a/src/security/LockingPointer.h b/src/security/LockingPointer.h
index 837012f356c96cf6a1f9c82507e31ef1c86153e9..48de7f728272d95b7b72cb39c93d5e2d6749da92 100644 (file)
--- a/src/security/LockingPointer.h
+++ b/src/security/LockingPointer.h
@@ -12,6 +12,7 @@
 #include "base/TidyPointer.h"
 
 #if USE_OPENSSL
+
 #if HAVE_OPENSSL_CRYPTO_H
 #include <openssl/crypto.h>
 #endif
@@ -24,6 +25,11 @@
             sk_object ## _pop_free(a, freefunction); \
         }
 
+#else // !USE_OPENSSL
+
+#include "base/Lock.h"
+#include <unordered_map>
+
 #endif
 
 // Macro to be used to define the C++ equivalent function of an extern "C"
@@ -46,41 +52,61 @@ public:
     typedef TidyPointer<T, DeAllocator> Parent;
     typedef LockingPointer<T, DeAllocator, lock> SelfType;
 
-    explicit LockingPointer(T *t = nullptr): Parent(t) {}
+    explicit LockingPointer(T *t = nullptr): Parent() {reset(t);}
+
+    virtual ~LockingPointer() { Parent::reset(nullptr); }
 
     explicit LockingPointer(const SelfType &o): Parent() {
-        resetAndLock(o.get());
+        reset(o.get());
     }
 
     SelfType &operator =(const SelfType & o) {
-        resetAndLock(o.get());
+        reset(o.get());
         return *this;
     }
 
-#if __cplusplus >= 201103L
-    explicit LockingPointer(LockingPointer<T, DeAllocator, lock> &&o): Parent(o.release()) {
-    }
+    explicit LockingPointer(LockingPointer<T, DeAllocator, lock> &&o): Parent(o.release()) {}
 
     LockingPointer<T, DeAllocator, lock> &operator =(LockingPointer<T, DeAllocator, lock> &&o) {
         if (o.get() != this->get())
             this->reset(o.release());
         return *this;
     }
+
+    virtual void reset(T *t) {
+        if (t == this->get())
+            return;
+
+#if !USE_OPENSSL
+        // OpenSSL maintains the reference locks through calls to Deallocator
+        // our manual locking does not have that luxury
+        if (this->get()) {
+            if (SelfType::Locks().at(this->get()).unlock())
+                SelfType::Locks().erase(this->get());
+        }
 #endif
+        Parent::reset(t);
 
-    void resetAndLock(T *t) {
-        if (t != this->get()) {
-            this->reset(t);
+        if (t) {
 #if USE_OPENSSL
-            if (t)
-                CRYPTO_add(&t->references, 1, lock);
-#elif USE_GNUTLS
-            // XXX: GnuTLS does not provide locking ?
+            CRYPTO_add(&t->references, 1, lock);
 #else
-            assert(false);
+            SelfType::Locks()[t].lock(); // find/create and lock
 #endif
         }
     }
+
+private:
+#if !USE_OPENSSL
+    // since we can never be sure if a raw-* passed to us is already being
+    // lock counted by another LockingPointer<> and the types pointed to are
+    // defined by third-party libraries we have to maintain the locks in a
+    // type-specific static external to both the Pointer and base classes.
+    static std::unordered_map<T*, Lock> & Locks() {
+        static std::unordered_map<T*, Lock> Instance;
+        return Instance;
+    }
+#endif
 };
 
 } // namespace Security

diff --git a/src/security/Makefile.am b/src/security/Makefile.am
index 7e8dc50d4ceddf2a9d2e672d7617db609af30231..0db5e90ac3a005c53bd38db0fbbb83582a04670f 100644 (file)
--- a/src/security/Makefile.am
+++ b/src/security/Makefile.am
@@ -26,4 +26,5 @@ libsecurity_la_SOURCES= \
        ServerOptions.cc \
        ServerOptions.h \
        Session.cc \
-       Session.h
+       Session.h \
+       support.cc

diff --git a/src/security/forward.h b/src/security/forward.h
index 54d6b06fb34fccf967cdc2ace88a7fd46dce1110..382f2c96165d4cdcc977c8c370a1d3b6d3884cdb 100644 (file)
--- a/src/security/forward.h
+++ b/src/security/forward.h
@@ -43,7 +43,7 @@ typedef Security::LockingPointer<X509, X509_free_cpp, CRYPTO_LOCK_X509> CertPoin
 CtoCpp1(gnutls_x509_crt_deinit, gnutls_x509_crt_t)
 typedef Security::LockingPointer<struct gnutls_x509_crt_int, gnutls_x509_crt_deinit, -1> CertPointer;
 #else
-typedef void * CertPointer;
+typedef Security::LockingPointer<void, nullptr, -1> CertPointer;
 #endif
 
 #if USE_OPENSSL
@@ -53,7 +53,7 @@ typedef LockingPointer<X509_CRL, X509_CRL_free_cpp, CRYPTO_LOCK_X509_CRL> CrlPoi
 CtoCpp1(gnutls_x509_crl_deinit, gnutls_x509_crl_t)
 typedef Security::LockingPointer<struct gnutls_x509_crl_int, gnutls_x509_crl_deinit, -1> CrlPointer;
 #else
-typedef void *CrlPointer;
+typedef Security::LockingPointer<void, nullptr, -1> CrlPointer;
 #endif
 
 typedef std::list<Security::CrlPointer> CertRevokeList;
@@ -62,7 +62,7 @@ typedef std::list<Security::CrlPointer> CertRevokeList;
 CtoCpp1(DH_free, DH *);
 typedef Security::LockingPointer<DH, DH_free_cpp, CRYPTO_LOCK_DH> DhePointer;
 #else
-typedef void *DhePointer;
+typedef Security::LockingPointer<void, nullptr, -1> DhePointer;
 #endif
 
 class KeyData;

diff --git a/src/security/support.cc b/src/security/support.cc
new file mode 100644 (file)
index 0000000..0e376d1
--- /dev/null
+++ b/src/security/support.cc
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+#include "security/forward.h"
+
+#if !USE_OPENSSL
+
+namespace Security
+{
+
+#if USE_GNUTLS
+template std::unordered_map<Security::ContextPtr, Lock> & ContextPointer::Locks();
+
+template std::unordered_map<gnutls_x509_crt_t, Lock> & CertPointer::Locks();
+
+template std::unordered_map<gnutls_x509_crl_t, Lock> & CrlPointer::Locks();
+#endif
+
+template std::unordered_map<void*, Lock> & DhePointer::Locks();
+
+} // namespace Security
+
+#endif /* !USE_OPENSSL */
+

diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc
index b6ce27ff9222e2eb8f7cd9144d409789eda36d5c..ec3ff474618afb238f1d137dc4a108e45c168f2a 100644 (file)
--- a/src/ssl/ErrorDetail.cc
+++ b/src/ssl/ErrorDetail.cc
@@ -628,12 +628,12 @@ const String &Ssl::ErrorDetail::toString() const
 Ssl::ErrorDetail::ErrorDetail( Ssl::ssl_error_t err_no, X509 *cert, X509 *broken, const char *aReason): error_no (err_no), lib_error_no(SSL_ERROR_NONE), errReason(aReason)
 {
     if (cert)
-        peer_cert.resetAndLock(cert);
+        peer_cert.reset(cert);
 
     if (broken)
-        broken_cert.resetAndLock(broken);
+        broken_cert.reset(broken);
     else
-        broken_cert.resetAndLock(cert);
+        broken_cert.reset(cert);
 
     detailEntry.error_no = SSL_ERROR_NONE;
 }
@@ -644,11 +644,11 @@ Ssl::ErrorDetail::ErrorDetail(Ssl::ErrorDetail const &anErrDetail)
     request = anErrDetail.request;
 
     if (anErrDetail.peer_cert.get()) {
-        peer_cert.resetAndLock(anErrDetail.peer_cert.get());
+        peer_cert.reset(anErrDetail.peer_cert.get());
     }
 
     if (anErrDetail.broken_cert.get()) {
-        broken_cert.resetAndLock(anErrDetail.broken_cert.get());
+        broken_cert.reset(anErrDetail.broken_cert.get());
     }
 
     detailEntry = anErrDetail.detailEntry;

diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index 88ae4ed4d407b6783f945c6dd3f3c38faf16165e..3e359a1aba3dbeeb8a690d8d0675de91b13fe49b 100644 (file)
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -232,7 +232,7 @@ Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
         if (!serverBump->serverCert.get()) {
             // remember the server certificate from the ErrorDetail object
             if (error && error->detail && error->detail->peerCert())
-                serverBump->serverCert.resetAndLock(error->detail->peerCert());
+                serverBump->serverCert.reset(error->detail->peerCert());
             else {
                 handleServerCertificate();
             }
@@ -358,7 +358,7 @@ Ssl::PeekingPeerConnector::serverCertificateVerified()
     if (ConnStateData *csd = request->clientConnectionManager.valid()) {
         Security::CertPointer serverCert;
         if(Ssl::ServerBump *serverBump = csd->serverBump())
-            serverCert.resetAndLock(serverBump->serverCert.get());
+            serverCert.reset(serverBump->serverCert.get());
         else {
             const int fd = serverConnection()->fd;
             Security::SessionPtr ssl = fd_table[fd].ssl.get();

diff --git a/src/ssl/ServerBump.cc b/src/ssl/ServerBump.cc
index 7d7f06ba892310e4b311f3f3560afdb70efcdc23..0dfe1c15d82443d0915367f49d87f98c980f9f7b 100644 (file)
--- a/src/ssl/ServerBump.cc
+++ b/src/ssl/ServerBump.cc
@@ -57,7 +57,7 @@ Ssl::ServerBump::attachServerSSL(SSL *ssl)
     if (serverSSL.get())
         return;
 
-    serverSSL.resetAndLock(ssl);
+    serverSSL.reset(ssl);
 }
 
 const Ssl::CertErrors *

diff --git a/src/ssl/cert_validate_message.cc b/src/ssl/cert_validate_message.cc
index 7c2249ed617cb9a5261af21a8c7ab15ee7d612c8..409283309f67fc5be06697bd8e65680ec8bf447a 100644 (file)
--- a/src/ssl/cert_validate_message.cc
+++ b/src/ssl/cert_validate_message.cc
@@ -216,7 +216,7 @@ Ssl::CertValidationResponse::RecvdError & Ssl::CertValidationResponse::RecvdErro
 void
 Ssl::CertValidationResponse::RecvdError::setCert(X509 *aCert)
 {
-    cert.resetAndLock(aCert);
+    cert.reset(aCert);
 }
 
 Ssl::CertValidationMsg::CertItem::CertItem(const CertItem &old)
@@ -235,7 +235,7 @@ Ssl::CertValidationMsg::CertItem & Ssl::CertValidationMsg::CertItem::operator =
 void
 Ssl::CertValidationMsg::CertItem::setCert(X509 *aCert)
 {
-    cert.resetAndLock(aCert);
+    cert.reset(aCert);
 }
 
 const std::string Ssl::CertValidationMsg::code_cert_validate("cert_validate");

diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc
index 218167b3f5a0051581592445dc2e488017b22948..bc6dcb3c60df3a7b996c96802c4d4fce26b8e0fc 100644 (file)
--- a/src/ssl/gadgets.cc
+++ b/src/ssl/gadgets.cc
@@ -424,7 +424,7 @@ static bool generateFakeSslCertificate(Security::CertPointer & certToStore, Ssl:
     Ssl::EVP_PKEY_Pointer pkey;
     // Use signing certificates private key as generated certificate private key
     if (properties.signWithPkey.get())
-        pkey.resetAndLock(properties.signWithPkey.get());
+        pkey.reset(properties.signWithPkey.get());
     else // if not exist generate one
         pkey.reset(Ssl::createSslPrivateKey());
 

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 611c412d33063e41014b46f92669d1339be29e34..8a59478f01383825f26f4b25954245a78a2ae74b 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -306,7 +306,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
                 ACLFilledChecklist *filledCheck = Filled(check);
                 assert(!filledCheck->sslErrors);
                 filledCheck->sslErrors = new Ssl::CertErrors(Ssl::CertError(error_no, broken_cert));
-                filledCheck->serverCert.resetAndLock(peer_cert);
+                filledCheck->serverCert.reset(peer_cert);
                 if (check->fastCheck() == ACCESS_ALLOWED) {
                     debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
                     ok = 1;
@@ -1277,8 +1277,8 @@ bool Ssl::generateUntrustedCert(Security::CertPointer &untrustedCert, EVP_PKEY_P
     // O, OU, and other CA subject fields will be mimicked
     // Expiration date and other common properties will be mimicked
     certProperties.signAlgorithm = Ssl::algSignSelf;
-    certProperties.signWithPkey.resetAndLock(pkey.get());
-    certProperties.mimicCert.resetAndLock(cert.get());
+    certProperties.signWithPkey.reset(pkey.get());
+    certProperties.mimicCert.reset(cert.get());
     return Ssl::generateSslCertificate(untrustedCert, untrustedPkey, certProperties);
 }
 
@@ -1330,19 +1330,19 @@ Ssl::CreateServer(Security::ContextPtr sslContext, const int fd, const char *squ
 
 Ssl::CertError::CertError(ssl_error_t anErr, X509 *aCert, int aDepth): code(anErr), depth(aDepth)
 {
-    cert.resetAndLock(aCert);
+    cert.reset(aCert);
 }
 
 Ssl::CertError::CertError(CertError const &err): code(err.code), depth(err.depth)
 {
-    cert.resetAndLock(err.cert.get());
+    cert.reset(err.cert.get());
 }
 
 Ssl::CertError &
 Ssl::CertError::operator = (const CertError &old)
 {
     code = old.code;
-    cert.resetAndLock(old.cert.get());
+    cert.reset(old.cert.get());
     return *this;
 }