Updated Branches: refs/heads/master 3f2b11310 -> aeea92c23
TS-2096: improve SSL certificate loading error messages We were passing a format string to SSLDiagnostic(), but never actually using it when we logged the error. Improve the ssl_multicert.config error logging so that we log the file and line of where we detected an error. Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/aeea92c2 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/aeea92c2 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/aeea92c2 Branch: refs/heads/master Commit: aeea92c2344dbb9efc091d528dc2dc3baa74a393 Parents: 3f2b113 Author: James Peach <[email protected]> Authored: Sat Aug 10 20:31:10 2013 -0700 Committer: James Peach <[email protected]> Committed: Sat Aug 10 20:31:10 2013 -0700 ---------------------------------------------------------------------- CHANGES | 2 ++ iocore/net/SSLUtils.cc | 28 +++++++++++++++++++--------- lib/ts/Diags.cc | 2 +- lib/ts/Diags.h | 4 ++-- 4 files changed, 24 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/aeea92c2/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index e64dabc..8448ec5 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,8 @@ Changes with Apache Traffic Server 3.5.0 + *)[ TS-2096] improve SSL certificate loading error messages + *) [TS-2129] Check for existence of ExtUtils::MakeMaker. *) [TS-2128] Don't link libGeoIP.so.1 into binaries http://git-wip-us.apache.org/repos/asf/trafficserver/blob/aeea92c2/iocore/net/SSLUtils.cc ---------------------------------------------------------------------- diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 4c8cf2e..8c4b888 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -172,8 +172,8 @@ static SSL_CTX * ssl_context_enable_sni(SSL_CTX * ctx, SSLCertLookup * lookup) { #if TS_USE_TLS_SNI - Debug("ssl", "setting SNI callbacks with for ctx %p", ctx); if (ctx) { + Debug("ssl", "setting SNI callbacks with for ctx %p", ctx); SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback); SSL_CTX_set_tlsext_servername_arg(ctx, lookup); } @@ -217,22 +217,30 @@ SSLDiagnostic(const SrcLoc& loc, bool debug, const char * fmt, ...) unsigned long es; va_list ap; - va_start(ap, fmt); es = CRYPTO_thread_id(); while ((l = ERR_get_error_line_data(&file, &line, &data, &flags)) != 0) { 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 : ""); + "SSL::%lu:%s:%s:%d%s%s", es, ERR_error_string(l, buf), file, line, + (flags & ERR_TXT_STRING) ? ":" : "", (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 : ""); + "SSL::%lu:%s:%s:%d%s%s", es, ERR_error_string(l, buf), file, line, + (flags & ERR_TXT_STRING) ? ":" : "", (flags & ERR_TXT_STRING) ? data : ""); } } + va_start(ap, fmt); + if (debug) { + diags->log_va("ssl", DL_Debug, &loc, fmt, ap); + } else { + diags->error_va(DL_Error, loc.file, loc.func, loc.line, fmt, ap); + } va_end(ap); + } const char * @@ -533,7 +541,7 @@ ssl_index_certificate(SSLCertLookup * lookup, SSL_CTX * ctx, const char * certfi X509_free(cert); } -static void +static bool ssl_store_ssl_context( const SSLConfigParams * params, SSLCertLookup * lookup, @@ -547,8 +555,7 @@ ssl_store_ssl_context( ctx = ssl_context_enable_sni(SSLInitServerContext(params, cert, ca, key), lookup); if (!ctx) { - SSLError("failed to create new SSL server context"); - return; + return false; } #if TS_USE_TLS_NPN @@ -578,6 +585,7 @@ ssl_store_ssl_context( // this code is updated to reconfigure the SSL certificates, it will need some sort of // refcounting or alternate way of avoiding double frees. ssl_index_certificate(lookup, ctx, certpath); + return true; } static bool @@ -678,7 +686,10 @@ SSLParseCertificateConfiguration( REC_SignalError(errBuf, alarmAlready); } else { if (ssl_extract_certificate(&line_info, addr, cert, ca, key)) { - ssl_store_ssl_context(params, lookup, addr, cert, ca, key); + if (!ssl_store_ssl_context(params, lookup, addr, cert, ca, key)) { + Error("failed to load SSL certificate specification from %s line %u", + params->configFilePath, line_num); + } } else { snprintf(errBuf, sizeof(errBuf), "%s: discarding invalid %s entry at line %u", __func__, params->configFilePath, line_num); @@ -701,4 +712,3 @@ SSLParseCertificateConfiguration( return true; } - http://git-wip-us.apache.org/repos/asf/trafficserver/blob/aeea92c2/lib/ts/Diags.cc ---------------------------------------------------------------------- diff --git a/lib/ts/Diags.cc b/lib/ts/Diags.cc index 3ccd355..ba4e105 100644 --- a/lib/ts/Diags.cc +++ b/lib/ts/Diags.cc @@ -185,7 +185,7 @@ Diags::~Diags() ////////////////////////////////////////////////////////////////////////////// void -Diags::print_va(const char *debug_tag, DiagsLevel diags_level ,SrcLoc *loc, +Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SrcLoc *loc, const char *format_string, va_list ap) const { struct timeval tp; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/aeea92c2/lib/ts/Diags.h ---------------------------------------------------------------------- diff --git a/lib/ts/Diags.h b/lib/ts/Diags.h index 2744355..bad36c5 100644 --- a/lib/ts/Diags.h +++ b/lib/ts/Diags.h @@ -174,7 +174,7 @@ public: const char *level_name(DiagsLevel dl) const; inkcoreapi void print_va(const char *tag, DiagsLevel dl, - SrcLoc *loc, const char *format_string, va_list ap) const; + const SrcLoc *loc, const char *format_string, va_list ap) const; ////////////////////////////// @@ -200,7 +200,7 @@ public: // on the value of the enable flag, and the state of the debug tags. // /////////////////////////////////////////////////////////////////////// - void log_va(const char *tag, DiagsLevel dl, SrcLoc * loc, const char *format_string, va_list ap) + void log_va(const char *tag, DiagsLevel dl, const SrcLoc * loc, const char *format_string, va_list ap) { if (!on(tag)) return;
