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 2017/03/01 16:56:26 UTC
[trafficserver] branch master updated: Need dedicated
TS_SSL_SERVERNAME_HOOK
This is an automated email from the ASF dual-hosted git repository.
shinrich pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new e6e0447 Need dedicated TS_SSL_SERVERNAME_HOOK
e6e0447 is described below
commit e6e0447d78d170267c0bb0f6a8800e9d2a554da2
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 5ad895b..50f4bdc 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 99a4e43..363bf79 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 f4f21ab..32a762c 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>'].