From: Amos Jeffries Date: Tue, 8 Dec 2015 01:48:40 +0000 (-0800) Subject: TLS: refactor cert=/key= storage in libsecurity X-Git-Tag: SQUID_4_0_4~46 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d1d72d438501b135b7443bffa41fcb432dd2132b;p=thirdparty%2Fsquid.git 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 ae3c677d61..8240f2042f 100644 --- 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 index 0000000000..cd0c5e298e --- /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 3c6d8fb3e9..414754a596 100644 --- 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 b87cea1671..3bcdc1fd64 100644 --- 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 266a832121..02a266a6a7 100644 --- 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 certs; ///< details from the cert= and file= config parameters std::list 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 63da965958..71395d86cd 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -65,6 +65,8 @@ typedef Security::LockingPointer 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 cd3ff24c6b..17bc85459b 100644 --- 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 bb4a37e7c9..baf25c8109 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -96,7 +96,7 @@ typedef CbDataList 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 68291c35b5..c815bcaeb6 100644 --- 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