TLS: refactor cert=/key= storage in libsecurity

This updates the cert=/key= filename storage from single entries
in PeerOptions to a list of key pairs in preparation for supporting
multiple certificates on client or server TLS contexts.

key= following a cert= parameter is now enforced, rather than just
warned about.

squid.conf can now be configured with multiple [cert= [key=...]]
pairs of filenames, however only the first is used. This differs
from older behaviour where the last value(s) were used. But since
configurations with multiple values was not supported previously
this seems acceptible breakage.

Since the multi-cert support is not fully existing yet this config
ability is left undocumented for now.

diff --git a/src/anyp/PortCfg.cc b/src/anyp/PortCfg.cc
index ae3c677d612ee73239398836c4194f3a68645d12..8240f2042faee6851fcde110ff95421dec325bea 100644 (file)
--- a/src/anyp/PortCfg.cc
+++ b/src/anyp/PortCfg.cc
@@ -115,8 +115,10 @@ AnyP::PortCfg::clone() const
 void
 AnyP::PortCfg::configureSslServerContext()
 {
-    if (!secure.certFile.isEmpty())
-        Ssl::readCertChainAndPrivateKeyFromFiles(signingCert, signPkey, certsToChain, secure.certFile.c_str(), secure.privateKeyFile.c_str());
+    if (!secure.certs.empty()) {
+        Security::KeyData &keys = secure.certs.front();
+        Ssl::readCertChainAndPrivateKeyFromFiles(signingCert, signPkey, certsToChain, keys.certFile.c_str(), keys.privateKeyFile.c_str());
+    }
 
     if (!signingCert) {
         char buf[128];

diff --git a/src/security/KeyData.h b/src/security/KeyData.h
new file mode 100644 (file)
index 0000000..cd0c5e2
--- /dev/null
+++ b/src/security/KeyData.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 1996-2015 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.
+ */
+
+#ifndef SQUID_SRC_SECURITY_KEYDATA_H
+#define SQUID_SRC_SECURITY_KEYDATA_H
+
+#include "SBuf.h"
+#include "security/forward.h"
+
+namespace Security
+{
+
+/// TLS certificate and private key details from squid.conf
+class KeyData
+{
+public:
+    SBuf certFile;       ///< path of file containing PEM format X.509 certificate
+    SBuf privateKeyFile; ///< path of file containing private key in PEM format
+};
+
+} // namespace Security
+
+#endif /* SQUID_SRC_SECURITY_KEYDATA_H */
+

diff --git a/src/security/Makefile.am b/src/security/Makefile.am
index 3c6d8fb3e9988aab4acdcc582032ef255f8e12bd..414754a59676a85d4b0cace946cb10c95fc12386 100644 (file)
--- a/src/security/Makefile.am
+++ b/src/security/Makefile.am
@@ -15,6 +15,7 @@ libsecurity_la_SOURCES= \
        EncryptorAnswer.cc \
        EncryptorAnswer.h \
        forward.h \
+       KeyData.h \
        LockingPointer.h \
        PeerOptions.cc \
        PeerOptions.h \

diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc
index b87cea1671b3f6f6d1724ddd4e9b7534324a1d8f..3bcdc1fd64f3ce5723ca6dc54c02fbf05028526c 100644 (file)
--- a/src/security/PeerOptions.cc
+++ b/src/security/PeerOptions.cc
@@ -23,8 +23,6 @@
 Security::PeerOptions Security::ProxyOutgoingConfig;
 
 Security::PeerOptions::PeerOptions(const Security::PeerOptions &p) :
-    certFile(p.certFile),
-    privateKeyFile(p.privateKeyFile),
     sslOptions(p.sslOptions),
     caDir(p.caDir),
     crlFile(p.crlFile),
@@ -33,6 +31,7 @@ Security::PeerOptions::PeerOptions(const Security::PeerOptions &p) :
     sslDomain(p.sslDomain),
     parsedOptions(p.parsedOptions),
     parsedFlags(p.parsedFlags),
+    certs(p.certs),
     caFiles(p.caFiles),
     parsedCrl(p.parsedCrl),
     sslVersion(p.sslVersion),
@@ -56,15 +55,16 @@ Security::PeerOptions::parse(const char *token)
     }
 
     if (strncmp(token, "cert=", 5) == 0) {
-        certFile = SBuf(token + 5);
-        if (privateKeyFile.isEmpty())
-            privateKeyFile = certFile;
+        KeyData t;
+        t.privateKeyFile = t.certFile = SBuf(token + 5);
+        certs.emplace_back(t);
     } else if (strncmp(token, "key=", 4) == 0) {
-        privateKeyFile = SBuf(token + 4);
-        if (certFile.isEmpty()) {
-            debugs(3, DBG_PARSE_NOTE(1), "WARNING: cert= option needs to be set before key= is used.");
-            certFile = privateKeyFile;
+        if (certs.empty() || certs.back().certFile.isEmpty()) {
+            debugs(3, DBG_PARSE_NOTE(1), "ERROR: cert= option must be set before key= is used.");
+            return;
         }
+        KeyData &t = certs.back();
+        t.privateKeyFile = SBuf(token + 4);
     } else if (strncmp(token, "version=", 8) == 0) {
         debugs(0, DBG_PARSE_NOTE(1), "UPGRADE WARNING: SSL version= is deprecated. Use options= to limit protocols instead.");
         sslVersion = xatoi(token + 8);
@@ -109,11 +109,13 @@ Security::PeerOptions::dumpCfg(Packable *p, const char *pfx) const
         return; // no other settings are relevant
     }
 
-    if (!certFile.isEmpty())
-        p->appendf(" %scert=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(certFile));
+    for (auto &i : certs) {
+        if (!i.certFile.isEmpty())
+            p->appendf(" %scert=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(i.certFile));
 
-    if (!privateKeyFile.isEmpty() && privateKeyFile != certFile)
-        p->appendf(" %skey=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(privateKeyFile));
+        if (!i.privateKeyFile.isEmpty() && i.privateKeyFile != i.certFile)
+            p->appendf(" %skey=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(i.privateKeyFile));
+    }
 
     if (!sslOptions.isEmpty())
         p->appendf(" %soptions=" SQUIDSBUFPH, pfx, SQUIDSBUFPRINT(sslOptions));
@@ -233,8 +235,7 @@ Security::PeerOptions::createClientContext(bool setOptions)
 
 #if USE_OPENSSL
     // XXX: temporary performance regression. c_str() data copies and prevents this being a const method
-    t = sslCreateClientContext(*this, certFile.c_str(), privateKeyFile.c_str(), sslCipher.c_str(),
-                               (setOptions ? parsedOptions : 0), parsedFlags);
+    t = sslCreateClientContext(*this, (setOptions ? parsedOptions : 0), parsedFlags);
 
 #elif USE_GNUTLS && WHEN_READY_FOR_GNUTLS
     t = createBlankContext();

diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h
index 266a832121d9a7a11c2462268ce6f8f410be7da1..02a266a6a7c8082a2521edf997f4f3b6158097c3 100644 (file)
--- a/src/security/PeerOptions.h
+++ b/src/security/PeerOptions.h
@@ -10,8 +10,7 @@
 #define SQUID_SRC_SECURITY_PEEROPTIONS_H
 
 #include "ConfigParser.h"
-#include "SBuf.h"
-#include "security/forward.h"
+#include "security/KeyData.h"
 
 class Packable;
 
@@ -56,8 +55,6 @@ private:
     void loadCrlFile();
 
 public:
-    SBuf certFile;       ///< path of file containing PEM format X509 certificate
-    SBuf privateKeyFile; ///< path of file containing private key in PEM format
     SBuf sslOptions;     ///< library-specific options string
     SBuf caDir;          ///< path of directory containing a set of trusted Certificate Authorities
     SBuf crlFile;        ///< path of file containing Certificate Revoke List
@@ -71,6 +68,7 @@ public:
     long parsedOptions; ///< parsed value of sslOptions
     long parsedFlags;   ///< parsed value of sslFlags
 
+    std::list<Security::KeyData> certs; ///< details from the cert= and file= config parameters
     std::list<SBuf> caFiles;  ///< paths of files containing trusted Certificate Authority
     Security::CertRevokeList parsedCrl; ///< CRL to use when verifying the remote end certificate
 

diff --git a/src/security/forward.h b/src/security/forward.h
index 63da96595899c07b644baa2f009b1e2ec6ff7e3e..71395d86cdb561786823f1ddd0be32e8702fa45e 100644 (file)
--- a/src/security/forward.h
+++ b/src/security/forward.h
@@ -65,6 +65,8 @@ typedef Security::LockingPointer<DH, DH_free_cpp, CRYPTO_LOCK_DH> DhePointer;
 typedef void *DhePointer;
 #endif
 
+class KeyData;
+
 } // namespace Security
 
 #endif /* SQUID_SRC_SECURITY_FORWARD_H */

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index cd3ff24c6b3b2c4661c24808c3bf360e58a31591..17bc85459bb05fcacea8cb5b5d1e83f5e02bf0f5 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -565,14 +565,16 @@ sslCreateServerContext(AnyP::PortCfg &port)
 
     if (!SSL_CTX_use_certificate(sslContext, port.signingCert.get())) {
         const int ssl_error = ERR_get_error();
-        debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire SSL certificate '" << port.secure.certFile << "': " << ERR_error_string(ssl_error, NULL));
+        const auto &keys = port.secure.certs.front();
+        debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire TLS certificate '" << keys.certFile << "': " << ERR_error_string(ssl_error, NULL));
         SSL_CTX_free(sslContext);
         return NULL;
     }
 
     if (!SSL_CTX_use_PrivateKey(sslContext, port.signPkey.get())) {
         const int ssl_error = ERR_get_error();
-        debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire SSL private key '" << port.secure.privateKeyFile << "': " << ERR_error_string(ssl_error, NULL));
+        const auto &keys = port.secure.certs.front();
+        debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire TLS private key '" << keys.privateKeyFile << "': " << ERR_error_string(ssl_error, NULL));
         SSL_CTX_free(sslContext);
         return NULL;
     }
@@ -631,7 +633,7 @@ ssl_next_proto_cb(SSL *s, unsigned char **out, unsigned char *outlen, const unsi
 #endif
 
 Security::ContextPtr
-sslCreateClientContext(Security::PeerOptions &peer, const char *certfile, const char *keyfile, const char *cipher, long options, long fl)
+sslCreateClientContext(Security::PeerOptions &peer, long options, long fl)
 {
     Security::ContextPtr sslContext(peer.createBlankContext());
     if (!sslContext)
@@ -643,9 +645,10 @@ sslCreateClientContext(Security::PeerOptions &peer, const char *certfile, const
     SSL_CTX_set_info_callback(sslContext, ssl_info_cb);
 #endif
 
-    if (*cipher) {
-        debugs(83, 5, "Using chiper suite " << cipher << ".");
+    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)) {
             const int ssl_error = ERR_get_error();
             fatalf("Failed to set SSL cipher suite '%s': %s\n",
@@ -653,16 +656,20 @@ sslCreateClientContext(Security::PeerOptions &peer, const char *certfile, const
         }
     }
 
-    if (*certfile) {
-        debugs(83, DBG_IMPORTANT, "Using certificate in " << certfile);
+    // TODO: support loading multiple cert/key pairs
+    auto &keys = peer.certs.front();
+    if (!keys.certFile.isEmpty()) {
+        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)) {
             const int ssl_error = ERR_get_error();
             fatalf("Failed to acquire SSL certificate '%s': %s\n",
                    certfile, ERR_error_string(ssl_error, NULL));
         }
 
-        debugs(83, DBG_IMPORTANT, "Using private key in " << keyfile);
+        debugs(83, DBG_IMPORTANT, "Using private key in " << keys.privateKeyFile);
+        const char *keyfile = keys.privateKeyFile.c_str();
         ssl_ask_password(sslContext, keyfile);
 
         if (!SSL_CTX_use_PrivateKey_file(sslContext, keyfile, SSL_FILETYPE_PEM)) {

diff --git a/src/ssl/support.h b/src/ssl/support.h
index bb4a37e7c9d3e6168d4acc7867e32cd1f8f7d130..baf25c810951f853c072c67dccc93517eba87f37 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -96,7 +96,7 @@ typedef CbDataList<Ssl::CertError> CertErrors;
 Security::ContextPtr sslCreateServerContext(AnyP::PortCfg &port);
 
 /// \ingroup ServerProtocolSSLAPI
-Security::ContextPtr sslCreateClientContext(Security::PeerOptions &, const char *certfile, const char *keyfile, const char *cipher, long options, long flags);
+Security::ContextPtr sslCreateClientContext(Security::PeerOptions &, long options, long flags);
 
 /// \ingroup ServerProtocolSSLAPI
 int ssl_read_method(int, char *, int);

diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc
index 68291c35b5883d8da3f42ed1a6725bd40e9abe15..c815bcaeb6ae2ea4106662abc5b3c08863eb97e2 100644 (file)
--- a/src/tests/stub_libsslsquid.cc
+++ b/src/tests/stub_libsslsquid.cc
@@ -57,7 +57,7 @@ bool CertError::operator == (const CertError &ce) const STUB_RETVAL(false)
 bool CertError::operator != (const CertError &ce) const STUB_RETVAL(false)
 } // namespace Ssl
 Security::ContextPtr sslCreateServerContext(AnyP::PortCfg &port) STUB_RETVAL(NULL)
-Security::ContextPtr sslCreateClientContext(Security::PeerOptions &, const char *, const char *, const char *, long, const char *) STUB_RETVAL(nullptr)
+Security::ContextPtr sslCreateClientContext(Security::PeerOptions &, long, const char *) STUB_RETVAL(nullptr)
 int ssl_read_method(int, char *, int) STUB_RETVAL(0)
 int ssl_write_method(int, const char *, int) STUB_RETVAL(0)
 void ssl_shutdown_method(SSL *ssl) STUB