From 7e62a74f1094fd994fe1377a8e3b7fc2ccb1b85a Mon Sep 17 00:00:00 2001
From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sat, 15 Nov 2014 04:18:32 -0800
Subject: [PATCH] Deprecate various sslproxy_* directives

... replaced by tls_outgoing_options.

Also fix sslCreateClientContext() expectations. SBuf.c_str() produces
'\0' for empty strings instead of NULL.

Also, rename the global config object ProxyOutgoingConfig to avoid SSL
naming.
---
 doc/release-notes/release-3.6.sgml |  26 +++++
 src/SquidConfig.h                  |  10 --
 src/cache_cf.cc                    |  34 +++++-
 src/cf.data.pre                    | 162 +++++++++--------------------
 src/security/PeerOptions.cc        |   2 +-
 src/security/PeerOptions.h         |   4 +-
 src/ssl/PeerConnector.cc           |   1 +
 src/ssl/support.cc                 |  14 +--
 src/tests/stub_libsecurity.cc      |   2 +-
 9 files changed, 118 insertions(+), 137 deletions(-)

diff --git a/doc/release-notes/release-3.6.sgml b/doc/release-notes/release-3.6.sgml
index 367c8494b9..052b6c50f3 100644
--- a/doc/release-notes/release-3.6.sgml
+++ b/doc/release-notes/release-3.6.sgml
@@ -64,6 +64,9 @@ This section gives a thorough account of those changes in three categories:
 <sect1>New tags<label id="newtags">
 <p>
 <descrip>
+	<tag>tls_outgoing_options</tag>
+	<p>New tag to define TLS security context options for outgoing
+	   connections. For example to HTTPS servers.
 
 </descrip>
 
@@ -76,6 +79,29 @@ This section gives a thorough account of those changes in three categories:
 <sect1>Removed tags<label id="removedtags">
 <p>
 <descrip>
+	<tag>sslproxy_cafile</tag>
+	<p>Replaced by <em>tls_outgoing_options cafile=</em>.
+
+	<tag>sslproxy_capath</tag>
+	<p>Replaced by <em>tls_outgoing_options capath=</em>.
+
+	<tag>sslproxy_cipher</tag>
+	<p>Replaced by <em>tls_outgoing_options cipher=</em>.
+
+	<tag>sslproxy_client_certificate</tag>
+	<p>Replaced by <em>tls_outgoing_options cert=</em>.
+
+	<tag>sslproxy_client_key</tag>
+	<p>Replaced by <em>tls_outgoing_options key=</em>.
+
+	<tag>sslproxy_flags</tag>
+	<p>Replaced by <em>tls_outgoing_options flags=</em>.
+
+	<tag>sslproxy_options</tag>
+	<p>Replaced by <em>tls_outgoing_options options=</em>.
+
+	<tag>sslproxy_version</tag>
+	<p>Replaced by <em>tls_outgoing_options version=</em>.
 
 </descrip>
 
diff --git a/src/SquidConfig.h b/src/SquidConfig.h
index 47dda24563..f4e82a667d 100644
--- a/src/SquidConfig.h
+++ b/src/SquidConfig.h
@@ -493,17 +493,7 @@ public:
     external_acl *externalAclHelperList;
 
 #if USE_OPENSSL
-
     struct {
-        char *cert;
-        char *key;
-        int version;
-        char *options;
-        char *cipher;
-        char *cafile;
-        char *capath;
-        char *crlfile;
-        char *flags;
         acl_access *cert_error;
         SSL_CTX *sslContext;
         sslproxy_cert_sign *cert_sign;
diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index a5ec5ab344..7ea8e9e60a 100644
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -881,7 +881,9 @@ configDoConfigure(void)
 
     debugs(3, DBG_IMPORTANT, "Initializing https proxy context");
 
-    Config.ssl_client.sslContext = sslCreateClientContext(Config.ssl_client.cert, Config.ssl_client.key, Config.ssl_client.version, Config.ssl_client.cipher, Config.ssl_client.options, Config.ssl_client.flags, Config.ssl_client.cafile, Config.ssl_client.capath, Config.ssl_client.crlfile);
+    // BUG: ssl_client.sslContext will leak on reconfigure when Config gets memset()
+    // it makes more sense to create a context per outbound connection instead of this
+    Config.ssl_client.sslContext = Security::ProxyOutgoingConfig.createContext();
 
     for (CachePeer *p = Config.peers; p != NULL; p = p->next) {
         if (p->secure.ssl) {
@@ -972,6 +974,36 @@ parse_obsolete(const char *name)
         parse_onoff(&temp);
         Config.onoff.cache_miss_revalidate = !temp;
     }
+
+    if (!strncmp(name, "sslproxy_", 9)) {
+        // the replacement directive tls_outgoing_options uses options instead of whole-line input
+        SBuf tmp;
+        if (!strcmp(name, "sslproxy_cafile"))
+            tmp.append("cafile=");
+        else if (!strcmp(name, "sslproxy_capath"))
+            tmp.append("capath=");
+        else if (!strcmp(name, "sslproxy_cipher"))
+            tmp.append("cipher=");
+        else if (!strcmp(name, "sslproxy_client_certificate"))
+            tmp.append("cert=");
+        else if (!strcmp(name, "sslproxy_client_key"))
+            tmp.append("key=");
+        else if (!strcmp(name, "sslproxy_flags"))
+            tmp.append("flags=");
+        else if (!strcmp(name, "sslproxy_options"))
+            tmp.append("options=");
+        else if (!strcmp(name, "sslproxy_version"))
+            tmp.append("version=");
+        else {
+            debugs(3, DBG_CRITICAL, "ERROR: unknown directive: " << name);
+            self_destruct();
+        }
+
+        // add the value as unquoted-string because the old values did not support whitespace
+        const char *token = ConfigParser::NextQuotedOrToEol();
+        tmp.append(token, strlen(token));
+        Security::ProxyOutgoingConfig.parse(tmp.c_str());
+    }
 }
 
 /* Parse a time specification from the config file.  Store the
diff --git a/src/cf.data.pre b/src/cf.data.pre
index ed00f4255e..99abcf530d 100644
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -169,6 +169,55 @@ DOC_START
 	This option is not yet supported by Squid-3.
 DOC_END
 
+# Options removed in 3.6
+NAME: sslproxy_cafile
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options cafile= instead.
+DOC_END
+
+NAME: sslproxy_capath
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options capath= instead.
+DOC_END
+
+NAME: sslproxy_cipher
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options cipher= instead.
+DOC_END
+
+NAME: sslproxy_client_certificate
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options cert= instead.
+DOC_END
+
+NAME: sslproxy_client_key
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options key= instead.
+DOC_END
+
+NAME: sslproxy_flags
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options flags= instead.
+DOC_END
+
+NAME: sslproxy_options
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options options= instead.
+DOC_END
+
+NAME: sslproxy_version
+TYPE: obsolete
+DOC_START
+	Remove this line. Use tls_outgoing_options version= instead.
+DOC_END
+
 # Options removed in 3.5
 NAME: hierarchy_stoplist
 TYPE: obsolete
@@ -2373,7 +2422,7 @@ NAME: tls_outgoing_options
 IFDEF: USE_GNUTLS||USE_OPENSSL
 TYPE: securePeerOptions
 DEFAULT: disable
-LOC: Security::SslProxyConfig
+LOC: Security::ProxyOutgoingConfig
 DOC_START
 	disable		Do not support https:// URLs.
 	
@@ -2464,104 +2513,6 @@ DOC_START
 	would like to use hardware SSL acceleration for example.
 DOC_END
 
-NAME: sslproxy_client_certificate
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.cert
-TYPE: string
-DOC_START
-	Client SSL Certificate to use when proxying https:// URLs
-DOC_END
-
-NAME: sslproxy_client_key
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.key
-TYPE: string
-DOC_START
-	Client SSL Key to use when proxying https:// URLs
-DOC_END
-
-NAME: sslproxy_version
-IFDEF: USE_OPENSSL
-DEFAULT: 1
-DEFAULT_DOC: automatic SSL/TLS version negotiation
-LOC: Config.ssl_client.version
-TYPE: int
-DOC_START
-	SSL version level to use when proxying https:// URLs
-
-	The versions of SSL/TLS supported:
-
-	    1	automatic (default)
-	    2	SSLv2 only
-	    3	SSLv3 only
-	    4	TLSv1.0 only
-	    5	TLSv1.1 only
-	    6	TLSv1.2 only
-DOC_END
-
-NAME: sslproxy_options
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.options
-TYPE: string
-DOC_START
-	SSL implementation options to use when proxying https:// URLs
-	
-	The most important being:
-
-	    NO_SSLv2    Disallow the use of SSLv2
-	    NO_SSLv3    Disallow the use of SSLv3
-	    NO_TLSv1    Disallow the use of TLSv1.0
-	    NO_TLSv1_1  Disallow the use of TLSv1.1
-	    NO_TLSv1_2  Disallow the use of TLSv1.2
-	    SINGLE_DH_USE
-		      Always create a new key when using temporary/ephemeral
-		      DH key exchanges
-	    SSL_OP_NO_TICKET
-		      Disable use of RFC5077 session tickets. Some servers
-		      may have problems understanding the TLS extension due
-		      to ambiguous specification in RFC4507.
-	    ALL       Enable various bug workarounds suggested as "harmless"
-		      by OpenSSL. Be warned that this may reduce SSL/TLS
-		      strength to some attacks.
-	
-	See the OpenSSL SSL_CTX_set_options documentation for a
-	complete list of possible options.
-DOC_END
-
-NAME: sslproxy_cipher
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.cipher
-TYPE: string
-DOC_START
-	SSL cipher list to use when proxying https:// URLs
-
-	Colon separated list of supported ciphers.
-DOC_END
-
-NAME: sslproxy_cafile
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.cafile
-TYPE: string
-DOC_START
-	file containing CA certificates to use when verifying server
-	certificates while proxying https:// URLs
-DOC_END
-
-NAME: sslproxy_capath
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.capath
-TYPE: string
-DOC_START
-	directory containing CA certificates to use when verifying
-	server certificates while proxying https:// URLs
-DOC_END
-
 NAME: sslproxy_session_ttl
 IFDEF: USE_OPENSSL
 DEFAULT: 300
@@ -2683,19 +2634,6 @@ DOC_START
 	ssl_bump bump all
 DOC_END
 
-NAME: sslproxy_flags
-IFDEF: USE_OPENSSL
-DEFAULT: none
-LOC: Config.ssl_client.flags
-TYPE: string
-DOC_START
-	Various flags modifying the use of SSL while proxying https:// URLs:
-	    DONT_VERIFY_PEER	Accept certificates that fail verification.
-				For refined control, see sslproxy_cert_error.
-	    NO_DEFAULT_CA	Don't use the default CA list built in
-				to OpenSSL.
-DOC_END
-
 NAME: sslproxy_cert_error
 IFDEF: USE_OPENSSL
 DEFAULT: none
diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc
index 03d44e26f7..712f5a5896 100644
--- a/src/security/PeerOptions.cc
+++ b/src/security/PeerOptions.cc
@@ -16,7 +16,7 @@
 #include "ssl/support.h"
 #endif
 
-Security::PeerOptions Security::SslProxyConfig;
+Security::PeerOptions Security::ProxyOutgoingConfig;
 
 void
 Security::PeerOptions::parse(const char *token)
diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h
index 9dbf9ae4cf..4e35d41e49 100644
--- a/src/security/PeerOptions.h
+++ b/src/security/PeerOptions.h
@@ -46,7 +46,7 @@ public:
 };
 
 /// configuration options for DIRECT server access
-extern PeerOptions SslProxyConfig;
+extern PeerOptions ProxyOutgoingConfig;
 
 } // namespace Security
 
@@ -59,7 +59,7 @@ parse_securePeerOptions(Security::PeerOptions *opt)
     }
 }
 
-#define free_securePeerOptions(x) Security::SslProxyConfig.clear()
+#define free_securePeerOptions(x) Security::ProxyOutgoingConfig.clear()
 #define dump_securePeerOptions(e,n,x) // not supported yet
 
 #endif /* SQUID_SRC_SECURITY_PEEROPTIONS_H */
diff --git a/src/ssl/PeerConnector.cc b/src/ssl/PeerConnector.cc
index 0ae5b4b610..08f906ec67 100644
--- a/src/ssl/PeerConnector.cc
+++ b/src/ssl/PeerConnector.cc
@@ -114,6 +114,7 @@ Ssl::PeerConnector::initializeSsl()
         assert(peer->secure.ssl);
         sslContext = peer->sslContext;
     } else {
+        // XXX: locate a per-server context in Security:: instead ?
         sslContext = ::Config.ssl_client.sslContext;
     }
 
diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index b6a4f38618..993cfc9537 100644
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -1135,12 +1135,6 @@ sslCreateClientContext(const char *certfile, const char *keyfile, int version, c
 
     ssl_initialize();
 
-    if (!keyfile)
-        keyfile = certfile;
-
-    if (!certfile)
-        certfile = keyfile;
-
     if (!(method = Ssl::method(version)))
         return NULL;
 
@@ -1154,7 +1148,7 @@ sslCreateClientContext(const char *certfile, const char *keyfile, int version, c
 
     SSL_CTX_set_options(sslContext, Ssl::parse_options(options));
 
-    if (cipher) {
+    if (*cipher) {
         debugs(83, 5, "Using chiper suite " << cipher << ".");
 
         if (!SSL_CTX_set_cipher_list(sslContext, cipher)) {
@@ -1164,7 +1158,7 @@ sslCreateClientContext(const char *certfile, const char *keyfile, int version, c
         }
     }
 
-    if (certfile) {
+    if (*certfile) {
         debugs(83, DBG_IMPORTANT, "Using certificate in " << certfile);
 
         if (!SSL_CTX_use_certificate_chain_file(sslContext, certfile)) {
@@ -1204,12 +1198,12 @@ sslCreateClientContext(const char *certfile, const char *keyfile, int version, c
 
     debugs(83, 9, "Setting CA certificate locations.");
 
-    if ((CAfile || CApath) && !SSL_CTX_load_verify_locations(sslContext, CAfile, CApath)) {
+    if ((*CAfile || *CApath) && !SSL_CTX_load_verify_locations(sslContext, CAfile, CApath)) {
         ssl_error = ERR_get_error();
         debugs(83, DBG_IMPORTANT, "WARNING: Ignoring error setting CA certificate locations: " << ERR_error_string(ssl_error, NULL));
     }
 
-    if (CRLfile) {
+    if (*CRLfile) {
         ssl_load_crl(sslContext, CRLfile);
         fl |= SSL_FLAG_VERIFY_CRL;
     }
diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index e93f185e6b..51635edfba 100644
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -12,6 +12,6 @@
 #include "tests/STUB.h"
 
 #include "security/PeerOptions.h"
-Security::PeerOptions Security::SslProxyConfig;
+Security::PeerOptions Security::ProxyOutgoingConfig;
 void Security::PeerOptions::parse(char const*) STUB
 Security::ContextPointer Security::PeerOptions::createContext() STUB_RETVAL(NULL)
-- 
2.47.3