From: Amos Jeffries Date: Thu, 2 Jun 2016 09:52:43 +0000 (+1200) Subject: CryptoNG: cleanup TLS/SSL context initialization sequence X-Git-Tag: SQUID_4_0_11~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c75aba0236b968dc5d4c60597f3351b0704a2b69;p=thirdparty%2Fsquid.git CryptoNG: cleanup TLS/SSL context initialization sequence The libsecurity ServerOptions and PeerOptions class methods are now supposed to be the API for creating SSL contexts for https_port, cache_peer and outgoing connections. Continue the API transition by making the callers of sslCreate*Context() functions use the libsecurity API instead and repurpose the now obsolete functions into the Ssl:: namespace to initialize the keying material and other not-yet-converted OpenSSL state details of an existing context. A side effect of this is that GnuTLS contexts are now actually created and initialized as far as they can be. SSL-Bump context initialization is not altered by this. --- diff --git a/src/anyp/PortCfg.cc b/src/anyp/PortCfg.cc index 69f64bc027..dfcc2dd9de 100644 --- a/src/anyp/PortCfg.cc +++ b/src/anyp/PortCfg.cc @@ -143,9 +143,7 @@ AnyP::PortCfg::configureSslServerContext() } } - secure.updateTlsVersionLimits(); - secure.staticContext.reset(sslCreateServerContext(*this)); - + secure.staticContext.reset(secure.createStaticServerContext(*this)); if (!secure.staticContext) { char buf[128]; fatalf("%s_port %s initialization error", AnyP::ProtocolType_str[transport.protocol], s.toUrl(buf, sizeof(buf))); diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc index 79350c4249..3360b9c9b9 100644 --- a/src/security/PeerOptions.cc +++ b/src/security/PeerOptions.cc @@ -240,7 +240,7 @@ Security::PeerOptions::createBlankContext() const } #else - fatal("Failed to allocate TLS client context: No TLS library\n"); + debugs(83, 1, "WARNING: Failed to allocate TLS client context: No TLS library"); #endif @@ -250,20 +250,14 @@ Security::PeerOptions::createBlankContext() const Security::ContextPtr Security::PeerOptions::createClientContext(bool setOptions) { - Security::ContextPtr t = nullptr; - updateTlsVersionLimits(); + Security::ContextPtr t = createBlankContext(); + if (t) { #if USE_OPENSSL - // XXX: temporary performance regression. c_str() data copies and prevents this being a const method - t = sslCreateClientContext(*this, (setOptions ? parsedOptions : 0), parsedFlags); - -#elif USE_GNUTLS && WHEN_READY_FOR_GNUTLS - t = createBlankContext(); - + // XXX: temporary performance regression. c_str() data copies and prevents this being a const method + Ssl::InitClientContext(t, *this, (setOptions ? parsedOptions : 0), parsedFlags); #endif - - if (t) { updateContextNpn(t); updateContextCa(t); updateContextCrl(t); diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index 34450af8ea..c6be66a8fc 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -117,6 +117,21 @@ Security::ServerOptions::createBlankContext() const return t; } +Security::ContextPtr +Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &port) +{ + updateTlsVersionLimits(); + + Security::ContextPtr t = createBlankContext(); + if (t) { +#if USE_OPENSSL + Ssl::InitServerContext(t, port); +#endif + } + + return t; +} + void Security::ServerOptions::loadDhParams() { diff --git a/src/security/ServerOptions.h b/src/security/ServerOptions.h index ced552387c..530aa4ab36 100644 --- a/src/security/ServerOptions.h +++ b/src/security/ServerOptions.h @@ -9,6 +9,7 @@ #ifndef SQUID_SRC_SECURITY_SERVEROPTIONS_H #define SQUID_SRC_SECURITY_SERVEROPTIONS_H +#include "anyp/forward.h" #include "security/PeerOptions.h" namespace Security @@ -31,6 +32,9 @@ public: virtual Security::ContextPtr createBlankContext() const; virtual void dumpCfg(Packable *, const char *pfx) const; + /// generate a security server-context from these configured options + Security::ContextPtr createStaticServerContext(AnyP::PortCfg &); + /// update the context with DH, EDH, EECDH settings void updateContextEecdh(Security::ContextPtr &); diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 70be136bea..7f9e569ec5 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -556,19 +556,18 @@ configureSslContext(Security::ContextPtr sslContext, AnyP::PortCfg &port) return true; } -Security::ContextPtr -sslCreateServerContext(AnyP::PortCfg &port) +bool +Ssl::InitServerContext(Security::ContextPtr &sslContext, AnyP::PortCfg &port) { - Security::ContextPtr sslContext(port.secure.createBlankContext()); if (!sslContext) - return nullptr; + return false; if (!SSL_CTX_use_certificate(sslContext, port.signingCert.get())) { const int ssl_error = ERR_get_error(); 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; + return false; } if (!SSL_CTX_use_PrivateKey(sslContext, port.signPkey.get())) { @@ -576,7 +575,7 @@ sslCreateServerContext(AnyP::PortCfg &port) 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; + return false; } Ssl::addChainToSslContext(sslContext, port.certsToChain.get()); @@ -588,7 +587,7 @@ sslCreateServerContext(AnyP::PortCfg &port) ssl_error = ERR_get_error(); debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire SSL certificate '" << certfile << "': " << ERR_error_string(ssl_error, NULL)); SSL_CTX_free(sslContext); - return NULL; + return false; } debugs(83, DBG_IMPORTANT, "Using private key in " << keyfile); @@ -598,7 +597,7 @@ sslCreateServerContext(AnyP::PortCfg &port) ssl_error = ERR_get_error(); debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire SSL private key '" << keyfile << "': " << ERR_error_string(ssl_error, NULL)); SSL_CTX_free(sslContext); - return NULL; + return false; } debugs(83, 5, "Comparing private and public SSL keys."); @@ -608,25 +607,24 @@ sslCreateServerContext(AnyP::PortCfg &port) debugs(83, DBG_CRITICAL, "ERROR: SSL private key '" << certfile << "' does not match public key '" << keyfile << "': " << ERR_error_string(ssl_error, NULL)); SSL_CTX_free(sslContext); - return NULL; + return false; } */ if (!configureSslContext(sslContext, port)) { debugs(83, DBG_CRITICAL, "ERROR: Configuring static SSL context"); SSL_CTX_free(sslContext); - return NULL; + return false; } - return sslContext; + return true; } -Security::ContextPtr -sslCreateClientContext(Security::PeerOptions &peer, long options, long fl) +bool +Ssl::InitClientContext(Security::ContextPtr &sslContext, Security::PeerOptions &peer, long options, long fl) { - Security::ContextPtr sslContext(peer.createBlankContext()); if (!sslContext) - return nullptr; + return false; SSL_CTX_set_options(sslContext, options); @@ -689,7 +687,7 @@ sslCreateClientContext(Security::PeerOptions &peer, long options, long fl) SSL_CTX_set_verify(sslContext, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ssl_verify_cb); } - return sslContext; + return true; } /// \ingroup ServerProtocolSSLInternal diff --git a/src/ssl/support.h b/src/ssl/support.h index 16b4b2fb54..1c28d7f47f 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -111,13 +111,13 @@ void SetSessionCallbacks(Security::ContextPtr); extern Ipc::MemMap *SessionCache; extern const char *SessionCacheName; -} //namespace Ssl +/// initialize a TLS server context with OpenSSL specific settings +bool InitServerContext(Security::ContextPtr &, AnyP::PortCfg &); -/// \ingroup ServerProtocolSSLAPI -Security::ContextPtr sslCreateServerContext(AnyP::PortCfg &port); +/// initialize a TLS client context with OpenSSL specific settings +bool InitClientContext(Security::ContextPtr &, Security::PeerOptions &, long options, long flags); -/// \ingroup ServerProtocolSSLAPI -Security::ContextPtr sslCreateClientContext(Security::PeerOptions &, long options, long flags); +} //namespace Ssl /// \ingroup ServerProtocolSSLAPI int ssl_read_method(int, char *, int); diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index 3cc147a58c..c622726eed 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -55,9 +55,9 @@ namespace Ssl CertError & CertError::operator = (const CertError &old) STUB_RETVAL(*this) bool CertError::operator == (const CertError &ce) const STUB_RETVAL(false) bool CertError::operator != (const CertError &ce) const STUB_RETVAL(false) +bool InitServerContext(Security::ContextPtr &, AnyP::PortCfg &) STUB_RETVAL(false) +bool InitClientContext(Security::ContextPtr &, Security::PeerOptions &, long, const char *) STUB_RETVAL(false) } // namespace Ssl -Security::ContextPtr sslCreateServerContext(AnyP::PortCfg &port) STUB_RETVAL(NULL) -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