From: Amos Jeffries Date: Tue, 5 Jul 2016 17:00:36 +0000 (+1200) Subject: Cleanup: make createStaticServerContext deal with ContextPointer X-Git-Tag: SQUID_4_0_13~40 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9ad528b8bd0f01f17eb67882ddec56cf1bdcab96;p=thirdparty%2Fsquid.git Cleanup: make createStaticServerContext deal with ContextPointer This method is about initializing the staticContext member of ServerOptions. There is no need to involve the caller directly in those logistics. --- diff --git a/src/anyp/PortCfg.cc b/src/anyp/PortCfg.cc index dfcc2dd9de..6c0c3a7a26 100644 --- a/src/anyp/PortCfg.cc +++ b/src/anyp/PortCfg.cc @@ -143,8 +143,7 @@ AnyP::PortCfg::configureSslServerContext() } } - secure.staticContext.reset(secure.createStaticServerContext(*this)); - if (!secure.staticContext) { + if (!secure.createStaticServerContext(*this)) { char buf[128]; fatalf("%s_port %s initialization error", AnyP::ProtocolType_str[transport.protocol], s.toUrl(buf, sizeof(buf))); } diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index c6be66a8fc..955ad7dfe1 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -117,19 +117,21 @@ Security::ServerOptions::createBlankContext() const return t; } -Security::ContextPtr +bool Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &port) { updateTlsVersionLimits(); - Security::ContextPtr t = createBlankContext(); + Security::ContextPointer t(createBlankContext()); if (t) { #if USE_OPENSSL - Ssl::InitServerContext(t, port); + if (!Ssl::InitServerContext(t, port)) + return false; #endif } - return t; + staticContext = std::move(t); + return bool(staticContext); } void diff --git a/src/security/ServerOptions.h b/src/security/ServerOptions.h index 530aa4ab36..c679835f72 100644 --- a/src/security/ServerOptions.h +++ b/src/security/ServerOptions.h @@ -33,7 +33,9 @@ public: virtual void dumpCfg(Packable *, const char *pfx) const; /// generate a security server-context from these configured options - Security::ContextPtr createStaticServerContext(AnyP::PortCfg &); + /// the resulting context is stored in staticContext + /// \returns true if a context could be created + bool 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 7f9e569ec5..6c05f1babd 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -557,63 +557,57 @@ configureSslContext(Security::ContextPtr sslContext, AnyP::PortCfg &port) } bool -Ssl::InitServerContext(Security::ContextPtr &sslContext, AnyP::PortCfg &port) +Ssl::InitServerContext(const Security::ContextPointer &ctx, AnyP::PortCfg &port) { - if (!sslContext) + if (!ctx) return false; - if (!SSL_CTX_use_certificate(sslContext, port.signingCert.get())) { + if (!SSL_CTX_use_certificate(ctx.get(), 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 false; } - if (!SSL_CTX_use_PrivateKey(sslContext, port.signPkey.get())) { + if (!SSL_CTX_use_PrivateKey(ctx.get(), port.signPkey.get())) { const int ssl_error = ERR_get_error(); 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 false; } - Ssl::addChainToSslContext(sslContext, port.certsToChain.get()); + Ssl::addChainToSslContext(ctx.get(), port.certsToChain.get()); /* Alternate code; debugs(83, DBG_IMPORTANT, "Using certificate in " << certfile); - if (!SSL_CTX_use_certificate_chain_file(sslContext, certfile)) { + if (!SSL_CTX_use_certificate_chain_file(ctx.get(), certfile)) { 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 false; } debugs(83, DBG_IMPORTANT, "Using private key in " << keyfile); - ssl_ask_password(sslContext, keyfile); + ssl_ask_password(ctx.get(), keyfile); - if (!SSL_CTX_use_PrivateKey_file(sslContext, keyfile, SSL_FILETYPE_PEM)) { + if (!SSL_CTX_use_PrivateKey_file(ctx.get(), keyfile, SSL_FILETYPE_PEM)) { 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 false; } debugs(83, 5, "Comparing private and public SSL keys."); - if (!SSL_CTX_check_private_key(sslContext)) { + if (!SSL_CTX_check_private_key(ctx.get())) { ssl_error = ERR_get_error(); 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 false; } */ - if (!configureSslContext(sslContext, port)) { + if (!configureSslContext(ctx.get(), port)) { debugs(83, DBG_CRITICAL, "ERROR: Configuring static SSL context"); - SSL_CTX_free(sslContext); return false; } diff --git a/src/ssl/support.h b/src/ssl/support.h index 1c28d7f47f..0ef586762b 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -112,7 +112,7 @@ extern Ipc::MemMap *SessionCache; extern const char *SessionCacheName; /// initialize a TLS server context with OpenSSL specific settings -bool InitServerContext(Security::ContextPtr &, AnyP::PortCfg &); +bool InitServerContext(const Security::ContextPointer &, AnyP::PortCfg &); /// initialize a TLS client context with OpenSSL specific settings bool InitClientContext(Security::ContextPtr &, Security::PeerOptions &, long options, long flags); diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index bc7c4d1b9b..ba40c5591a 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -34,7 +34,7 @@ void parse_securePeerOptions(Security::PeerOptions *) STUB void Security::ServerOptions::parse(const char *) STUB void Security::ServerOptions::dumpCfg(Packable *, const char *) const STUB Security::ContextPtr Security::ServerOptions::createBlankContext() const STUB -Security::ContextPtr Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) STUB_RETVAL(nullptr) +bool Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) STUB_RETVAL(false) void Security::ServerOptions::updateContextEecdh(Security::ContextPtr &) STUB #include "security/NegotiationHistory.h" diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index c622726eed..564dc3b53b 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -55,7 +55,7 @@ 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 InitServerContext(const Security::ContextPointer &, AnyP::PortCfg &) STUB_RETVAL(false) bool InitClientContext(Security::ContextPtr &, Security::PeerOptions &, long, const char *) STUB_RETVAL(false) } // namespace Ssl int ssl_read_method(int, char *, int) STUB_RETVAL(0)