From: Amos Jeffries Date: Sun, 11 Oct 2015 12:13:57 +0000 (-0700) Subject: Shuffle DH params parsing and pre-loading to libsecurity X-Git-Tag: SQUID_4_0_1~6^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=104deb987bb37afbb21fd36366b88aaf5a2a0e1e;p=thirdparty%2Fsquid.git Shuffle DH params parsing and pre-loading to libsecurity --- diff --git a/src/anyp/PortCfg.cc b/src/anyp/PortCfg.cc index 1a38f83f78..1377d489d9 100644 --- a/src/anyp/PortCfg.cc +++ b/src/anyp/PortCfg.cc @@ -55,8 +55,7 @@ AnyP::PortCfg::PortCfg() : certsToChain(), untrustedSigningCert(), untrustedSignPkey(), - clientCA(), - dhParams() + clientCA() #endif { memset(&tcp_keepalive, 0, sizeof(tcp_keepalive)); @@ -148,9 +147,6 @@ AnyP::PortCfg::configureSslServerContext() secure.updateTlsVersionLimits(); - if (!secure.dhParamsFile.isEmpty()) - dhParams.reset(Ssl::readDHParams(secure.dhParamsFile.c_str())); - staticSslContext.reset(sslCreateServerContext(*this)); if (!staticSslContext) { diff --git a/src/anyp/PortCfg.h b/src/anyp/PortCfg.h index 9364cf9271..7c77e506eb 100644 --- a/src/anyp/PortCfg.h +++ b/src/anyp/PortCfg.h @@ -86,7 +86,6 @@ public: Ssl::EVP_PKEY_Pointer untrustedSignPkey; ///< private key for signing untrusted generated certificates Ssl::X509_NAME_STACK_Pointer clientCA; ///< CA certificates to use when verifying client certificates - Ssl::DH_Pointer dhParams; ///< DH parameters for temporary/ephemeral DH key exchanges #endif }; diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h index 9045d48168..aff0810e49 100644 --- a/src/security/PeerOptions.h +++ b/src/security/PeerOptions.h @@ -30,7 +30,7 @@ public: virtual void parse(const char *); /// reset the configuration details to default - void clear() {*this = PeerOptions();} + virtual void clear() {*this = PeerOptions();} /// generate a security client-context from these configured options Security::ContextPointer createClientContext(bool setOptions); diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index 8df08048b6..37e774cc3b 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -21,7 +21,8 @@ Security::ServerOptions::ServerOptions(const Security::ServerOptions &s) : dh(s.dh), dhParamsFile(s.dhParamsFile), - eecdhCurve(s.eecdhCurve) + eecdhCurve(s.eecdhCurve), + parsedDhParams(s.parsedDhParams) { } @@ -54,6 +55,8 @@ Security::ServerOptions::parse(const char *token) } } + loadDhParams(); + } else if (strncmp(token, "dhparams=", 9) == 0) { if (!eecdhCurve.isEmpty()) { debugs(83, DBG_PARSE_NOTE(1), "UPGRADE WARNING: EECDH settings in tls-dh= override dhparams="); @@ -65,6 +68,8 @@ Security::ServerOptions::parse(const char *token) dh.append(token + 9); dhParamsFile = dh; + loadDhParams(); + } else { // parse generic TLS options Security::PeerOptions::parse(token); @@ -86,35 +91,74 @@ Security::ServerOptions::dumpCfg(Packable *p, const char *pfx) const } void -Security::ServerOptions::updateContextEecdh(Security::ContextPointer &ctx) +Security::ServerOptions::loadDhParams() { - if (eecdhCurve.isEmpty()) + if (dhParamsFile.isEmpty()) return; - debugs(83, 9, "Setting Ephemeral ECDH curve to " << eecdhCurve << "."); - -#if USE_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x0090800fL && !defined(OPENSSL_NO_ECDH) - int nid = OBJ_sn2nid(eecdhCurve.c_str()); - if (!nid) { - debugs(83, DBG_CRITICAL, "ERROR: Unknown EECDH curve '" << eecdhCurve << "'"); - return; +#if USE_OPENSSL + DH *dhp = nullptr; + if (FILE *in = fopen(dhParamsFile.c_str(), "r")) { + dhp = PEM_read_DHparams(in, NULL, NULL, NULL); + fclose(in); } - auto ecdh = EC_KEY_new_by_curve_name(nid); - if (!ecdh) { - auto ssl_error = ERR_get_error(); - debugs(83, DBG_CRITICAL, "ERROR: Unable to configure Ephemeral ECDH: " << ERR_error_string(ssl_error, NULL)); + if (!dhp) { + debugs(83, DBG_IMPORTANT, "WARNING: Failed to read DH parameters '" << dhParamsFile << "'"); return; } - if (SSL_CTX_set_tmp_ecdh(ctx, ecdh) != 0) { - auto ssl_error = ERR_get_error(); - debugs(83, DBG_CRITICAL, "ERROR: Unable to set Ephemeral ECDH: " << ERR_error_string(ssl_error, NULL)); + int codes; + if (DH_check(dhp, &codes) == 0) { + if (codes) { + debugs(83, DBG_IMPORTANT, "WARNING: Failed to verify DH parameters '" << dhParamsFile << "' (" << std::hex << codes << ")"); + DH_free(dhp); + dhp = nullptr; + } } - EC_KEY_free(ecdh); + + parsedDhParams.reset(dhp); +#endif +} + +void +Security::ServerOptions::updateContextEecdh(Security::ContextPointer &ctx) +{ + // set Elliptic Curve details into the server context + if (!eecdhCurve.isEmpty()) { + debugs(83, 9, "Setting Ephemeral ECDH curve to " << eecdhCurve << "."); + +#if USE_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x0090800fL && !defined(OPENSSL_NO_ECDH) + int nid = OBJ_sn2nid(eecdhCurve.c_str()); + if (!nid) { + debugs(83, DBG_CRITICAL, "ERROR: Unknown EECDH curve '" << eecdhCurve << "'"); + return; + } + + auto ecdh = EC_KEY_new_by_curve_name(nid); + if (!ecdh) { + auto ssl_error = ERR_get_error(); + debugs(83, DBG_CRITICAL, "ERROR: Unable to configure Ephemeral ECDH: " << ERR_error_string(ssl_error, NULL)); + return; + } + + if (SSL_CTX_set_tmp_ecdh(ctx, ecdh) != 0) { + auto ssl_error = ERR_get_error(); + debugs(83, DBG_CRITICAL, "ERROR: Unable to set Ephemeral ECDH: " << ERR_error_string(ssl_error, NULL)); + } + EC_KEY_free(ecdh); + #else - debugs(83, DBG_CRITICAL, "ERROR: EECDH is not available in this build." << - " Please link against OpenSSL>=0.9.8 and ensure OPENSSL_NO_ECDH is not set."); + debugs(83, DBG_CRITICAL, "ERROR: EECDH is not available in this build." << + " Please link against OpenSSL>=0.9.8 and ensure OPENSSL_NO_ECDH is not set."); +#endif + } + + // set DH parameters into the server context +#if USE_OPENSSL + if (parsedDhParams.get()) { + SSL_CTX_set_tmp_dh(ctx, parsedDhParams.get()); + } #endif } diff --git a/src/security/ServerOptions.h b/src/security/ServerOptions.h index 5b24a21137..9c09ea21f5 100644 --- a/src/security/ServerOptions.h +++ b/src/security/ServerOptions.h @@ -30,10 +30,17 @@ public: /// update the context with DH, EDH, EECDH settings void updateContextEecdh(Security::ContextPointer &); -public: +private: + void loadDhParams(); + +//public: SBuf dh; ///< Diffi-Helman cipher config + +private: SBuf dhParamsFile; ///< Diffi-Helman ciphers parameter file SBuf eecdhCurve; ///< Elliptic curve for ephemeral EC-based DH key exchanges + + Security::DhePointer parsedDhParams; ///< DH parameters for temporary/ephemeral DH key exchanges }; } // namespace Security diff --git a/src/security/forward.h b/src/security/forward.h index 2a0522f1ce..9286f836dc 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -76,6 +76,13 @@ typedef void *CrlPointer; typedef std::list CertRevokeList; +#if USE_OPENSSL +CtoCpp1(DH_free, DH *); +typedef Security::LockingPointer DhePointer; +#else +typedef void *DhePointer; +#endif + } // namespace Security #endif /* SQUID_SRC_SECURITY_FORWARD_H */ diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h index 366538ca15..c5c44e2370 100644 --- a/src/ssl/gadgets.h +++ b/src/ssl/gadgets.h @@ -72,9 +72,6 @@ typedef TidyPointer SSL_CTX_Pointer; CtoCpp1(SSL_free, SSL *) typedef TidyPointer SSL_Pointer; -CtoCpp1(DH_free, DH *); -typedef TidyPointer DH_Pointer; - sk_free_wrapper(sk_X509_NAME, STACK_OF(X509_NAME) *, X509_NAME_free) typedef TidyPointer X509_NAME_STACK_Pointer; diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 6860a86534..2ee7e21c80 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -471,30 +471,6 @@ ssl_initialize(void) ssl_ex_index_ssl_validation_counter = SSL_get_ex_new_index(0, (void *) "ssl_validation_counter", NULL, NULL, &ssl_free_int); } -DH * -Ssl::readDHParams(const char *dhfile) -{ - FILE *in = fopen(dhfile, "r"); - DH *dh = NULL; - int codes; - - if (in) { - dh = PEM_read_DHparams(in, NULL, NULL, NULL); - fclose(in); - } - - if (!dh) - debugs(83, DBG_IMPORTANT, "WARNING: Failed to read DH parameters '" << dhfile << "'"); - else if (dh && DH_check(dh, &codes) == 0) { - if (codes) { - debugs(83, DBG_IMPORTANT, "WARNING: Failed to verify DH parameters '" << dhfile << "' (" << std::hex << codes << ")"); - DH_free(dh); - dh = NULL; - } - } - return dh; -} - #if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) static void ssl_info_cb(const SSL *ssl, int where, int ret) @@ -571,10 +547,6 @@ configureSslContext(SSL_CTX *sslContext, AnyP::PortCfg &port) SSL_CTX_set_verify(sslContext, SSL_VERIFY_NONE, NULL); } - if (port.dhParams.get()) { - SSL_CTX_set_tmp_dh(sslContext, port.dhParams.get()); - } - if (port.secure.parsedFlags & SSL_FLAG_DONT_VERIFY_DOMAIN) SSL_CTX_set_ex_data(sslContext, ssl_ctx_ex_index_dont_verify_domain, (void *) -1); diff --git a/src/ssl/support.h b/src/ssl/support.h index 75cb39ee24..949fea79a2 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -155,12 +155,6 @@ inline const char *bumpMode(int bm) return (0 <= bm && bm < Ssl::bumpEnd) ? Ssl::BumpModeStr[bm] : NULL; } -/** - \ingroup ServerProtocolSSLAPI - * Load DH params from file - */ -DH *readDHParams(const char *dhfile); - /** \ingroup ServerProtocolSSLAPI * Generate a certificate to be used as untrusted signing certificate, based on a trusted CA diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index 401cb7afc0..b4ee0098ff 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -72,7 +72,6 @@ namespace Ssl //GETX509ATTRIBUTE GetX509CAAttribute; //GETX509ATTRIBUTE GetX509Fingerprint; const char *BumpModeStr[] = {""}; -DH *readDHParams(const char *dhfile) STUB_RETVAL(NULL) bool generateUntrustedCert(Security::CertPointer & untrustedCert, EVP_PKEY_Pointer & untrustedPkey, Security::CertPointer const & cert, EVP_PKEY_Pointer const & pkey) STUB_RETVAL(false) SSL_CTX * generateSslContext(CertificateProperties const &properties, AnyP::PortCfg &port) STUB_RETVAL(NULL) bool verifySslCertificate(SSL_CTX * sslContext, CertificateProperties const &properties) STUB_RETVAL(false)