Do not leak [SSL] objects tied to http_port and https_port on reconfigure

PortCfg objects were not destroyed at all (no delete call) and were
incorrectly stored (excessive cbdata locking). This change adds destruction
and removes excessive locking to allow the destructed object to be freed.
It also cleans up forgotten(?) clientca and crlfile PortCfg members.

This change fixes a serious leak but also carries an elevated risk:
There is a lot of code throughout Squid that does not check the pointers to
the objects that are now properly destroyed. It is possible that some of 
that code will crash some time after reconfigure. It is not possible to 
ensure that this does not happen without rewriting/fixing the offending
code to use refcounting. Such a rewrite would be a relatively large 
change outside this patch scope. We may decide that it is better to leak
than to take this additional risk.

=== modified file 'src/anyp/PortCfg.cc'
--- src/anyp/PortCfg.cc	2014-04-16 21:58:08 +0000
+++ src/anyp/PortCfg.cc	2014-04-24 20:19:11 +0000
@@ -57,44 +57,46 @@ AnyP::PortCfg::PortCfg() :
         sslContextFlags(0),
         sslOptions(0)
 #endif
 {
     memset(&tcp_keepalive, 0, sizeof(tcp_keepalive));
 }
 
 AnyP::PortCfg::~PortCfg()
 {
     if (Comm::IsConnOpen(listenConn)) {
         listenConn->close();
         listenConn = NULL;
     }
 
     safe_free(name);
     safe_free(defaultsite);
 
 #if USE_OPENSSL
     safe_free(cert);
     safe_free(key);
-    safe_free(options);
     safe_free(cipher);
+    safe_free(options);
+    safe_free(clientca);
     safe_free(cafile);
     safe_free(capath);
+    safe_free(crlfile);
     safe_free(dhfile);
     safe_free(sslflags);
     safe_free(sslContextSessionId);
 #endif
 }
 
 AnyP::PortCfg *
 AnyP::PortCfg::clone() const
 {
     AnyP::PortCfg *b = new AnyP::PortCfg();
     b->s = s;
     if (name)
         b->name = xstrdup(name);
     if (defaultsite)
         b->defaultsite = xstrdup(defaultsite);
 
     b->transport = transport;
     b->flags = flags;
     b->allow_direct = allow_direct;
     b->vhost = vhost;

=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc	2014-04-12 17:32:34 +0000
+++ src/cache_cf.cc	2014-04-24 20:23:43 +0000
@@ -3761,43 +3766,43 @@ parse_port_option(AnyP::PortCfg * s, cha
         s->generateHostCertificates = true;
     } else if (strcmp(token, "generate-host-certificates=off") == 0) {
         s->generateHostCertificates = false;
     } else if (strncmp(token, "dynamic_cert_mem_cache_size=", 28) == 0) {
         parseBytesOptionValue(&s->dynamicCertMemCacheSize, B_BYTES_STR, token + 28);
 #endif
     } else {
         debugs(3, DBG_CRITICAL, "FATAL: Unknown http(s)_port option '" << token << "'.");
         self_destruct();
     }
 }
 
 void
 add_http_port(char *portspec)
 {
     AnyP::PortCfg *s = new AnyP::PortCfg();
     s->setTransport("HTTP");
     parsePortSpecification(s, portspec);
     // we may need to merge better if the above returns a list with clones
     assert(s->next == NULL);
-    s->next = cbdataReference(Config.Sockaddr.http);
-    cbdataReferenceDone(Config.Sockaddr.http);
-    Config.Sockaddr.http = cbdataReference(s);
+    // no cbdata locking here: Config owns the ports (deleted in free_PortCfg)
+    s->next = Config.Sockaddr.http;
+    Config.Sockaddr.http = s;
 }
 
 static void
 parsePortCfg(AnyP::PortCfg ** head, const char *optionName)
 {
     const char *protocol = NULL;
     if (strcmp(optionName, "http_port") == 0 ||
             strcmp(optionName, "ascii_port") == 0)
         protocol = "http";
     else if (strcmp(optionName, "https_port") == 0)
         protocol = "https";
     if (!protocol) {
         self_destruct();
         return;
     }
 
     char *token = ConfigParser::NextToken();
 
     if (!token) {
         self_destruct();
@@ -3820,42 +3825,42 @@ parsePortCfg(AnyP::PortCfg ** head, cons
         if (s->flags.tunnelSslBumping && !hijacked) {
             debugs(3, DBG_CRITICAL, "FATAL: ssl-bump on https_port requires tproxy/intercept which is missing.");
             self_destruct();
         }
         if (hijacked && !s->flags.tunnelSslBumping) {
             debugs(3, DBG_CRITICAL, "FATAL: tproxy/intercept on https_port requires ssl-bump which is missing.");
             self_destruct();
         }
     }
 #endif
 
     if (Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK && s->s.isAnyAddr()) {
         // clone the port options from *s to *(s->next)
         s->next = cbdataReference(s->clone());
         s->next->s.setIPv4();
         debugs(3, 3, AnyP::UriScheme(s->transport.protocol).c_str() << "_port: clone wildcard address for split-stack: " << s->s << " and " << s->next->s);
     }
 
     while (*head)
         head = &(*head)->next;
-
-    *head = cbdataReference(s);
+    // no cbdata locking here: Config owns the ports (deleted in free_PortCfg)
+    *head = s;
 }
 
 static void
 dump_generic_port(StoreEntry * e, const char *n, const AnyP::PortCfg * s)
 {
     char buf[MAX_IPSTRLEN];
 
     storeAppendPrintf(e, "%s %s",
                       n,
                       s->s.toUrl(buf,MAX_IPSTRLEN));
 
     // MODES and specific sub-options.
     if (s->flags.natIntercept)
         storeAppendPrintf(e, " intercept");
 
     else if (s->flags.tproxyIntercept)
         storeAppendPrintf(e, " tproxy");
 
     else if (s->flags.accelSurrogate) {
         storeAppendPrintf(e, " accel");
@@ -3965,50 +3970,51 @@ dump_generic_port(StoreEntry * e, const 
 #endif
 }
 
 static void
 dump_PortCfg(StoreEntry * e, const char *n, const AnyP::PortCfg * s)
 {
     while (s) {
         dump_generic_port(e, n, s);
         storeAppendPrintf(e, "\n");
         s = s->next;
     }
 }
 
 static void
 free_PortCfg(AnyP::PortCfg ** head)
 {
     AnyP::PortCfg *s;
 
     while ((s = *head) != NULL) {
         *head = s->next;
-        cbdataReferenceDone(s);
+        delete s;
     }
 }
 
 void
 configFreeMemory(void)
 {
     free_all();
 #if USE_OPENSSL
     SSL_CTX_free(Config.ssl_client.sslContext);
+    Config.ssl_client.sslContext = NULL;
 #endif
 }
 
 void
 requirePathnameExists(const char *name, const char *path)
 {
 
     struct stat sb;
     char pathbuf[BUFSIZ];
     assert(path != NULL);
 
     if (Config.chroot_dir && (geteuid() == 0)) {
         snprintf(pathbuf, BUFSIZ, "%s/%s", Config.chroot_dir, path);
         path = pathbuf;
     }
 
     if (stat(path, &sb) < 0) {
         debugs(0, DBG_CRITICAL, (opt_parse_cfg_only?"FATAL ":"") << "ERROR: " << name << " " << path << ": " << xstrerror());
         // keep going to find more issues if we are only checking the config file with "-k parse"
         if (opt_parse_cfg_only)

=== modified file 'src/client_side.cc'
--- src/client_side.cc	2014-03-31 06:57:27 +0000
+++ src/client_side.cc	2014-04-24 20:12:30 +0000
@@ -3704,43 +3704,48 @@ ConnStateData::sslCrtdHandleReplyWrapper
     ConnStateData * state_data = (ConnStateData *)(data);
     state_data->sslCrtdHandleReply(reply);
 }
 
 void
 ConnStateData::sslCrtdHandleReply(const HelperReply &reply)
 {
     if (reply.result == HelperReply::BrokenHelper) {
         debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply);
     } else if (!reply.other().hasContent()) {
         debugs(1, DBG_IMPORTANT, HERE << "\"ssl_crtd\" helper returned <NULL> reply.");
     } else {
         Ssl::CrtdMessage reply_message(Ssl::CrtdMessage::REPLY);
         if (reply_message.parse(reply.other().content(), reply.other().contentSize()) != Ssl::CrtdMessage::OK) {
             debugs(33, 5, HERE << "Reply from ssl_crtd for " << sslConnectHostOrIp << " is incorrect");
         } else {
             if (reply.result != HelperReply::Okay) {
                 debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply_message.getBody());
             } else {
                 debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " was successfully recieved from ssl_crtd");
-                SSL_CTX *ctx = Ssl::generateSslContextUsingPkeyAndCertFromMemory(reply_message.getBody().c_str(), *port);
-                getSslContextDone(ctx, true);
-                return;
+                if (cbdataReferenceValid(port)) {
+                    SSL_CTX *ctx = Ssl::generateSslContextUsingPkeyAndCertFromMemory(reply_message.getBody().c_str(), *port);
+                    getSslContextDone(ctx, true);
+                    return;
+                } else {
+                    debugs(33, 3, "but our port is gone");
+                    // getSslContextDone() will cleanup
+                }
             }
         }
     }
     getSslContextDone(NULL);
 }
 
 void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties)
 {
     certProperties.commonName =  sslCommonName.size() > 0 ? sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
 
     // fake certificate adaptation requires bump-server-first mode
     if (!sslServerBump) {
         assert(port->signingCert.get());
         certProperties.signWithX509.resetAndLock(port->signingCert.get());
         if (port->signPkey.get())
             certProperties.signWithPkey.resetAndLock(port->signPkey.get());
         certProperties.signAlgorithm = Ssl::algSignTrusted;
         return;
     }
 
@@ -3806,41 +3811,41 @@ void ConnStateData::buildSslCertGenerati
         assert(port->untrustedSigningCert.get());
         certProperties.signWithX509.resetAndLock(port->untrustedSigningCert.get());
         certProperties.signWithPkey.resetAndLock(port->untrustedSignPkey.get());
     } else {
         assert(port->signingCert.get());
         certProperties.signWithX509.resetAndLock(port->signingCert.get());
 
         if (port->signPkey.get())
             certProperties.signWithPkey.resetAndLock(port->signPkey.get());
     }
     signAlgorithm = certProperties.signAlgorithm;
 }
 
 void
 ConnStateData::getSslContextStart()
 {
     assert(areAllContextsForThisConnection());
     freeAllContexts();
     /* careful: freeAllContexts() above frees request, host, etc. */
 
-    if (port->generateHostCertificates) {
+    if (cbdataReferenceValid(port) && port->generateHostCertificates) {
         Ssl::CertificateProperties certProperties;
         buildSslCertGenerationParams(certProperties);
         sslBumpCertKey = certProperties.dbKey().c_str();
         assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
 
         debugs(33, 5, HERE << "Finding SSL certificate for " << sslBumpCertKey << " in cache");
         Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
         SSL_CTX * dynCtx = NULL;
         Ssl::SSL_CTX_Pointer *cachedCtx = ssl_ctx_cache ? ssl_ctx_cache->get(sslBumpCertKey.termedBuf()) : NULL;
         if (cachedCtx && (dynCtx = cachedCtx->get())) {
             debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey << " have found in cache");
             if (Ssl::verifySslCertificate(dynCtx, certProperties)) {
                 debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is valid");
                 getSslContextDone(dynCtx);
                 return;
             } else {
                 debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is out of date. Delete this certificate from cache");
                 if (ssl_ctx_cache)
                     ssl_ctx_cache->del(sslBumpCertKey.termedBuf());
             }
@@ -3860,40 +3865,48 @@ ConnStateData::getSslContextStart()
         } catch (const std::exception &e) {
             debugs(33, DBG_IMPORTANT, "ERROR: Failed to compose ssl_crtd " <<
                    "request for " << certProperties.commonName <<
                    " certificate: " << e.what() << "; will now block to " <<
                    "generate that certificate.");
             // fall through to do blocking in-process generation.
         }
 #endif // USE_SSL_CRTD
 
         debugs(33, 5, HERE << "Generating SSL certificate for " << certProperties.commonName);
         dynCtx = Ssl::generateSslContext(certProperties, *port);
         getSslContextDone(dynCtx, true);
         return;
     }
     getSslContextDone(NULL);
 }
 
 void
 ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew)
 {
+    if (!cbdataReferenceValid(port)) { // reconfigure may delete our port
+        debugs(83, 2, "Closing SSL " << clientConnection->remote << " for lost port");
+        if (sslContext && Comm::IsConnOpen(clientConnection))
+            fd_table[clientConnection->fd].dynamicSslContext = sslContext;
+        clientConnection->close();
+        return;
+    }
+
     // Try to add generated ssl context to storage.
     if (port->generateHostCertificates && isNew) {
 
         if (signAlgorithm == Ssl::algSignTrusted) {
             // Add signing certificate to the certificates chain
             X509 *cert = port->signingCert.get();
             if (SSL_CTX_add_extra_chain_cert(sslContext, cert)) {
                 // increase the certificate lock
                 CRYPTO_add(&(cert->references),1,CRYPTO_LOCK_X509);
             } else {
                 const int ssl_error = ERR_get_error();
                 debugs(33, DBG_IMPORTANT, "WARNING: can not add signing certificate to SSL context chain: " << ERR_error_string(ssl_error, NULL));
             }
             Ssl::addChainToSslContext(sslContext, port->certsToChain.get());
         }
         //else it is self-signed or untrusted do not attrach any certificate
 
         Ssl::LocalContextStorage *ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s);
         assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0');
         if (sslContext) {