You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2017/03/27 20:16:21 UTC
[trafficserver] branch 7.1.x updated (369db08 -> 317738e)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.
from 369db08 Insure that parent health stats are updated properly on a markdown when the retry window has elapsed. This avoids premature markdowns.
new 712eedb Need dedicated TS_SSL_SERVERNAME_HOOK
new 317738e Fix ssl hook state logic.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "adds" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../api/functions/TSVConnTunnel.en.rst | 5 +---
doc/developer-guide/api/types/TSHttpHookID.en.rst | 4 ++++
iocore/net/SSLNetVConnection.cc | 27 ++++++++++++++-------
iocore/net/SSLUtils.cc | 28 +++++++++++++++-------
lib/ts/apidefs.h.in | 6 +++--
proxy/InkAPIInternal.h | 1 +
proxy/InkAPITest.cc | 3 ++-
proxy/http/HttpDebugNames.cc | 2 ++
8 files changed, 52 insertions(+), 24 deletions(-)
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].
[trafficserver] 02/02: Fix ssl hook state logic.
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 317738edbcb6622aca949892f416f410c9801388
Author: Susan Hinrichs <sh...@ieee.org>
AuthorDate: Tue Mar 7 19:04:12 2017 +0000
Fix ssl hook state logic.
Was not correctly resetting the ssl hook state after the servername hooks.
---
iocore/net/SSLNetVConnection.cc | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 6f2784c..197b5df 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -1465,12 +1465,15 @@ SSLNetVConnection::callHooks(TSEvent eventId)
this->sslHandshakeHookState = HANDSHAKE_HOOKS_INVOKE;
curHook->invoke(eventId, this);
reenabled = eventId != TS_EVENT_SSL_CERT || (this->sslHandshakeHookState != HANDSHAKE_HOOKS_INVOKE);
- } else {
- // no SNI-Hooks set, set state to HOOKS_DONE
- // no plugins registered for this hook, return (reenabled == true)
+ }
+
+ // All done with the current hook chain
+ if (curHook == nullptr) {
if (eventId == TS_EVENT_SSL_CERT) {
+ // Set the HookState to done because we are all done with the CERT/SERVERNAME hook chains
sslHandshakeHookState = HANDSHAKE_HOOKS_DONE;
} else if (eventId == TS_EVENT_SSL_SERVERNAME) {
+ // Reset the HookState to PRE, so the cert hook chain can start
sslHandshakeHookState = HANDSHAKE_HOOKS_PRE;
}
}
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.
[trafficserver] 01/02: Need dedicated TS_SSL_SERVERNAME_HOOK
Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 712eedb1dbd52cbb941625db807ab778852a7ef4
Author: Susan Hinrichs <sh...@ieee.org>
AuthorDate: Sat Feb 11 00:35:01 2017 +0000
Need dedicated TS_SSL_SERVERNAME_HOOK
If you have a plugin that needs to trigger on every TLS client hello, this won't
work the TS_SSL_SNI_HOOK with openssl-1.0.2. In that version TS_SSL_SNI_HOOK
is shared with the TS_SSL_CERT_HOOK which does not trigger on session reuse.
I propose adding a dedicated TS_SSL_SERVERNAME_HOOK and deprecate the
TS_SSL_SNI_HOOK to drop on the next major release
---
.../api/functions/TSVConnTunnel.en.rst | 5 +---
doc/developer-guide/api/types/TSHttpHookID.en.rst | 4 ++++
iocore/net/SSLNetVConnection.cc | 18 ++++++++++----
iocore/net/SSLUtils.cc | 28 +++++++++++++++-------
lib/ts/apidefs.h.in | 6 +++--
proxy/InkAPIInternal.h | 1 +
proxy/InkAPITest.cc | 3 ++-
proxy/http/HttpDebugNames.cc | 2 ++
8 files changed, 46 insertions(+), 21 deletions(-)
diff --git a/doc/developer-guide/api/functions/TSVConnTunnel.en.rst b/doc/developer-guide/api/functions/TSVConnTunnel.en.rst
index 0669b9a..85fa184 100644
--- a/doc/developer-guide/api/functions/TSVConnTunnel.en.rst
+++ b/doc/developer-guide/api/functions/TSVConnTunnel.en.rst
@@ -32,8 +32,5 @@ Description
===========
Set the SSL connection :arg:`svc` to convert to a blind tunnel. Can be called
-from :member:`TS_VCONN_PRE_ACCEPT_HOOK` or :member:`TS_SSL_SNI_HOOK` / :member:`TS_SSL_CERT_HOOK`.
+from :member:`TS_VCONN_PRE_ACCEPT_HOOK`, :member:`TS_SSL_SERVERNAME_HOOK`, or :member:`TS_SSL_SNI_HOOK` / :member:`TS_SSL_CERT_HOOK`.
-For this to work from the :member:`TS_SSL_SNI_HOOK` or :member:`TS_SSL_CERT_HOOK`,
-either the server must be running OpenSSL 1.0.2 or a version of OpenSSL 1.0.1
-with the appropriate patch installed.
diff --git a/doc/developer-guide/api/types/TSHttpHookID.en.rst b/doc/developer-guide/api/types/TSHttpHookID.en.rst
index 6ebdcae..0520931 100644
--- a/doc/developer-guide/api/types/TSHttpHookID.en.rst
+++ b/doc/developer-guide/api/types/TSHttpHookID.en.rst
@@ -74,6 +74,8 @@ Enumeration Members
.. c:member:: TSHttpHookID TS_SSL_CERT_HOOK
+.. c:member:: TSHttpHookID TS_SSL_SERVERNAME_HOOK
+
.. c:member:: TSHttpHookID TS_SSL_LAST_HOOK
.. c:member:: TSHttpHookID TS_HTTP_LAST_HOOK
@@ -81,3 +83,5 @@ Enumeration Members
Description
===========
+Note that :member:`TS_SSL_CERT_HOOK` and :member:`TS_SSL_SNI_HOOK` hook the same openssl callbacks.
+In openssl 1.0.2 and beyond :member:`TS_SSL_SERVERNAME_HOOK` is involved only for the openssl servername callback. :member:`TS_SSL_SNI_HOOK` and :member:`TS_SSL_CERT_HOOK` are called for the openssl certificate callback.
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index c543675..6f2784c 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -1438,17 +1438,22 @@ bool
SSLNetVConnection::callHooks(TSEvent eventId)
{
// Only dealing with the SNI/CERT hook so far.
- ink_assert(eventId == TS_EVENT_SSL_CERT);
+ ink_assert(eventId == TS_EVENT_SSL_CERT || eventId == TS_EVENT_SSL_SERVERNAME);
Debug("ssl", "callHooks sslHandshakeHookState=%d", this->sslHandshakeHookState);
// First time through, set the type of the hook that is currently being invoked
- if (HANDSHAKE_HOOKS_PRE == sslHandshakeHookState) {
+ if ((this->sslHandshakeHookState == HANDSHAKE_HOOKS_PRE || this->sslHandshakeHookState == HANDSHAKE_HOOKS_DONE) &&
+ eventId == TS_EVENT_SSL_CERT) {
// the previous hook should be DONE and set curHook to nullptr before trigger the sni hook.
ink_assert(curHook == nullptr);
// set to HOOKS_CERT means CERT/SNI hooks has called by SSL_accept()
this->sslHandshakeHookState = HANDSHAKE_HOOKS_CERT;
// get Hooks
curHook = ssl_hooks->get(TS_SSL_CERT_INTERNAL_HOOK);
+ } else if (eventId == TS_EVENT_SSL_SERVERNAME) {
+ if (!curHook) {
+ curHook = ssl_hooks->get(TS_SSL_SERVERNAME_INTERNAL_HOOK);
+ }
} else {
// Not in the right state
// reenable and continue
@@ -1457,14 +1462,17 @@ SSLNetVConnection::callHooks(TSEvent eventId)
bool reenabled = true;
if (curHook != nullptr) {
- // Otherwise, we have plugin hooks to run
this->sslHandshakeHookState = HANDSHAKE_HOOKS_INVOKE;
curHook->invoke(eventId, this);
- reenabled = (this->sslHandshakeHookState != HANDSHAKE_HOOKS_INVOKE);
+ reenabled = eventId != TS_EVENT_SSL_CERT || (this->sslHandshakeHookState != HANDSHAKE_HOOKS_INVOKE);
} else {
// no SNI-Hooks set, set state to HOOKS_DONE
// no plugins registered for this hook, return (reenabled == true)
- sslHandshakeHookState = HANDSHAKE_HOOKS_DONE;
+ if (eventId == TS_EVENT_SSL_CERT) {
+ sslHandshakeHookState = HANDSHAKE_HOOKS_DONE;
+ } else if (eventId == TS_EVENT_SSL_SERVERNAME) {
+ sslHandshakeHookState = HANDSHAKE_HOOKS_PRE;
+ }
}
return reenabled;
}
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 01268ad..9f065e7 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -402,9 +402,21 @@ ssl_cert_callback(SSL *ssl, void * /*arg*/)
// Return 1 for success, 0 for error, or -1 to pause
return retval;
}
+
+/*
+ * Cannot stop this callback. Always reeneabled
+ */
+static int
+ssl_servername_only_callback(SSL *ssl, int * /* ad */, void * /*arg*/)
+{
+ SSLNetVConnection *netvc = (SSLNetVConnection *)SSL_get_app_data(ssl);
+ netvc->callHooks(TS_EVENT_SSL_SERVERNAME);
+ return SSL_TLSEXT_ERR_OK;
+}
+
#else
static int
-ssl_servername_callback(SSL *ssl, int * /* ad */, void * /*arg*/)
+ssl_servername_and_cert_callback(SSL *ssl, int * /* ad */, void * /*arg*/)
{
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
bool reenabled;
@@ -566,16 +578,13 @@ ssl_context_enable_tickets(SSL_CTX *ctx, const char *ticket_key_path)
// so that we don't leave a ticket_key pointer attached if it fails.
if (SSL_CTX_set_tlsext_ticket_key_cb(ctx, ssl_callback_session_ticket) == 0) {
Error("failed to set session ticket callback");
- goto fail;
+ ticket_block_free(keyblock);
+ return nullptr;
}
SSL_CTX_clear_options(ctx, SSL_OP_NO_TICKET);
return keyblock;
-fail:
- ticket_block_free(keyblock);
- return nullptr;
-
#else /* !HAVE_OPENSSL_SESSION_TICKETS */
(void)ticket_key_path;
return nullptr;
@@ -1188,7 +1197,7 @@ SSLDiagnostic(const SourceLocation &loc, bool debug, SSLNetVConnection *vc, cons
while ((l = ERR_get_error_line_data(&file, &line, &data, &flags)) != 0) {
if (debug) {
if (unlikely(diags->on())) {
- diags->log("ssl", DL_Debug, &loc, "SSL::%lu:%s:%s:%d%s%s%s%s", es, ERR_error_string(l, buf), file, line,
+ diags->log("ssl-diag", DL_Debug, &loc, "SSL::%lu:%s:%s:%d%s%s%s%s", es, ERR_error_string(l, buf), file, line,
(flags & ERR_TXT_STRING) ? ":" : "", (flags & ERR_TXT_STRING) ? data : "", vc ? ": peer address is " : "",
ip_buf);
}
@@ -1212,7 +1221,7 @@ SSLDiagnostic(const SourceLocation &loc, bool debug, SSLNetVConnection *vc, cons
va_start(ap, fmt);
if (debug) {
- diags->log_va("ssl", DL_Debug, &loc, fmt, ap);
+ diags->log_va("ssl-diag", DL_Debug, &loc, fmt, ap);
} else {
diags->error_va(DL_Error, &loc, fmt, ap);
}
@@ -1453,8 +1462,9 @@ ssl_set_handshake_callbacks(SSL_CTX *ctx)
// Make sure the callbacks are set
#if TS_USE_CERT_CB
SSL_CTX_set_cert_cb(ctx, ssl_cert_callback, nullptr);
+ SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_only_callback);
#else
- SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback);
+ SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_and_cert_callback);
#endif
#endif
}
diff --git a/lib/ts/apidefs.h.in b/lib/ts/apidefs.h.in
index d38acb6..3807220 100644
--- a/lib/ts/apidefs.h.in
+++ b/lib/ts/apidefs.h.in
@@ -288,7 +288,8 @@ typedef enum {
TS_VCONN_PRE_ACCEPT_HOOK = TS_SSL_FIRST_HOOK,
TS_SSL_SNI_HOOK,
TS_SSL_CERT_HOOK = TS_SSL_SNI_HOOK,
- TS_SSL_LAST_HOOK = TS_SSL_CERT_HOOK,
+ TS_SSL_SERVERNAME_HOOK,
+ TS_SSL_LAST_HOOK = TS_SSL_SERVERNAME_HOOK,
TS_HTTP_LAST_HOOK
} TSHttpHookID;
@@ -448,7 +449,8 @@ typedef enum {
TS_EVENT_INTERNAL_60200 = 60200,
TS_EVENT_INTERNAL_60201 = 60201,
TS_EVENT_INTERNAL_60202 = 60202,
- TS_EVENT_SSL_CERT = 60203
+ TS_EVENT_SSL_CERT = 60203,
+ TS_EVENT_SSL_SERVERNAME = 60204
} TSEvent;
#define TS_EVENT_HTTP_READ_REQUEST_PRE_REMAP TS_EVENT_HTTP_PRE_REMAP /* backwards compat */
diff --git a/proxy/InkAPIInternal.h b/proxy/InkAPIInternal.h
index a3248b8..5e364a9 100644
--- a/proxy/InkAPIInternal.h
+++ b/proxy/InkAPIInternal.h
@@ -285,6 +285,7 @@ typedef enum {
TS_SSL_INTERNAL_FIRST_HOOK,
TS_VCONN_PRE_ACCEPT_INTERNAL_HOOK = TS_SSL_INTERNAL_FIRST_HOOK,
TS_SSL_CERT_INTERNAL_HOOK,
+ TS_SSL_SERVERNAME_INTERNAL_HOOK,
TS_SSL_INTERNAL_LAST_HOOK
} TSSslHookInternalID;
diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index 55c56d5..6721f01 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -5550,7 +5550,8 @@ typedef enum {
ORIG_TS_SSL_FIRST_HOOK,
ORIG_TS_VCONN_PRE_ACCEPT_HOOK = ORIG_TS_SSL_FIRST_HOOK,
ORIG_TS_SSL_SNI_HOOK,
- ORIG_TS_SSL_LAST_HOOK = ORIG_TS_SSL_SNI_HOOK,
+ ORIG_TS_SSL_SERVERNAME_HOOK,
+ ORIG_TS_SSL_LAST_HOOK = TS_SSL_SERVERNAME_HOOK,
ORIG_TS_HTTP_LAST_HOOK
} ORIG_TSHttpHookID;
diff --git a/proxy/http/HttpDebugNames.cc b/proxy/http/HttpDebugNames.cc
index 7285787..b9e5263 100644
--- a/proxy/http/HttpDebugNames.cc
+++ b/proxy/http/HttpDebugNames.cc
@@ -482,6 +482,8 @@ HttpDebugNames::get_api_hook_name(TSHttpHookID t)
return "TS_VCONN_PRE_ACCEPT_HOOK";
case TS_SSL_CERT_HOOK:
return "TS_SSL_CERT_HOOK";
+ case TS_SSL_SERVERNAME_HOOK:
+ return "TS_SSL_SERVERNAME_HOOK";
}
return "unknown hook";
--
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.