Updated Branches: refs/heads/master d91bef30e -> 783ce7204
TS-1946: reduce the verbosity of SSL handshake errors Update SSL error logging routines so that we can log the SSL error stack at debug level as well as error level. Update SSL error logging to log the location of the caller, rather than the location of the logging function. Drop SSL server handshake logs to debug level so that they don't overwhelm the diagnostics log. Handshake errors can be very common and are not generally interesting. Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/783ce720 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/783ce720 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/783ce720 Branch: refs/heads/master Commit: 783ce7204f5bb903b10877314f78f0d1b249fc4c Parents: d91bef3 Author: James Peach <[email protected]> Authored: Fri Jun 7 13:53:57 2013 -0700 Committer: James Peach <[email protected]> Committed: Mon Jun 24 20:15:37 2013 -0700 ---------------------------------------------------------------------- CHANGES | 2 ++ iocore/net/P_SSLUtils.h | 10 +++++++- iocore/net/SSLNetVConnection.cc | 37 ++++++++++++---------------- iocore/net/SSLUtils.cc | 38 ++++++++++++++++++++++++++--- lib/ts/Diags.h | 2 ++ proxy/config/records.config.default.in | 2 ++ 6 files changed, 64 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/783ce720/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index eefc9e7..6fd000c 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,8 @@ Changes with Apache Traffic Server 3.3.5 + *) [TS-1946] Reduce the verbosity of SSL handshake errors. + *) [TS-1971] Switch jtest over to standard argument parsing. *) [TS-1786] Only enable -Werror for development builds. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/783ce720/iocore/net/P_SSLUtils.h ---------------------------------------------------------------------- diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index 5f6dc01..4f1ec96 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -23,6 +23,7 @@ #define __P_SSLUTILS_H__ #include "ink_config.h" +#include "Diags.h" #define OPENSSL_THREAD_DEFINES #include <openssl/opensslconf.h> @@ -53,7 +54,14 @@ SSL_CTX * SSLInitClientContext(const SSLConfigParams * param); void SSLInitializeLibrary(); // Log an SSL error. -void SSLError(const char * fmt, ...) TS_PRINTFLIKE(1, 2); +#define SSLError(fmt, ...) SSLDiagnostic(DiagsMakeLocation(), false, fmt, ##__VA_ARGS__) +// Log a SSL diagnostic using the "ssl" diagnostic tag. +#define SSLDebug(fmt, ...) SSLDiagnostic(DiagsMakeLocation(), true, fmt, ##__VA_ARGS__) + +void SSLDiagnostic(const SrcLoc& loc, bool debug, const char * fmt, ...) TS_PRINTFLIKE(3, 4); + +// Return a static string name for a SSL_ERROR constant. +const char * SSLErrorName(int ssl_error); // Log a SSL network buffer. void SSLDebugBufferPrint(const char * tag, const char * buffer, unsigned buflen, const char * message); http://git-wip-us.apache.org/repos/asf/trafficserver/blob/783ce720/iocore/net/SSLNetVConnection.cc ---------------------------------------------------------------------- diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc index e114895..3f429eb 100644 --- a/iocore/net/SSLNetVConnection.cc +++ b/iocore/net/SSLNetVConnection.cc @@ -479,14 +479,22 @@ int SSLNetVConnection::sslServerHandShakeEvent(int &err) { int ret; + int ssl_error; ret = SSL_accept(ssl); - switch (SSL_get_error(ssl, ret)) { + + ssl_error = SSL_get_error(ssl, ret); + if (ssl_error != SSL_ERROR_NONE) { + err = errno; + SSLDebug("SSL handshake error: %s (%d), errno=%d", SSLErrorName(ssl_error), ssl_error, err); + } + + switch (ssl_error) { case SSL_ERROR_NONE: - Debug("ssl", "SSLNetVConnection::sslServerHandShakeEvent, handshake completed successfully"); + Debug("ssl", "handshake completed successfully"); client_cert = SSL_get_peer_certificate(ssl); if (client_cert != NULL) { -/* str = X509_NAME_oneline (X509_get_subject_name (client_cert), 0, 0); +/* str = X509_NAME_oneline (X509_get_subject_name (client_cert), 0, 0); Free (str); str = X509_NAME_oneline (X509_get_issuer_name (client_cert), 0, 0); @@ -520,9 +528,6 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err) return EVENT_DONE; - case SSL_ERROR_WANT_ACCEPT: - break; - case SSL_ERROR_WANT_CONNECT: return SSL_HANDSHAKE_WANT_CONNECT; @@ -531,30 +536,18 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err) case SSL_ERROR_WANT_READ: return SSL_HANDSHAKE_WANT_READ; + + case SSL_ERROR_WANT_ACCEPT: case SSL_ERROR_WANT_X509_LOOKUP: - Debug("ssl", "SSLNetVConnection::sslServerHandShakeEvent, would block on read or write"); - break; + return EVENT_CONT; case SSL_ERROR_ZERO_RETURN: - Debug("ssl", "SSLNetVConnection::sslServerHandShakeEvent, EOS"); - return EVENT_ERROR; - break; - case SSL_ERROR_SYSCALL: - err = errno; - Debug("ssl", "SSLNetVConnection::sslServerHandShakeEvent, syscall"); - return EVENT_ERROR; - break; - case SSL_ERROR_SSL: default: - err = errno; - Debug("ssl", "SSLNetVConnection::sslServerHandShakeEvent, error"); - SSLError("SSL_ServerHandShake"); return EVENT_ERROR; - break; } - return EVENT_CONT; + } http://git-wip-us.apache.org/repos/asf/trafficserver/blob/783ce720/iocore/net/SSLUtils.cc ---------------------------------------------------------------------- diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 2cbcf77..f19ee25 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -224,7 +224,7 @@ SSLInitializeLibrary() } void -SSLError(const char * fmt, ...) +SSLDiagnostic(const SrcLoc& loc, bool debug, const char * fmt, ...) { unsigned long l; char buf[256]; @@ -234,13 +234,43 @@ SSLError(const char * fmt, ...) va_list ap; va_start(ap, fmt); - ErrorV(fmt, ap); - va_end(ap); es = CRYPTO_thread_id(); while ((l = ERR_get_error_line_data(&file, &line, &data, &flags)) != 0) { - Error("SSL::%lu:%s:%s:%d:%s", es, ERR_error_string(l, buf), file, line, (flags & ERR_TXT_STRING) ? data : ""); + if (debug) { + if (unlikely(diags->on())) { + diags->log("ssl", DL_Debug, loc.file, loc.func, loc.line, + "SSL::%lu:%s:%s:%d:%s", es, ERR_error_string(l, buf), file, line, (flags & ERR_TXT_STRING) ? data : ""); + } + } else { + diags->error(DL_Error, loc.file, loc.func, loc.line, + "SSL::%lu:%s:%s:%d:%s", es, ERR_error_string(l, buf), file, line, (flags & ERR_TXT_STRING) ? data : ""); + } } + + va_end(ap); +} + +const char * +SSLErrorName(int ssl_error) +{ + static const char * names[] = { + "SSL_ERROR_NONE", + "SSL_ERROR_SSL", + "SSL_ERROR_WANT_READ", + "SSL_ERROR_WANT_WRITE", + "SSL_ERROR_WANT_X509_LOOKUP", + "SSL_ERROR_SYSCALL", + "SSL_ERROR_ZERO_RETURN", + "SSL_ERROR_WANT_CONNECT", + "SSL_ERROR_WANT_ACCEPT" + }; + + if (ssl_error < 0 || ssl_error >= (int)countof(names)) { + return "unknown SSL error"; + } + + return names[ssl_error]; } void http://git-wip-us.apache.org/repos/asf/trafficserver/blob/783ce720/lib/ts/Diags.h ---------------------------------------------------------------------- diff --git a/lib/ts/Diags.h b/lib/ts/Diags.h index 556b693..2744355 100644 --- a/lib/ts/Diags.h +++ b/lib/ts/Diags.h @@ -100,6 +100,8 @@ struct DiagsConfigState // ////////////////////////////////////////////////////////////////////////////// +#define DiagsMakeLocation() SrcLoc(__FILE__, __FUNCTION__, __LINE__) + class SrcLoc { public: http://git-wip-us.apache.org/repos/asf/trafficserver/blob/783ce720/proxy/config/records.config.default.in ---------------------------------------------------------------------- diff --git a/proxy/config/records.config.default.in b/proxy/config/records.config.default.in index ab7c6ea..42dfd99 100644 --- a/proxy/config/records.config.default.in +++ b/proxy/config/records.config.default.in @@ -603,6 +603,8 @@ CONFIG proxy.config.diags.debug.tags STRING http.*|dns.* # ink allocators CONFIG proxy.config.dump_mem_info_frequency INT 0 + # Log the source code location of diagnostic messages. +CONFIG proxy.config.diags.show_location 0 ############################################################################## # # Configuration for Reclaimable InkFreeList memory pool
