From: Amos Jeffries Date: Sat, 19 Nov 2016 15:46:24 +0000 (+1300) Subject: TLS: Add ErrorString() function to libsecurity API X-Git-Tag: SQUID_4_0_17~18 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2b4bc9cf19942d96e27e478e333e6b9e2c65cb69;p=thirdparty%2Fsquid.git TLS: Add ErrorString() function to libsecurity API To convert library error codes to strings in a library agnostic way. --- diff --git a/src/client_side.cc b/src/client_side.cc index 0ceffb4835..506867e9c2 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2601,7 +2601,7 @@ Squid_SSL_accept(ConnStateData *conn, PF *callback) debugs(83, 2, "Error negotiating SSL connection on FD " << fd << ": Aborted by client: " << ssl_error); } else { debugs(83, (xerrno == ECONNRESET) ? 1 : 2, "Error negotiating SSL connection on FD " << fd << ": " << - (xerrno == 0 ? ERR_error_string(ssl_error, NULL) : xstrerr(xerrno))); + (xerrno == 0 ? Security::ErrorString(ssl_error) : xstrerr(xerrno))); } return -1; @@ -2611,7 +2611,7 @@ Squid_SSL_accept(ConnStateData *conn, PF *callback) default: debugs(83, DBG_IMPORTANT, "Error negotiating SSL connection on FD " << - fd << ": " << ERR_error_string(ERR_get_error(), NULL) << + fd << ": " << Security::ErrorString(ERR_get_error()) << " (" << ssl_error << "/" << ret << ")"); return -1; } diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 51e77833bc..14bbbcd07a 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -104,9 +104,11 @@ Security::PeerConnector::initialize(Security::SessionPointer &serverSession) assert(ctx); if (!Ssl::CreateClient(ctx, serverConnection(), "server https start")) { + const auto xerrno = errno; + const auto ssl_error = ERR_get_error(); ErrorState *anErr = new ErrorState(ERR_SOCKET_FAILURE, Http::scInternalServerError, request.getRaw()); - anErr->xerrno = errno; - debugs(83, DBG_IMPORTANT, "Error allocating TLS handle: " << ERR_error_string(ERR_get_error(), NULL)); + anErr->xerrno = xerrno; + debugs(83, DBG_IMPORTANT, "Error allocating TLS handle: " << Security::ErrorString(ssl_error)); noteNegotiationDone(anErr); bail(anErr); return false; @@ -443,7 +445,7 @@ Security::PeerConnector::noteNegotiationError(const int ret, const int ssl_error const int fd = serverConnection()->fd; debugs(83, DBG_IMPORTANT, "Error negotiating SSL on FD " << fd << - ": " << ERR_error_string(ssl_lib_error, NULL) << " (" << + ": " << Security::ErrorString(ssl_lib_error) << " (" << ssl_error << "/" << ret << "/" << errno << ")"); ErrorState *anErr = NULL; diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc index 825a15759a..42d985e47b 100644 --- a/src/security/PeerOptions.cc +++ b/src/security/PeerOptions.cc @@ -228,8 +228,8 @@ Security::PeerOptions::createBlankContext() const SSL_CTX *t = SSL_CTX_new(SSLv23_client_method()); #endif if (!t) { - const auto x = ERR_error_string(ERR_get_error(), nullptr); - fatalf("Failed to allocate TLS client context: %s\n", x); + const auto x = ERR_get_error(); + fatalf("Failed to allocate TLS client context: %s\n", Security::ErrorString(x)); } ctx.resetWithoutLocking(t); @@ -237,7 +237,7 @@ Security::PeerOptions::createBlankContext() const // Initialize for X.509 certificate exchange gnutls_certificate_credentials_t t; if (const int x = gnutls_certificate_allocate_credentials(&t)) { - fatalf("Failed to allocate TLS client context: error=%d\n", x); + fatalf("Failed to allocate TLS client context: %s\n", Security::ErrorString(x)); } ctx.resetWithoutLocking(t); @@ -574,12 +574,12 @@ loadSystemTrustedCa(Security::ContextPointer &ctx) { #if USE_OPENSSL if (SSL_CTX_set_default_verify_paths(ctx.get()) == 0) - return ERR_error_string(ERR_get_error(), nullptr); + return Security::ErrorString(ERR_get_error()); #elif USE_GNUTLS auto x = gnutls_certificate_set_x509_system_trust(ctx.get()); if (x < 0) - return gnutls_strerror(x); + return Security::ErrorString(x); #endif return nullptr; @@ -595,12 +595,15 @@ Security::PeerOptions::updateContextCa(Security::ContextPointer &ctx) for (auto i : caFiles) { #if USE_OPENSSL if (!SSL_CTX_load_verify_locations(ctx.get(), i.c_str(), path)) { - const int ssl_error = ERR_get_error(); - debugs(83, DBG_IMPORTANT, "WARNING: Ignoring error setting CA certificate locations: " << ERR_error_string(ssl_error, NULL)); + const auto x = ERR_get_error(); + debugs(83, DBG_IMPORTANT, "WARNING: Ignoring error setting CA certificate location " << + i << ": " << Security::ErrorString(x)); } #elif USE_GNUTLS - if (gnutls_certificate_set_x509_trust_file(ctx.get(), i.c_str(), GNUTLS_X509_FMT_PEM) < 0) { - debugs(83, DBG_IMPORTANT, "WARNING: Ignoring error setting CA certificate location: " << i); + const auto x = gnutls_certificate_set_x509_trust_file(ctx.get(), i.c_str(), GNUTLS_X509_FMT_PEM); + if (x < 0) { + debugs(83, DBG_IMPORTANT, "WARNING: Ignoring error setting CA certificate location " << + i << ": " << Security::ErrorString(x)); } #endif } diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index 13d49f11ee..1cbe49c66a 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -98,8 +98,8 @@ Security::ServerOptions::createBlankContext() const SSL_CTX *t = SSL_CTX_new(SSLv23_server_method()); #endif if (!t) { - const auto x = ERR_error_string(ERR_get_error(), nullptr); - debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: " << x); + const auto x = ERR_get_error(); + debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: " << Security::ErrorString(x)); } ctx.resetWithoutLocking(t); @@ -107,7 +107,7 @@ Security::ServerOptions::createBlankContext() const // Initialize for X.509 certificate exchange gnutls_certificate_credentials_t t; if (const int x = gnutls_certificate_allocate_credentials(&t)) { - debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: error=" << x); + debugs(83, DBG_CRITICAL, "ERROR: Failed to allocate TLS server context: " << Security::ErrorString(x)); } ctx.resetWithoutLocking(t); @@ -183,14 +183,14 @@ Security::ServerOptions::updateContextEecdh(Security::ContextPointer &ctx) 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)); + const auto x = ERR_get_error(); + debugs(83, DBG_CRITICAL, "ERROR: Unable to configure Ephemeral ECDH: " << Security::ErrorString(x)); return; } if (!SSL_CTX_set_tmp_ecdh(ctx.get(), ecdh)) { - auto ssl_error = ERR_get_error(); - debugs(83, DBG_CRITICAL, "ERROR: Unable to set Ephemeral ECDH: " << ERR_error_string(ssl_error, NULL)); + const auto x = ERR_get_error(); + debugs(83, DBG_CRITICAL, "ERROR: Unable to set Ephemeral ECDH: " << Security::ErrorString(x)); } EC_KEY_free(ecdh); diff --git a/src/security/Session.cc b/src/security/Session.cc index be77734858..1d4f6bc767 100644 --- a/src/security/Session.cc +++ b/src/security/Session.cc @@ -43,7 +43,7 @@ Security::MaybeGetSessionResumeData(const Security::SessionPointer &s, Security: gnutls_datum_t *tmp = nullptr; const auto x = gnutls_session_get_data2(s.get(), tmp); if (x != GNUTLS_E_SUCCESS) { - debugs(83, 3, "session=" << (void*)s.get() << " error: " << gnutls_strerror(x)); + debugs(83, 3, "session=" << (void*)s.get() << " error: " << Security::ErrorString(x)); } data.reset(tmp); #endif @@ -61,13 +61,13 @@ Security::SetSessionResumeData(const Security::SessionPointer &s, const Security if (!SSL_set_session(s.get(), data.get())) { const auto ssl_error = ERR_get_error(); debugs(83, 3, "session=" << (void*)s.get() << " data=" << (void*)data.get() << - " resume error: " << ERR_error_string(ssl_error, nullptr)); + " resume error: " << Security::ErrorString(ssl_error)); } #elif USE_GNUTLS const auto x = gnutls_session_set_data(s.get(), data->data, data->size); if (x != GNUTLS_E_SUCCESS) { debugs(83, 3, "session=" << (void*)s.get() << " data=" << (void*)data.get() << - " resume error: " << gnutls_strerror(x)); + " resume error: " << Security::ErrorString(x)); } #else // critical because, how did it get here? diff --git a/src/security/forward.h b/src/security/forward.h index 5369968088..3273402889 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -13,12 +13,13 @@ #include "security/Context.h" #include "security/Session.h" -#if USE_GNUTLS -#if HAVE_GNUTLS_X509_H +#if USE_GNUTLS && HAVE_GNUTLS_X509_H #include #endif -#endif #include +#if USE_OPENSSL && HAVE_OPENSSL_ERR_H +#include +#endif #include #if USE_OPENSSL @@ -93,6 +94,16 @@ class EncryptorAnswer; /// Squid defined error code (<0), an error code returned by X.509 API, or SSL_ERROR_NONE typedef int ErrorCode; +inline const char *ErrorString(const ErrorCode code) { +#if USE_OPENSSL + return ERR_error_string(code, nullptr); +#elif USE_GNUTLS + return gnutls_strerror(code); +#else + return "[no TLS library]"; +#endif +} + /// set of Squid defined TLS error codes /// \note using std::unordered_set ensures values are unique, with fast lookup typedef std::unordered_set Errors; diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc index 830303aea7..5ad12858e1 100644 --- a/src/ssl/ErrorDetail.cc +++ b/src/ssl/ErrorDetail.cc @@ -553,7 +553,7 @@ const char *Ssl::ErrorDetail::err_lib_error() const if (errReason.size() > 0) return errReason.termedBuf(); else if (lib_error_no != SSL_ERROR_NONE) - return ERR_error_string(lib_error_no, NULL); + return Security::ErrorString(lib_error_no); else return "[No Error]"; } @@ -564,7 +564,7 @@ const char *Ssl::ErrorDetail::err_lib_error() const * Error meta information: * %err_name: The name of a high-level SSL error (e.g., X509_V_ERR_*) * %ssl_error_descr: A short description of the SSL error - * %ssl_lib_error: human-readable low-level error string by ERR_error_string(3SSL) + * %ssl_lib_error: human-readable low-level error string by Security::ErrorString() * * Certificate information extracted from broken (not necessarily peer!) cert * %ssl_cn: The comma-separated list of common and alternate names diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index 359e835054..223034904f 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -308,7 +308,7 @@ Ssl::PeekingPeerConnector::noteNegotiationError(const int result, const int ssl_ (srvBio->bumpMode() == Ssl::bumpPeek || srvBio->bumpMode() == Ssl::bumpStare) && srvBio->holdWrite()) { Security::CertPointer serverCert(SSL_get_peer_certificate(session.get())); if (serverCert) { - debugs(81, 3, "Error (" << ERR_error_string(ssl_lib_error, NULL) << ") but, hold write on SSL connection on FD " << fd); + debugs(81, 3, "Error (" << Security::ErrorString(ssl_lib_error) << ") but, hold write on SSL connection on FD " << fd); checkForPeekAndSplice(); return; } diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 6c4495cde6..b8cc4db883 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -455,7 +455,7 @@ Ssl::Initialize(void) if (!ENGINE_set_default(e, ENGINE_METHOD_ALL)) { const int ssl_error = ERR_get_error(); - fatalf("Failed to initialise SSL engine: %s\n", ERR_error_string(ssl_error, NULL)); + fatalf("Failed to initialise SSL engine: %s\n", Security::ErrorString(ssl_error)); } } #else @@ -519,7 +519,7 @@ configureSslContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) if (!SSL_CTX_set_cipher_list(ctx.get(), port.secure.sslCipher.c_str())) { ssl_error = ERR_get_error(); - debugs(83, DBG_CRITICAL, "ERROR: Failed to set SSL cipher suite '" << port.secure.sslCipher << "': " << ERR_error_string(ssl_error, NULL)); + debugs(83, DBG_CRITICAL, "ERROR: Failed to set SSL cipher suite '" << port.secure.sslCipher << "': " << Security::ErrorString(ssl_error)); return false; } } @@ -536,7 +536,7 @@ configureSslContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) SSL_CTX_set_client_CA_list(ctx.get(), clientca); } else { ssl_error = ERR_get_error(); - debugs(83, DBG_CRITICAL, "ERROR: Failed to dupe the client CA list: " << ERR_error_string(ssl_error, NULL)); + debugs(83, DBG_CRITICAL, "ERROR: Failed to dupe the client CA list: " << Security::ErrorString(ssl_error)); return false; } @@ -572,14 +572,14 @@ Ssl::InitServerContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) 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)); + debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire TLS certificate '" << keys.certFile << "': " << Security::ErrorString(ssl_error)); return false; } 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)); + debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire TLS private key '" << keys.privateKeyFile << "': " << Security::ErrorString(ssl_error)); return false; } @@ -590,7 +590,7 @@ Ssl::InitServerContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) 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)); + debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire SSL certificate '" << certfile << "': " << Security::ErrorString(ssl_error)); return false; } @@ -599,7 +599,7 @@ Ssl::InitServerContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) 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)); + debugs(83, DBG_CRITICAL, "ERROR: Failed to acquire SSL private key '" << keyfile << "': " << Security::ErrorString(ssl_error)); return false; } @@ -608,7 +608,7 @@ Ssl::InitServerContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) 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)); + keyfile << "': " << Security::ErrorString(ssl_error)); return false; } */ @@ -640,7 +640,7 @@ Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &pee if (!SSL_CTX_set_cipher_list(ctx.get(), cipher)) { const int ssl_error = ERR_get_error(); fatalf("Failed to set SSL cipher suite '%s': %s\n", - cipher, ERR_error_string(ssl_error, NULL)); + cipher, Security::ErrorString(ssl_error)); } } @@ -654,7 +654,7 @@ Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &pee if (!SSL_CTX_use_certificate_chain_file(ctx.get(), certfile)) { const int ssl_error = ERR_get_error(); fatalf("Failed to acquire SSL certificate '%s': %s\n", - certfile, ERR_error_string(ssl_error, NULL)); + certfile, Security::ErrorString(ssl_error)); } debugs(83, DBG_IMPORTANT, "Using private key in " << keys.privateKeyFile); @@ -664,7 +664,7 @@ Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &pee if (!SSL_CTX_use_PrivateKey_file(ctx.get(), keyfile, SSL_FILETYPE_PEM)) { const int ssl_error = ERR_get_error(); fatalf("Failed to acquire SSL private key '%s': %s\n", - keyfile, ERR_error_string(ssl_error, NULL)); + keyfile, Security::ErrorString(ssl_error)); } debugs(83, 5, "Comparing private and public SSL keys."); @@ -672,7 +672,7 @@ Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &pee if (!SSL_CTX_check_private_key(ctx.get())) { const int ssl_error = ERR_get_error(); fatalf("SSL private key '%s' does not match public key '%s': %s\n", - certfile, keyfile, ERR_error_string(ssl_error, NULL)); + certfile, keyfile, Security::ErrorString(ssl_error)); } } } @@ -978,7 +978,7 @@ Ssl::chainCertificatesToSSLContext(Security::ContextPointer &ctx, AnyP::PortCfg X509_up_ref(signingCert); } else { const int ssl_error = ERR_get_error(); - debugs(33, DBG_IMPORTANT, "WARNING: can not add signing certificate to SSL context chain: " << ERR_error_string(ssl_error, NULL)); + debugs(33, DBG_IMPORTANT, "WARNING: can not add signing certificate to SSL context chain: " << Security::ErrorString(ssl_error)); } Ssl::addChainToSslContext(ctx, port.certsToChain.get()); } @@ -1069,7 +1069,7 @@ Ssl::setClientSNI(SSL *ssl, const char *fqdn) if (!SSL_set_tlsext_host_name(ssl, fqdn)) { const int ssl_error = ERR_get_error(); debugs(83, 3, "WARNING: unable to set TLS servername extension (SNI): " << - ERR_error_string(ssl_error, NULL) << "\n"); + Security::ErrorString(ssl_error) << "\n"); return false; } return true; @@ -1092,7 +1092,7 @@ Ssl::addChainToSslContext(Security::ContextPointer &ctx, STACK_OF(X509) *chain) X509_up_ref(cert); } else { const int ssl_error = ERR_get_error(); - debugs(83, DBG_IMPORTANT, "WARNING: can not add certificate to SSL context chain: " << ERR_error_string(ssl_error, NULL)); + debugs(83, DBG_IMPORTANT, "WARNING: can not add certificate to SSL context chain: " << Security::ErrorString(ssl_error)); } } } @@ -1439,7 +1439,7 @@ SslCreate(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &co } debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction << - ": " << ERR_error_string(errCode, NULL)); + ": " << Security::ErrorString(errCode)); return false; }