certificateMatchesProperties segfault

It is reported at least one case where squid crashed with segfault because the
signing certificate was NULL.

This patch:
  - Add assertion checks inside  buildSslCertGenerationParams and
    Ssl::certificateMatchesProperties functions (a)to avoid segfaults
  - In the case the signing certificate is not given in http_port configuration
    or the given certificate filename was not valid, squid does not start.
  - Creates the http_port_list::configureSslServerContext method and move
    here the cache_cf.cc code which was responsible to initialize ssl contexts
    and sslBump feature.

diff --git a/src/ProtoPort.cc b/src/ProtoPort.cc
index 9fba35322666525159c050346e7eb674cbdcb493..a520d9b5689c2008b0f0c8a06f8b760cbfe0ad4e 100644 (file)
--- a/src/ProtoPort.cc
+++ b/src/ProtoPort.cc
@@ -4,6 +4,9 @@
 #if HAVE_LIMITS
 #include <limits>
 #endif
+#if USE_SSL
+#include "ssl/support.h"
+#endif
 
 http_port_list::http_port_list(const char *aProtocol)
 #if USE_SSL
@@ -37,3 +40,41 @@ http_port_list::~http_port_list()
     safe_free(sslContextSessionId);
 #endif
 }
+
+#if USE_SSL
+void http_port_list::configureSslServerContext()
+{
+    staticSslContext.reset(
+        sslCreateServerContext(cert, key,
+                               version, cipher, options, sslflags, clientca,
+                               cafile, capath, crlfile, dhfile,
+                               sslContextSessionId));
+
+    if (!staticSslContext) {
+        char buf[128];
+        fatalf("%s_port %s initialization error", protocol,  s.ToURL(buf, sizeof(buf)));
+    }
+
+    if (!sslBump)
+        return;
+
+    if (cert)
+        Ssl::readCertChainAndPrivateKeyFromFiles(signingCert, signPkey, certsToChain, cert, key);
+
+    if (!signingCert) {
+        char buf[128];
+        fatalf("No valid signing SSL certificate configured for %s_port %s", protocol,  s.ToURL(buf, sizeof(buf)));
+    }
+
+    if (!signPkey)
+        debugs(3, DBG_IMPORTANT, "No SSL private key configured for  " <<  protocol << "_port " << s);
+
+    Ssl::generateUntrustedCert(untrustedSigningCert, untrustedSignPkey,
+                               signingCert, signPkey);
+
+    if (!untrustedSigningCert) {
+        char buf[128];
+        fatalf("Unable to generate  signing SSL certificate for untrusted sites for %s_port %s", protocol, s.ToURL(buf, sizeof(buf)));
+    }
+}
+#endif

diff --git a/src/ProtoPort.h b/src/ProtoPort.h
index 30405b43e9f6e050993f927111f017fae8064d25..6e3cad24b107f904069ffba36be73d9aaa4f7871 100644 (file)
--- a/src/ProtoPort.h
+++ b/src/ProtoPort.h
@@ -15,6 +15,9 @@
 struct http_port_list {
     http_port_list(const char *aProtocol);
     ~http_port_list();
+#if USE_SSL
+    void configureSslServerContext();
+#endif
 
     http_port_list *next;
 

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index cf687ed0c54e14756623390142076488d2693f7d..49f09077d86c94fe05960b4d375d16acd9206fd5 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -906,24 +906,11 @@ configDoConfigure(void)
         http_port_list *s;
 
         for (s = Config.Sockaddr.http; s != NULL; s = (http_port_list *) s->next) {
-            if (!s->cert)
+            if (!s->sslBump)
                 continue;
 
             debugs(3, 1, "Initializing http_port " << s->s << " SSL context");
-
-            s->staticSslContext.reset(
-                sslCreateServerContext(s->cert, s->key,
-                                       s->version, s->cipher, s->options, s->sslflags, s->clientca,
-                                       s->cafile, s->capath, s->crlfile, s->dhfile,
-                                       s->sslContextSessionId));
-
-            Ssl::readCertChainAndPrivateKeyFromFiles(s->signingCert, s->signPkey, s->certsToChain, s->cert, s->key);
-
-            if (!s->signPkey)
-                debugs(3, DBG_IMPORTANT, "No SSL private key configured for  http_port " << s->s);
-
-            Ssl::generateUntrustedCert(s->untrustedSigningCert, s->untrustedSignPkey,
-                                       s->signingCert, s->signPkey);
+            s->configureSslServerContext();
         }
     }
 
@@ -933,21 +920,7 @@ configDoConfigure(void)
 
         for (s = Config.Sockaddr.https; s != NULL; s = s->next) {
             debugs(3, 1, "Initializing https_port " << s->s << " SSL context");
-
-            s->staticSslContext.reset(
-                sslCreateServerContext(s->cert, s->key,
-                                       s->version, s->cipher, s->options, s->sslflags, s->clientca,
-                                       s->cafile, s->capath, s->crlfile, s->dhfile,
-                                       s->sslContextSessionId));
-
-            if (s->cert && s->sslBump) {
-                Ssl::readCertChainAndPrivateKeyFromFiles(s->signingCert, s->signPkey, s->certsToChain, s->cert, s->key);
-                if (!s->signPkey)
-                    debugs(3, DBG_IMPORTANT, "No SSL private key configured for  https_port " << s->s);
-
-                Ssl::generateUntrustedCert(s->untrustedSigningCert, s->untrustedSignPkey,
-                                           s->signingCert, s->signPkey);
-            }
+            s->configureSslServerContext();
         }
     }
 

diff --git a/src/client_side.cc b/src/client_side.cc
index f29464ec1ad3d712c2fa07abedf2cb4ca7197ddf..9381f09b5cdafbeca1cd5d8bf13b247372b6e1e2 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3741,12 +3741,13 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
     assert(certProperties.signAlgorithm != Ssl::algSignEnd);
 
     if (certProperties.signAlgorithm == Ssl::algSignUntrusted) {
+        assert(port->untrustedSigningCert.get());
         certProperties.signWithX509.resetAndLock(port->untrustedSigningCert.get());
         certProperties.signWithPkey.resetAndLock(port->untrustedSignPkey.get());
     }
     else {
-        if (port->signingCert.get())
-            certProperties.signWithX509.resetAndLock(port->signingCert.get());
+        assert(port->signingCert.get());
+        certProperties.signWithX509.resetAndLock(port->signingCert.get());
 
         if (port->signPkey.get())
             certProperties.signWithPkey.resetAndLock(port->signPkey.get());

diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc
index ec4b14d6f65cefce27f3d64189de6f8192d20856..55fcabe6f4f585e026a4b96e639008a8047040fe 100644 (file)
--- a/src/ssl/gadgets.cc
+++ b/src/ssl/gadgets.cc
@@ -554,6 +554,7 @@ bool Ssl::certificateMatchesProperties(X509 *cert, CertificateProperties const &
 
     // For non self-signed certificates we have to check if the signing certificate changed
     if (properties.signAlgorithm != Ssl::algSignSelf) {
+        assert(properties.signWithX509.get());
         if (X509_check_issued(properties.signWithX509.get(), cert) != X509_V_OK)
             return false;
     }

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index ec7f9af58b5ab2a8583f9fc61891675875cb4388..c40ad61b9f97844bd4f7282ef50bccffe1bae773 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -707,6 +707,11 @@ sslCreateServerContext(const char *certfile, const char *keyfile, int version, c
     if (!CAfile)
         CAfile = clientCA;
 
+    if (!certfile) {
+        debugs(83, DBG_CRITICAL, "ERROR: No certificate file");
+        return NULL;
+    }
+
     switch (version) {
 
     case 2:
@@ -741,8 +746,8 @@ sslCreateServerContext(const char *certfile, const char *keyfile, int version, c
 
     if (sslContext == NULL) {
         ssl_error = ERR_get_error();
-        fatalf("Failed to allocate SSL context: %s\n",
-               ERR_error_string(ssl_error, NULL));
+        debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate SSL context: " << ERR_error_string(ssl_error, NULL));
+        return NULL;
     }
 
     SSL_CTX_set_options(sslContext, ssl_parse_options(options));
@@ -766,8 +771,9 @@ sslCreateServerContext(const char *certfile, const char *keyfile, int version, c
 
         if (!SSL_CTX_set_cipher_list(sslContext, cipher)) {
             ssl_error = ERR_get_error();
-            fatalf("Failed to set SSL cipher suite '%s': %s\n",
-                   cipher, ERR_error_string(ssl_error, NULL));
+            debugs(83, DBG_CRITICAL, "ERROR: Failed to set SSL cipher suite '" << cipher << "': " << ERR_error_string(ssl_error, NULL));
+            SSL_CTX_free(sslContext);
+            return NULL;
         }
     }