You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2016/01/12 16:48:06 UTC
trafficserver git commit: [TS-4024] wire tracing enhancements. This
closes #337.
Repository: trafficserver
Updated Branches:
refs/heads/master 9399a7641 -> bb40d788b
[TS-4024] wire tracing enhancements. This closes #337.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/bb40d788
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/bb40d788
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/bb40d788
Branch: refs/heads/master
Commit: bb40d788bb092670b05e3d084bc14e330e3afa96
Parents: 9399a76
Author: ericcarlschwartz <es...@gmail.com>
Authored: Sat Nov 14 13:26:58 2015 -0800
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Tue Jan 12 09:44:00 2016 -0600
----------------------------------------------------------------------
iocore/net/SSLConfig.cc | 8 ++++----
iocore/net/SSLNetVConnection.cc | 39 ++++++++++++++++++++++++++----------
mgmt/RecordsConfig.cc | 8 ++++----
proxy/http/HttpSM.cc | 31 ++++++++++++++--------------
4 files changed, 52 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/iocore/net/SSLConfig.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
index 7bb60fe..63540ce 100644
--- a/iocore/net/SSLConfig.cc
+++ b/iocore/net/SSLConfig.cc
@@ -307,10 +307,10 @@ SSLConfigParams::initialize()
REC_ReadConfigInt32(ssl_allow_client_renegotiation, "proxy.config.ssl.allow_client_renegotiation");
// SSL Wire Trace configurations
- REC_ReadConfigInteger(ssl_wire_trace_enabled, "proxy.config.ssl.wire_trace_enabled");
+ REC_EstablishStaticConfigInt32(ssl_wire_trace_enabled, "proxy.config.ssl.wire_trace_enabled");
if (ssl_wire_trace_enabled) {
// wire trace specific source ip
- REC_ReadConfigStringAlloc(ssl_wire_trace_addr, "proxy.config.ssl.wire_trace_addr");
+ REC_EstablishStaticConfigStringAlloc(ssl_wire_trace_addr, "proxy.config.ssl.wire_trace_addr");
if (ssl_wire_trace_addr) {
ssl_wire_trace_ip = new IpAddr();
ssl_wire_trace_ip->load(ssl_wire_trace_addr);
@@ -318,8 +318,8 @@ SSLConfigParams::initialize()
ssl_wire_trace_ip = NULL;
}
// wire trace percentage of requests
- REC_ReadConfigInteger(ssl_wire_trace_percentage, "proxy.config.ssl.wire_trace_percentage");
- REC_ReadConfigStringAlloc(ssl_wire_trace_server_name, "proxy.config.ssl.wire_trace_server_name");
+ REC_EstablishStaticConfigInt32(ssl_wire_trace_percentage, "proxy.config.ssl.wire_trace_percentage");
+ REC_EstablishStaticConfigStringAlloc(ssl_wire_trace_server_name, "proxy.config.ssl.wire_trace_server_name");
} else {
ssl_wire_trace_addr = NULL;
ssl_wire_trace_ip = NULL;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 7199efa..dc48c63 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -286,12 +286,16 @@ ssl_read_from_net(SSLNetVConnection *sslvc, EThread *lthread, int64_t &ret)
Debug("ssl.error", "[SSL_NetVConnection::ssl_read_from_net] SSL_ERROR_ZERO_RETURN");
break;
case SSL_ERROR_SSL:
- default:
- TraceIn(trace, sslvc->get_remote_addr(), sslvc->get_remote_port(), "SSL Error: sslErr=%d, errno=%d", sslErr, errno);
+ default: {
+ char buf[512];
+ unsigned long e = ERR_peek_last_error();
+ ERR_error_string_n(e, buf, sizeof(buf));
+ TraceIn(trace, sslvc->get_remote_addr(), sslvc->get_remote_port(), "SSL Error: sslErr=%d, ERR_get_error=%ld (%s) errno=%d",
+ sslErr, e, buf, errno);
event = SSL_READ_ERROR;
ret = errno;
SSL_CLR_ERR_INCR_DYN_STAT(sslvc, ssl_error_ssl, "[SSL_NetVConnection::ssl_read_from_net]: errno=%d", errno);
- break;
+ } break;
} // switch
break;
} // while( block_write_avail > 0 )
@@ -833,11 +837,15 @@ SSLNetVConnection::load_buffer_and_write(int64_t towrite, int64_t &wattempted, i
Debug("ssl.error", "SSL_write-SSL_ERROR_ZERO_RETURN");
break;
case SSL_ERROR_SSL:
- default:
- TraceOut(trace, get_remote_addr(), get_remote_port(), "SSL Error: sslErr=%d, errno=%d", err, errno);
+ default: {
+ char buf[512];
+ unsigned long e = ERR_peek_last_error();
+ ERR_error_string_n(e, buf, sizeof(buf));
+ TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL Error: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", err, e, buf,
+ errno);
r = -errno;
SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSL_write-SSL_ERROR_SSL errno=%d", errno);
- break;
+ } break;
}
return (r);
}
@@ -1232,10 +1240,15 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err)
TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_WANT_ACCEPT");
return EVENT_CONT;
- case SSL_ERROR_SSL:
+ case SSL_ERROR_SSL: {
SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSLNetVConnection::sslServerHandShakeEvent, SSL_ERROR_SSL errno=%d", errno);
- TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_SSL");
+ char buf[512];
+ unsigned long e = ERR_peek_last_error();
+ ERR_error_string_n(e, buf, sizeof(buf));
+ TraceIn(trace, get_remote_addr(), get_remote_port(),
+ "SSL server handshake ERROR_SSL: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", ssl_error, e, buf, errno);
return EVENT_ERROR;
+ }
case SSL_ERROR_ZERO_RETURN:
TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_ZERO_RETURN");
@@ -1335,15 +1348,19 @@ SSLNetVConnection::sslClientHandShakeEvent(int &err)
case SSL_ERROR_SSL:
- default:
+ default: {
err = errno;
// FIXME -- This triggers a retry on cases of cert validation errors....
Debug("ssl", "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL");
SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL errno=%d", errno);
Debug("ssl.error", "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL");
- TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL client handshake SSL_ERROR");
+ char buf[512];
+ unsigned long e = ERR_peek_last_error();
+ ERR_error_string_n(e, buf, sizeof(buf));
+ TraceIn(trace, get_remote_addr(), get_remote_port(),
+ "SSL client handshake ERROR_SSL: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", ssl_error, e, buf, errno);
return EVENT_ERROR;
- break;
+ } break;
}
return EVENT_CONT;
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/mgmt/RecordsConfig.cc
----------------------------------------------------------------------
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index c390ac5..34e180d 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1304,13 +1304,13 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.ssl.handshake_timeout_in", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-65535]", RECA_NULL}
,
- {RECT_CONFIG, "proxy.config.ssl.wire_trace_enabled", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
+ {RECT_CONFIG, "proxy.config.ssl.wire_trace_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
,
- {RECT_CONFIG, "proxy.config.ssl.wire_trace_addr", RECD_STRING, NULL , RECU_RESTART_TS, RR_NULL, RECC_IP, "[0-255]\\.[0-255]\\.[0-255]\\.[0-255]", RECA_NULL}
+ {RECT_CONFIG, "proxy.config.ssl.wire_trace_addr", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_IP, "[0-255]\\.[0-255]\\.[0-255]\\.[0-255]", RECA_NULL}
,
- {RECT_CONFIG, "proxy.config.ssl.wire_trace_percentage", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-100]", RECA_NULL}
+ {RECT_CONFIG, "proxy.config.ssl.wire_trace_percentage", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-100]", RECA_NULL}
,
- {RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_RESTART_TS, RR_NULL, RECC_STR, ".*", RECA_NULL}
+ {RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_STR, ".*", RECA_NULL}
,
//##############################################################################
//#
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 5fae64c..ab97380 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -5636,24 +5636,25 @@ HttpSM::attach_server_session(HttpServerSession *s)
// es - is this a concern here in HttpSM? Does it belong somewhere else?
// Get server and client connections
- UnixNetVConnection *server_vc = (UnixNetVConnection *)(server_session->get_netvc());
+ UnixNetVConnection *server_vc = dynamic_cast<UnixNetVConnection *>(server_session->get_netvc());
UnixNetVConnection *client_vc = (UnixNetVConnection *)(ua_session->get_netvc());
SSLNetVConnection *ssl_vc = dynamic_cast<SSLNetVConnection *>(client_vc);
- if (ssl_vc != NULL) { // if incoming connection is SSL
- bool client_trace = ssl_vc->getSSLTrace();
- if (client_trace) {
- // get remote address and port to mark corresponding traces
- const sockaddr *remote_addr = ssl_vc->get_remote_addr();
- uint16_t remote_port = ssl_vc->get_remote_port();
- server_vc->setOriginTrace(true);
- server_vc->setOriginTraceAddr(remote_addr);
- server_vc->setOriginTracePort(remote_port);
- } else {
- server_vc->setOriginTrace(false);
- server_vc->setOriginTraceAddr(NULL);
- server_vc->setOriginTracePort(0);
+ bool associated_connection = false;
+ if (server_vc) { // if server_vc isn't a PluginVC
+ if (ssl_vc) { // if incoming connection is SSL
+ bool client_trace = ssl_vc->getSSLTrace();
+ if (client_trace) {
+ // get remote address and port to mark corresponding traces
+ const sockaddr *remote_addr = ssl_vc->get_remote_addr();
+ uint16_t remote_port = ssl_vc->get_remote_port();
+ server_vc->setOriginTrace(true);
+ server_vc->setOriginTraceAddr(remote_addr);
+ server_vc->setOriginTracePort(remote_port);
+ associated_connection = true;
+ }
}
- } else {
+ }
+ if (!associated_connection && server_vc) {
server_vc->setOriginTrace(false);
server_vc->setOriginTraceAddr(NULL);
server_vc->setOriginTracePort(0);
Re: trafficserver git commit: [TS-4024] wire tracing enhancements. This closes #337.
Posted by James Peach <jp...@apache.org>.
This doesn't look like it addresses Alan's last comment about "unifying the SSL error reporting logic"
> On Jan 12, 2016, at 7:48 AM, shinrich@apache.org wrote:
>
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 9399a7641 -> bb40d788b
>
>
> [TS-4024] wire tracing enhancements. This closes #337.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/bb40d788
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/bb40d788
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/bb40d788
>
> Branch: refs/heads/master
> Commit: bb40d788bb092670b05e3d084bc14e330e3afa96
> Parents: 9399a76
> Author: ericcarlschwartz <es...@gmail.com>
> Authored: Sat Nov 14 13:26:58 2015 -0800
> Committer: shinrich <sh...@yahoo-inc.com>
> Committed: Tue Jan 12 09:44:00 2016 -0600
>
> ----------------------------------------------------------------------
> iocore/net/SSLConfig.cc | 8 ++++----
> iocore/net/SSLNetVConnection.cc | 39 ++++++++++++++++++++++++++----------
> mgmt/RecordsConfig.cc | 8 ++++----
> proxy/http/HttpSM.cc | 31 ++++++++++++++--------------
> 4 files changed, 52 insertions(+), 34 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/iocore/net/SSLConfig.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
> index 7bb60fe..63540ce 100644
> --- a/iocore/net/SSLConfig.cc
> +++ b/iocore/net/SSLConfig.cc
> @@ -307,10 +307,10 @@ SSLConfigParams::initialize()
> REC_ReadConfigInt32(ssl_allow_client_renegotiation, "proxy.config.ssl.allow_client_renegotiation");
>
> // SSL Wire Trace configurations
> - REC_ReadConfigInteger(ssl_wire_trace_enabled, "proxy.config.ssl.wire_trace_enabled");
> + REC_EstablishStaticConfigInt32(ssl_wire_trace_enabled, "proxy.config.ssl.wire_trace_enabled");
> if (ssl_wire_trace_enabled) {
> // wire trace specific source ip
> - REC_ReadConfigStringAlloc(ssl_wire_trace_addr, "proxy.config.ssl.wire_trace_addr");
> + REC_EstablishStaticConfigStringAlloc(ssl_wire_trace_addr, "proxy.config.ssl.wire_trace_addr");
> if (ssl_wire_trace_addr) {
> ssl_wire_trace_ip = new IpAddr();
> ssl_wire_trace_ip->load(ssl_wire_trace_addr);
> @@ -318,8 +318,8 @@ SSLConfigParams::initialize()
> ssl_wire_trace_ip = NULL;
> }
> // wire trace percentage of requests
> - REC_ReadConfigInteger(ssl_wire_trace_percentage, "proxy.config.ssl.wire_trace_percentage");
> - REC_ReadConfigStringAlloc(ssl_wire_trace_server_name, "proxy.config.ssl.wire_trace_server_name");
> + REC_EstablishStaticConfigInt32(ssl_wire_trace_percentage, "proxy.config.ssl.wire_trace_percentage");
> + REC_EstablishStaticConfigStringAlloc(ssl_wire_trace_server_name, "proxy.config.ssl.wire_trace_server_name");
> } else {
> ssl_wire_trace_addr = NULL;
> ssl_wire_trace_ip = NULL;
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/iocore/net/SSLNetVConnection.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
> index 7199efa..dc48c63 100644
> --- a/iocore/net/SSLNetVConnection.cc
> +++ b/iocore/net/SSLNetVConnection.cc
> @@ -286,12 +286,16 @@ ssl_read_from_net(SSLNetVConnection *sslvc, EThread *lthread, int64_t &ret)
> Debug("ssl.error", "[SSL_NetVConnection::ssl_read_from_net] SSL_ERROR_ZERO_RETURN");
> break;
> case SSL_ERROR_SSL:
> - default:
> - TraceIn(trace, sslvc->get_remote_addr(), sslvc->get_remote_port(), "SSL Error: sslErr=%d, errno=%d", sslErr, errno);
> + default: {
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, sslvc->get_remote_addr(), sslvc->get_remote_port(), "SSL Error: sslErr=%d, ERR_get_error=%ld (%s) errno=%d",
> + sslErr, e, buf, errno);
> event = SSL_READ_ERROR;
> ret = errno;
> SSL_CLR_ERR_INCR_DYN_STAT(sslvc, ssl_error_ssl, "[SSL_NetVConnection::ssl_read_from_net]: errno=%d", errno);
> - break;
> + } break;
> } // switch
> break;
> } // while( block_write_avail > 0 )
> @@ -833,11 +837,15 @@ SSLNetVConnection::load_buffer_and_write(int64_t towrite, int64_t &wattempted, i
> Debug("ssl.error", "SSL_write-SSL_ERROR_ZERO_RETURN");
> break;
> case SSL_ERROR_SSL:
> - default:
> - TraceOut(trace, get_remote_addr(), get_remote_port(), "SSL Error: sslErr=%d, errno=%d", err, errno);
> + default: {
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL Error: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", err, e, buf,
> + errno);
> r = -errno;
> SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSL_write-SSL_ERROR_SSL errno=%d", errno);
> - break;
> + } break;
> }
> return (r);
> }
> @@ -1232,10 +1240,15 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err)
> TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_WANT_ACCEPT");
> return EVENT_CONT;
>
> - case SSL_ERROR_SSL:
> + case SSL_ERROR_SSL: {
> SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSLNetVConnection::sslServerHandShakeEvent, SSL_ERROR_SSL errno=%d", errno);
> - TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_SSL");
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, get_remote_addr(), get_remote_port(),
> + "SSL server handshake ERROR_SSL: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", ssl_error, e, buf, errno);
> return EVENT_ERROR;
> + }
>
> case SSL_ERROR_ZERO_RETURN:
> TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_ZERO_RETURN");
> @@ -1335,15 +1348,19 @@ SSLNetVConnection::sslClientHandShakeEvent(int &err)
>
>
> case SSL_ERROR_SSL:
> - default:
> + default: {
> err = errno;
> // FIXME -- This triggers a retry on cases of cert validation errors....
> Debug("ssl", "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL");
> SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL errno=%d", errno);
> Debug("ssl.error", "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL");
> - TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL client handshake SSL_ERROR");
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, get_remote_addr(), get_remote_port(),
> + "SSL client handshake ERROR_SSL: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", ssl_error, e, buf, errno);
> return EVENT_ERROR;
> - break;
> + } break;
> }
> return EVENT_CONT;
> }
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/mgmt/RecordsConfig.cc
> ----------------------------------------------------------------------
> diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
> index c390ac5..34e180d 100644
> --- a/mgmt/RecordsConfig.cc
> +++ b/mgmt/RecordsConfig.cc
> @@ -1304,13 +1304,13 @@ static const RecordElement RecordsConfig[] =
> ,
> {RECT_CONFIG, "proxy.config.ssl.handshake_timeout_in", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-65535]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_enabled", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_addr", RECD_STRING, NULL , RECU_RESTART_TS, RR_NULL, RECC_IP, "[0-255]\\.[0-255]\\.[0-255]\\.[0-255]", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_addr", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_IP, "[0-255]\\.[0-255]\\.[0-255]\\.[0-255]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_percentage", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-100]", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_percentage", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-100]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_RESTART_TS, RR_NULL, RECC_STR, ".*", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_STR, ".*", RECA_NULL}
> ,
> //##############################################################################
> //#
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/proxy/http/HttpSM.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
> index 5fae64c..ab97380 100644
> --- a/proxy/http/HttpSM.cc
> +++ b/proxy/http/HttpSM.cc
> @@ -5636,24 +5636,25 @@ HttpSM::attach_server_session(HttpServerSession *s)
>
> // es - is this a concern here in HttpSM? Does it belong somewhere else?
> // Get server and client connections
> - UnixNetVConnection *server_vc = (UnixNetVConnection *)(server_session->get_netvc());
> + UnixNetVConnection *server_vc = dynamic_cast<UnixNetVConnection *>(server_session->get_netvc());
> UnixNetVConnection *client_vc = (UnixNetVConnection *)(ua_session->get_netvc());
> SSLNetVConnection *ssl_vc = dynamic_cast<SSLNetVConnection *>(client_vc);
> - if (ssl_vc != NULL) { // if incoming connection is SSL
> - bool client_trace = ssl_vc->getSSLTrace();
> - if (client_trace) {
> - // get remote address and port to mark corresponding traces
> - const sockaddr *remote_addr = ssl_vc->get_remote_addr();
> - uint16_t remote_port = ssl_vc->get_remote_port();
> - server_vc->setOriginTrace(true);
> - server_vc->setOriginTraceAddr(remote_addr);
> - server_vc->setOriginTracePort(remote_port);
> - } else {
> - server_vc->setOriginTrace(false);
> - server_vc->setOriginTraceAddr(NULL);
> - server_vc->setOriginTracePort(0);
> + bool associated_connection = false;
> + if (server_vc) { // if server_vc isn't a PluginVC
> + if (ssl_vc) { // if incoming connection is SSL
> + bool client_trace = ssl_vc->getSSLTrace();
> + if (client_trace) {
> + // get remote address and port to mark corresponding traces
> + const sockaddr *remote_addr = ssl_vc->get_remote_addr();
> + uint16_t remote_port = ssl_vc->get_remote_port();
> + server_vc->setOriginTrace(true);
> + server_vc->setOriginTraceAddr(remote_addr);
> + server_vc->setOriginTracePort(remote_port);
> + associated_connection = true;
> + }
> }
> - } else {
> + }
> + if (!associated_connection && server_vc) {
> server_vc->setOriginTrace(false);
> server_vc->setOriginTraceAddr(NULL);
> server_vc->setOriginTracePort(0);
>
Re: trafficserver git commit: [TS-4024] wire tracing enhancements. This closes #337.
Posted by James Peach <jp...@apache.org>.
This doesn't look like it addresses Alan's last comment about "unifying the SSL error reporting logic"
> On Jan 12, 2016, at 7:48 AM, shinrich@apache.org wrote:
>
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 9399a7641 -> bb40d788b
>
>
> [TS-4024] wire tracing enhancements. This closes #337.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/bb40d788
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/bb40d788
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/bb40d788
>
> Branch: refs/heads/master
> Commit: bb40d788bb092670b05e3d084bc14e330e3afa96
> Parents: 9399a76
> Author: ericcarlschwartz <es...@gmail.com>
> Authored: Sat Nov 14 13:26:58 2015 -0800
> Committer: shinrich <sh...@yahoo-inc.com>
> Committed: Tue Jan 12 09:44:00 2016 -0600
>
> ----------------------------------------------------------------------
> iocore/net/SSLConfig.cc | 8 ++++----
> iocore/net/SSLNetVConnection.cc | 39 ++++++++++++++++++++++++++----------
> mgmt/RecordsConfig.cc | 8 ++++----
> proxy/http/HttpSM.cc | 31 ++++++++++++++--------------
> 4 files changed, 52 insertions(+), 34 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/iocore/net/SSLConfig.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
> index 7bb60fe..63540ce 100644
> --- a/iocore/net/SSLConfig.cc
> +++ b/iocore/net/SSLConfig.cc
> @@ -307,10 +307,10 @@ SSLConfigParams::initialize()
> REC_ReadConfigInt32(ssl_allow_client_renegotiation, "proxy.config.ssl.allow_client_renegotiation");
>
> // SSL Wire Trace configurations
> - REC_ReadConfigInteger(ssl_wire_trace_enabled, "proxy.config.ssl.wire_trace_enabled");
> + REC_EstablishStaticConfigInt32(ssl_wire_trace_enabled, "proxy.config.ssl.wire_trace_enabled");
> if (ssl_wire_trace_enabled) {
> // wire trace specific source ip
> - REC_ReadConfigStringAlloc(ssl_wire_trace_addr, "proxy.config.ssl.wire_trace_addr");
> + REC_EstablishStaticConfigStringAlloc(ssl_wire_trace_addr, "proxy.config.ssl.wire_trace_addr");
> if (ssl_wire_trace_addr) {
> ssl_wire_trace_ip = new IpAddr();
> ssl_wire_trace_ip->load(ssl_wire_trace_addr);
> @@ -318,8 +318,8 @@ SSLConfigParams::initialize()
> ssl_wire_trace_ip = NULL;
> }
> // wire trace percentage of requests
> - REC_ReadConfigInteger(ssl_wire_trace_percentage, "proxy.config.ssl.wire_trace_percentage");
> - REC_ReadConfigStringAlloc(ssl_wire_trace_server_name, "proxy.config.ssl.wire_trace_server_name");
> + REC_EstablishStaticConfigInt32(ssl_wire_trace_percentage, "proxy.config.ssl.wire_trace_percentage");
> + REC_EstablishStaticConfigStringAlloc(ssl_wire_trace_server_name, "proxy.config.ssl.wire_trace_server_name");
> } else {
> ssl_wire_trace_addr = NULL;
> ssl_wire_trace_ip = NULL;
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/iocore/net/SSLNetVConnection.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
> index 7199efa..dc48c63 100644
> --- a/iocore/net/SSLNetVConnection.cc
> +++ b/iocore/net/SSLNetVConnection.cc
> @@ -286,12 +286,16 @@ ssl_read_from_net(SSLNetVConnection *sslvc, EThread *lthread, int64_t &ret)
> Debug("ssl.error", "[SSL_NetVConnection::ssl_read_from_net] SSL_ERROR_ZERO_RETURN");
> break;
> case SSL_ERROR_SSL:
> - default:
> - TraceIn(trace, sslvc->get_remote_addr(), sslvc->get_remote_port(), "SSL Error: sslErr=%d, errno=%d", sslErr, errno);
> + default: {
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, sslvc->get_remote_addr(), sslvc->get_remote_port(), "SSL Error: sslErr=%d, ERR_get_error=%ld (%s) errno=%d",
> + sslErr, e, buf, errno);
> event = SSL_READ_ERROR;
> ret = errno;
> SSL_CLR_ERR_INCR_DYN_STAT(sslvc, ssl_error_ssl, "[SSL_NetVConnection::ssl_read_from_net]: errno=%d", errno);
> - break;
> + } break;
> } // switch
> break;
> } // while( block_write_avail > 0 )
> @@ -833,11 +837,15 @@ SSLNetVConnection::load_buffer_and_write(int64_t towrite, int64_t &wattempted, i
> Debug("ssl.error", "SSL_write-SSL_ERROR_ZERO_RETURN");
> break;
> case SSL_ERROR_SSL:
> - default:
> - TraceOut(trace, get_remote_addr(), get_remote_port(), "SSL Error: sslErr=%d, errno=%d", err, errno);
> + default: {
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL Error: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", err, e, buf,
> + errno);
> r = -errno;
> SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSL_write-SSL_ERROR_SSL errno=%d", errno);
> - break;
> + } break;
> }
> return (r);
> }
> @@ -1232,10 +1240,15 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err)
> TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_WANT_ACCEPT");
> return EVENT_CONT;
>
> - case SSL_ERROR_SSL:
> + case SSL_ERROR_SSL: {
> SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSLNetVConnection::sslServerHandShakeEvent, SSL_ERROR_SSL errno=%d", errno);
> - TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_SSL");
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, get_remote_addr(), get_remote_port(),
> + "SSL server handshake ERROR_SSL: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", ssl_error, e, buf, errno);
> return EVENT_ERROR;
> + }
>
> case SSL_ERROR_ZERO_RETURN:
> TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL server handshake ERROR_ZERO_RETURN");
> @@ -1335,15 +1348,19 @@ SSLNetVConnection::sslClientHandShakeEvent(int &err)
>
>
> case SSL_ERROR_SSL:
> - default:
> + default: {
> err = errno;
> // FIXME -- This triggers a retry on cases of cert validation errors....
> Debug("ssl", "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL");
> SSL_CLR_ERR_INCR_DYN_STAT(this, ssl_error_ssl, "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL errno=%d", errno);
> Debug("ssl.error", "SSLNetVConnection::sslClientHandShakeEvent, SSL_ERROR_SSL");
> - TraceIn(trace, get_remote_addr(), get_remote_port(), "SSL client handshake SSL_ERROR");
> + char buf[512];
> + unsigned long e = ERR_peek_last_error();
> + ERR_error_string_n(e, buf, sizeof(buf));
> + TraceIn(trace, get_remote_addr(), get_remote_port(),
> + "SSL client handshake ERROR_SSL: sslErr=%d, ERR_get_error=%ld (%s) errno=%d", ssl_error, e, buf, errno);
> return EVENT_ERROR;
> - break;
> + } break;
> }
> return EVENT_CONT;
> }
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/mgmt/RecordsConfig.cc
> ----------------------------------------------------------------------
> diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
> index c390ac5..34e180d 100644
> --- a/mgmt/RecordsConfig.cc
> +++ b/mgmt/RecordsConfig.cc
> @@ -1304,13 +1304,13 @@ static const RecordElement RecordsConfig[] =
> ,
> {RECT_CONFIG, "proxy.config.ssl.handshake_timeout_in", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-65535]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_enabled", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-2]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_addr", RECD_STRING, NULL , RECU_RESTART_TS, RR_NULL, RECC_IP, "[0-255]\\.[0-255]\\.[0-255]\\.[0-255]", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_addr", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_IP, "[0-255]\\.[0-255]\\.[0-255]\\.[0-255]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_percentage", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-100]", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_percentage", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-100]", RECA_NULL}
> ,
> - {RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_RESTART_TS, RR_NULL, RECC_STR, ".*", RECA_NULL}
> + {RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_STR, ".*", RECA_NULL}
> ,
> //##############################################################################
> //#
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/bb40d788/proxy/http/HttpSM.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
> index 5fae64c..ab97380 100644
> --- a/proxy/http/HttpSM.cc
> +++ b/proxy/http/HttpSM.cc
> @@ -5636,24 +5636,25 @@ HttpSM::attach_server_session(HttpServerSession *s)
>
> // es - is this a concern here in HttpSM? Does it belong somewhere else?
> // Get server and client connections
> - UnixNetVConnection *server_vc = (UnixNetVConnection *)(server_session->get_netvc());
> + UnixNetVConnection *server_vc = dynamic_cast<UnixNetVConnection *>(server_session->get_netvc());
> UnixNetVConnection *client_vc = (UnixNetVConnection *)(ua_session->get_netvc());
> SSLNetVConnection *ssl_vc = dynamic_cast<SSLNetVConnection *>(client_vc);
> - if (ssl_vc != NULL) { // if incoming connection is SSL
> - bool client_trace = ssl_vc->getSSLTrace();
> - if (client_trace) {
> - // get remote address and port to mark corresponding traces
> - const sockaddr *remote_addr = ssl_vc->get_remote_addr();
> - uint16_t remote_port = ssl_vc->get_remote_port();
> - server_vc->setOriginTrace(true);
> - server_vc->setOriginTraceAddr(remote_addr);
> - server_vc->setOriginTracePort(remote_port);
> - } else {
> - server_vc->setOriginTrace(false);
> - server_vc->setOriginTraceAddr(NULL);
> - server_vc->setOriginTracePort(0);
> + bool associated_connection = false;
> + if (server_vc) { // if server_vc isn't a PluginVC
> + if (ssl_vc) { // if incoming connection is SSL
> + bool client_trace = ssl_vc->getSSLTrace();
> + if (client_trace) {
> + // get remote address and port to mark corresponding traces
> + const sockaddr *remote_addr = ssl_vc->get_remote_addr();
> + uint16_t remote_port = ssl_vc->get_remote_port();
> + server_vc->setOriginTrace(true);
> + server_vc->setOriginTraceAddr(remote_addr);
> + server_vc->setOriginTracePort(remote_port);
> + associated_connection = true;
> + }
> }
> - } else {
> + }
> + if (!associated_connection && server_vc) {
> server_vc->setOriginTrace(false);
> server_vc->setOriginTraceAddr(NULL);
> server_vc->setOriginTracePort(0);
>