Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/developer-guide/api/types/TSHttpHookID.en.rst
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ Enumeration Members

.. c:macro:: TSHttpHookID TS_SSL_VERIFY_CLIENT_HOOK

.. c:macro:: TSHttpHookID TS_SSL_VERIFY_SERVER_HOOK

.. c:macro:: TSHttpHookID TS_SSL_LAST_HOOK

.. c:macro:: TSHttpHookID TS_HTTP_LAST_HOOK
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,29 @@ handshake processing will not proceed until :c:func:`TSSslVConnReenable()` is ca
It may be useful to delay the TLS handshake processing if other resources must be consulted to select or create
a certificate.

TS_SSL_VERIFY_CLIENT_HOOK
-------------------------

This hook is called when a client connects to Traffic Server and presents a
client certificate in the case of a mutual TLS handshake. The callback can
get the SSL object from the TSVConn argument and use that to access the client
certificate and make any additional checks.

Processing will continue regardless of whether the hook callback executes
:c:func:`TSSslVConnReenable()` since the openssl implementation does not allow
for pausing processing during the certificate verify callback.

TS_SSL_VERIFY_SERVER_HOOK
-------------------------

This hooks is called when a Traffic Server connects to an origin and the origin
presents a certificate. The callback can get the SSL object from the TSVConn
argument and use that to access the origin certificate and make any additional checks.

Processing will continue regardless of whether the hook callback executes
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong. #4427 adds the possibility to "interrupt" the workflow by calling TSVConnReenableEx() with TS_EVENT_ERROR. Seems contradictory once both PRs are committed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. The underlying openssl structure limits what we can do. However, in the case of the verify_server hook, we cannot leave the callback not-enabled. But we can return an ERROR and have the core ATS verify_callback interpret that to fail the certificate verification.

Copy link
Copy Markdown

@CrendKing CrendKing Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying openssl structure limits what we can do

Just to make sure we are on the side page. We agree that if the callback function from SSL_set_verify() returns 0, the OpenSSL will stop the TLS handshake, right? So what do you mean here?

https://www.openssl.org/docs/man1.0.2/ssl/SSL_set_verify.html

The return value of verify_callback controls the strategy of the further verification process. If verify_callback returns 0, the verification process is immediately stopped with "verification failed" state.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. The verify_callback() function is registered by Traffic Server via the SSL_CTX_set_verify or SSL_set_verify openssl function. It will eventually return either 0 or 1. If it returns 0, the handshake will fail . It it returns 1, the handshake will continue on.

The VERIFY_SERVER_HOOK gives a place for plugins to insert logic and influence the return value of the verify_callback(). If the hook reenables with a TS_EVENT_ERROR, the verify_callback() will return 0.

:c:func:`TSSslVConnReenable()` since the openssl implementation does not allow
for pausing processing during the certificate verify callback.

TLS Hook State Diagram
----------------------

Expand All @@ -92,9 +115,11 @@ TLS Hook State Diagram

digraph tls_hook_state_diagram{
HANDSHAKE_HOOKS_PRE -> TS_VCONN_START_HOOK;
HANDSHAKE_HOOKS_PRE -> TS_SSL_VERIFY_CLIENT_HOOK;
HANDSHAKE_HOOKS_PRE -> TS_SSL_CERT_HOOK;
HANDSHAKE_HOOKS_PRE -> TS_SSL_SERVERNAME_HOOK;
HANDSHAKE_HOOKS_PRE -> HANDSHAKE_HOOKS_DONE;
TS_SSL_VERIFY_CLIENT_HOOK -> HANDSHAKE_HOOKS_PRE;
TS_VCONN_START_HOOK -> HANDSHAKE_HOOKS_PRE_INVOKE;
HANDSHAKE_HOOKS_PRE_INVOKE -> TSSslVConnReenable;
TSSslVConnReenable -> HANDSHAKE_HOOKS_PRE;
Expand All @@ -110,6 +135,8 @@ TLS Hook State Diagram
HANDSHAKE_HOOKS_DONE -> TS_VCONN_CLOSE_HOOK;

HANDSHAKE_HOOKS_PRE [shape=box];
TS_VCONN_START_HOOK [shape=box];
TS_SSL_VERIFY_CLIENT_HOOK [shape=box];
HANDSHAKE_HOOKS_PRE_INVOKE [shape=box];
HANDSHAKE_HOOKS_SNI [shape=box];
HANDSHAKE_HOOKS_CERT [shape=box];
Expand Down
2 changes: 2 additions & 0 deletions include/ts/apidefs.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ typedef enum {
TS_SSL_CERT_HOOK = TS_SSL_SNI_HOOK,
TS_SSL_SERVERNAME_HOOK,
TS_SSL_SERVER_VERIFY_HOOK,
TS_SSL_VERIFY_SERVER_HOOK = TS_SSL_SERVER_VERIFY_HOOK,
TS_SSL_VERIFY_CLIENT_HOOK,
TS_SSL_SESSION_HOOK,
TS_SSL_LAST_HOOK = TS_SSL_SESSION_HOOK,
Expand Down Expand Up @@ -462,6 +463,7 @@ typedef enum {
TS_EVENT_SSL_CERT = 60203,
TS_EVENT_SSL_SERVERNAME = 60204,
TS_EVENT_SSL_SERVER_VERIFY_HOOK = 60205,
TS_EVENT_SSL_VERIFY_SERVER = 60205,
TS_EVENT_SSL_VERIFY_CLIENT = 60206
} TSEvent;
#define TS_EVENT_HTTP_READ_REQUEST_PRE_REMAP TS_EVENT_HTTP_PRE_REMAP /* backwards compat */
Expand Down
2 changes: 1 addition & 1 deletion iocore/net/SSLClientUtils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
}

if (netvc != nullptr) {
netvc->callHooks(TS_EVENT_SSL_SERVER_VERIFY_HOOK);
netvc->callHooks(TS_EVENT_SSL_VERIFY_SERVER);
char *matched_name = nullptr;
unsigned char *sni_name;
char buff[INET6_ADDRSTRLEN];
Expand Down
6 changes: 3 additions & 3 deletions iocore/net/SSLNetVConnection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1554,7 +1554,7 @@ bool
SSLNetVConnection::callHooks(TSEvent eventId)
{
// Only dealing with the SNI/CERT hook so far.
ink_assert(eventId == TS_EVENT_SSL_CERT || eventId == TS_EVENT_SSL_SERVERNAME || eventId == TS_EVENT_SSL_SERVER_VERIFY_HOOK ||
ink_assert(eventId == TS_EVENT_SSL_CERT || eventId == TS_EVENT_SSL_SERVERNAME || eventId == TS_EVENT_SSL_VERIFY_SERVER ||
eventId == TS_EVENT_SSL_VERIFY_CLIENT || eventId == TS_EVENT_VCONN_CLOSE);
Debug("ssl", "callHooks sslHandshakeHookState=%d", this->sslHandshakeHookState);

Expand All @@ -1581,9 +1581,9 @@ SSLNetVConnection::callHooks(TSEvent eventId)
case HANDSHAKE_HOOKS_SNI:
// The server verify event addresses ATS to origin handshake
// All the other events are for client to ATS
if (eventId == TS_EVENT_SSL_SERVER_VERIFY_HOOK) {
if (eventId == TS_EVENT_SSL_VERIFY_SERVER) {
if (!curHook) {
curHook = ssl_hooks->get(TS_SSL_SERVER_VERIFY_INTERNAL_HOOK);
curHook = ssl_hooks->get(TS_SSL_VERIFY_SERVER_INTERNAL_HOOK);
}
} else {
if (!curHook) {
Expand Down
2 changes: 1 addition & 1 deletion proxy/InkAPIInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ typedef enum {
TS_VCONN_CLOSE_INTERNAL_HOOK,
TS_SSL_CERT_INTERNAL_HOOK,
TS_SSL_SERVERNAME_INTERNAL_HOOK,
TS_SSL_SERVER_VERIFY_INTERNAL_HOOK,
TS_SSL_VERIFY_SERVER_INTERNAL_HOOK,
TS_SSL_VERIFY_CLIENT_INTERNAL_HOOK,
TS_SSL_SESSION_INTERNAL_HOOK,
TS_SSL_INTERNAL_LAST_HOOK
Expand Down
4 changes: 2 additions & 2 deletions proxy/http/HttpDebugNames.cc
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,8 @@ HttpDebugNames::get_api_hook_name(TSHttpHookID t)
return "TS_SSL_CERT_HOOK";
case TS_SSL_SERVERNAME_HOOK:
return "TS_SSL_SERVERNAME_HOOK";
case TS_SSL_SERVER_VERIFY_HOOK:
return "TS_SSL_SERVER_VERIFY_HOOK";
case TS_SSL_VERIFY_SERVER_HOOK:
return "TS_SSL_VERIFY_SERVER_HOOK";
case TS_SSL_VERIFY_CLIENT_HOOK:
return "TS_SSL_VERIFY_CLIENT_HOOK";
case TS_SSL_SESSION_HOOK:
Expand Down
2 changes: 1 addition & 1 deletion src/traffic_server/InkAPITest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6622,7 +6622,7 @@ typedef enum {
ORIG_TS_VCONN_CLOSE_HOOK,
ORIG_TS_SSL_SNI_HOOK,
ORIG_TS_SSL_SERVERNAME_HOOK,
ORIG_TS_SSL_SERVER_VERIFY_HOOK,
ORIG_TS_SSL_VERIFY_SERVER_HOOK,
ORIG_TS_SSL_VERIFY_CLIENT_HOOK,
ORIG_TS_SSL_SESSION_HOOK,
ORIG_TS_SSL_LAST_HOOK = ORIG_TS_SSL_SESSION_HOOK,
Expand Down