From 64359bec9c16dc1478f40a81f3f9b3b8a4b5eb9f Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Mon, 22 Oct 2018 20:27:20 +0000 Subject: [PATCH] Add VConn Reenable event so plugins can error processing. --- .../api/functions/TSVConnReenable.en.rst | 24 +++- .../hooks-and-transactions/index.en.rst | 10 +- .../hooks-and-transactions/ssl-hooks.en.rst | 18 +-- include/ts/ts.h | 3 + iocore/net/SSLNetVConnection.cc | 1 + src/traffic_server/InkAPI.cc | 18 ++- tests/gold_tests/tls/tls_hooks_verify.test.py | 110 +++++++++++++++ tests/tools/plugins/ssl_verify_test.cc | 125 ++++++++++++++++++ 8 files changed, 290 insertions(+), 19 deletions(-) create mode 100644 tests/gold_tests/tls/tls_hooks_verify.test.py create mode 100644 tests/tools/plugins/ssl_verify_test.cc diff --git a/doc/developer-guide/api/functions/TSVConnReenable.en.rst b/doc/developer-guide/api/functions/TSVConnReenable.en.rst index b076b6093f6..6c8f8c14596 100644 --- a/doc/developer-guide/api/functions/TSVConnReenable.en.rst +++ b/doc/developer-guide/api/functions/TSVConnReenable.en.rst @@ -26,7 +26,7 @@ Synopsis `#include ` -.. function:: void TSSslVConnReenable(TSVConn svc) +.. function:: void TSVConnReenable(TSVConn svc) Description =========== @@ -49,3 +49,25 @@ Traffic Server. This call does appropriate locking and scheduling, so it is safe to call from another thread. +TSVConnReenableEx +***************** + +Synopsis +======== + +`#include ` + +.. function:: void TSVConnReenableEx(TSVConn svc, TSEvent event) + +Description +=========== + +An extended verion of TSVConnEnable that allows the plugin to return a status to +the core logic. If all goes well this is TS_EVENT_CONTINUE. However, if +the plugin wants to stop the processing it can set the event to TS_EVENT_ERROR. + +For example, in the case of the TS_SSL_VERIFY_SERVER_HOOK, the plugin make decide the +origin certificate is bad. By calling TSVonnReenable with TS_EVENT_ERROR, the +certificate check will error and the TLS handshake will fail. + + diff --git a/doc/developer-guide/plugins/hooks-and-transactions/index.en.rst b/doc/developer-guide/plugins/hooks-and-transactions/index.en.rst index f68036f3b6f..0ffd5a4460e 100644 --- a/doc/developer-guide/plugins/hooks-and-transactions/index.en.rst +++ b/doc/developer-guide/plugins/hooks-and-transactions/index.en.rst @@ -114,9 +114,9 @@ HTTP Transaction State Diagram "lock URL in cache" -> "pick address"; "pick address" -> "try connect" [label = " "]; "try connect" -> "pick address" [label = "fail"]; - "try connect" -> TS_SSL_SERVER_VERIFY_HOOK [label = "HTTPS connection"]; - TS_SSL_SERVER_VERIFY_HOOK -> TS_HTTP_SEND_REQUEST_HDR_HOOK [label = "success"]; - TS_SSL_SERVER_VERIFY_HOOK -> "SSL Handshake Error" [label = "fail"]; + "try connect" -> TS_SSL_VERIFY_SERVER_HOOK [label = "HTTPS connection"]; + TS_SSL_VERIFY_SERVER_HOOK -> TS_HTTP_SEND_REQUEST_HDR_HOOK [label = "success"]; + TS_SSL_VERIFY_SERVER_HOOK -> "SSL Handshake Error" [label = "fail"]; "try connect" -> TS_HTTP_SEND_REQUEST_HDR_HOOK [label = "success"]; TS_HTTP_SEND_REQUEST_HDR_HOOK -> "send req hdrs"; "send req hdrs" -> "set up POST/PUT read" [label = "POST/PUT"]; @@ -145,8 +145,8 @@ HTTP Transaction State Diagram TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK[shape = box]; TS_HTTP_SELECT_ALT_HOOK [shape = box]; TS_HTTP_READ_CACHE_HDR_HOOK [shape = box]; - TS_SSL_SERVER_VERIFY_HOOK [shape = box]; - TS_SSL_SERVER_VERIFY_HOOK [tooltip = "verify server certificate"]; + TS_SSL_VERIFY_SERVER_HOOK [shape = box]; + TS_SSL_VERIFY_SERVER_HOOK [tooltip = "verify server certificate"]; TS_HTTP_SEND_REQUEST_HDR_HOOK [shape = box]; "set up req transform" [tooltip = "req transform takes place here"]; TS_HTTP_READ_RESPONSE_HDR_HOOK [shape = box]; diff --git a/doc/developer-guide/plugins/hooks-and-transactions/ssl-hooks.en.rst b/doc/developer-guide/plugins/hooks-and-transactions/ssl-hooks.en.rst index bb564470dfe..964f43bdeac 100644 --- a/doc/developer-guide/plugins/hooks-and-transactions/ssl-hooks.en.rst +++ b/doc/developer-guide/plugins/hooks-and-transactions/ssl-hooks.en.rst @@ -40,7 +40,7 @@ The following actions are valid from these callbacks. * Fetch the SSL object associated with the connection - :c:func:`TSVConnSslConnectionGet` * Set a connection to blind tunnel - :c:func:`TSVConnTunnel` - * Reenable the ssl connection - :c:func:`TSSslVConnReenable` + * Reenable the ssl connection - :c:func:`TSVConnReenable` * Find SSL context by name - :c:func:`TSSslContextFindByName` * Find SSL context by address - :c:func:`TSSslContextFindByAddr` * Determine whether the TSVConn is really representing a SSL connection - :c:func:`TSVConnIsSsl` @@ -52,7 +52,7 @@ This hook is invoked after the client has connected to ATS and before the SSL ha In theory this hook could apply and be useful for non-SSL connections as well, but at this point this hook is only called in the SSL sequence. -The TLS handshake processing will not proceed until :c:func:`TSSslVConnReenable()` is called either from within the hook +The TLS handshake processing will not proceed until :c:func:`TSVConnReenable()` is called either from within the hook callback or from another piece of code. TS_VCONN_CLOSE_HOOK @@ -67,7 +67,7 @@ This hook is called if the client provides SNI information in the SSL handshake. The Traffic Server core first evaluates the settings in the ssl_multicert.config file based on the server name. Then the core SNI callback executes the plugin registered SNI callback code. The plugin callback can access the servername by calling the openssl function SSL_get_servername(). -Processing will continue regardless of whether the hook callback executes :c:func:`TSSslVConnReenable()` since the openssl +Processing will continue regardless of whether the hook callback executes :c:func:`TSVConnReenable()` since the openssl implementation does not allow for pausing processing during the openssl servername callback. TS_SSL_CERT_HOOK @@ -78,8 +78,8 @@ code to create or select the certificate that should be used for the TLS handsha Traffic Server certificate selection. If you are running with openssl 1.0.2 or later, you can control whether the TLS handshake processing will -continue after the certificate hook callback execute by calling :c:func:`TSSslVConnReenable()` or not. The TLS -handshake processing will not proceed until :c:func:`TSSslVConnReenable()` is called. +continue after the certificate hook callback execute by calling :c:func:`TSVConnReenable()` or not. The TLS +handshake processing will not proceed until :c:func:`TSVConnReenable()` is called. It may be useful to delay the TLS handshake processing if other resources must be consulted to select or create a certificate. @@ -137,16 +137,16 @@ TLS Hook State Diagram 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; + HANDSHAKE_HOOKS_PRE_INVOKE -> TSVConnReenable; + TSVConnReenable -> HANDSHAKE_HOOKS_PRE; TS_SSL_SERVERNAME_HOOK -> HANDSHAKE_HOOKS_SNI; HANDSHAKE_HOOKS_SNI -> TS_SSL_SERVERNAME_HOOK; HANDSHAKE_HOOKS_SNI -> TS_SSL_CERT_HOOK; HANDSHAKE_HOOKS_SNI -> HANDSHAKE_HOOKS_DONE; HANDSHAKE_HOOKS_CERT -> TS_SSL_CERT_HOOK; TS_SSL_CERT_HOOK -> HANDSHAKE_HOOKS_CERT_INVOKE; - HANDSHAKE_HOOKS_CERT_INVOKE -> TSSslVConnReenable2; - TSSslVConnReenable2 -> HANDSHAKE_HOOKS_CERT; + HANDSHAKE_HOOKS_CERT_INVOKE -> TSVConnReenable2; + TSVConnReenable2 -> HANDSHAKE_HOOKS_CERT; HANDSHAKE_HOOKS_CERT -> HANDSHAKE_HOOKS_DONE; HANDSHAKE_HOOKS_DONE -> TS_VCONN_CLOSE_HOOK; diff --git a/include/ts/ts.h b/include/ts/ts.h index 4fbe2d10e91..a3ba7b3d7bf 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1215,6 +1215,9 @@ tsapi TSVConn TSHttpSsnServerVConnGet(TSHttpSsn ssnp); /* Re-enable an SSL connection from a hook. This must be called exactly once before the SSL connection will resume. */ tsapi void TSVConnReenable(TSVConn sslvcp); +/* Extended version that allows for passing a status event on reenabling + */ +tsapi void TSVConnReenableEx(TSVConn sslvcp, TSEvent event); /* Set the connection to go into blind tunnel mode */ tsapi TSReturnCode TSVConnTunnel(TSVConn sslp); /* Return the SSL object associated with the connection */ diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc index 6b3dde29c75..5a2b0db7e51 100644 --- a/iocore/net/SSLNetVConnection.cc +++ b/iocore/net/SSLNetVConnection.cc @@ -1619,6 +1619,7 @@ SSLNetVConnection::callHooks(TSEvent eventId) // Move state if it is appropriate switch (this->sslHandshakeHookState) { case HANDSHAKE_HOOKS_PRE: + case HANDSHAKE_HOOKS_OUTBOUND_PRE: if (eventId == TS_EVENT_SSL_SERVERNAME) { this->sslHandshakeHookState = HANDSHAKE_HOOKS_SNI; } else if (eventId == TS_EVENT_SSL_VERIFY_SERVER) { diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index 7513906eb49..892eac94355 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -8773,17 +8773,21 @@ TSHttpEventNameLookup(TSEvent event) class TSSslCallback : public Continuation { public: - TSSslCallback(SSLNetVConnection *vc) : Continuation(vc->nh->mutex), m_vc(vc) { SET_HANDLER(&TSSslCallback::event_handler); } + TSSslCallback(SSLNetVConnection *vc, TSEvent event) : Continuation(vc->nh->mutex), m_vc(vc), m_event(event) + { + SET_HANDLER(&TSSslCallback::event_handler); + } int event_handler(int event, void *) { - m_vc->reenable(m_vc->nh); + m_vc->reenable(m_vc->nh, m_event); delete this; return 0; } private: SSLNetVConnection *m_vc; + TSEvent m_event; }; /// SSL Hooks @@ -8948,6 +8952,12 @@ TSVConnIsSsl(TSVConn sslp) void TSVConnReenable(TSVConn vconn) +{ + TSVConnReenableEx(vconn, TS_EVENT_CONTINUE); +} + +void +TSVConnReenableEx(TSVConn vconn, TSEvent event) { NetVConnection *vc = reinterpret_cast(vconn); SSLNetVConnection *ssl_vc = dynamic_cast(vc); @@ -8958,10 +8968,10 @@ TSVConnReenable(TSVConn vconn) // We use the mutex of VC's NetHandler so we can put the VC into ready_list by reenable() MUTEX_TRY_LOCK(trylock, ssl_vc->nh->mutex, eth); if (trylock.is_locked()) { - ssl_vc->reenable(ssl_vc->nh); + ssl_vc->reenable(ssl_vc->nh, event); } else { // We schedule the reenable to the home thread of ssl_vc. - ssl_vc->thread->schedule_imm(new TSSslCallback(ssl_vc)); + ssl_vc->thread->schedule_imm(new TSSslCallback(ssl_vc, event)); } } } diff --git a/tests/gold_tests/tls/tls_hooks_verify.test.py b/tests/gold_tests/tls/tls_hooks_verify.test.py new file mode 100644 index 00000000000..88fd03737a9 --- /dev/null +++ b/tests/gold_tests/tls/tls_hooks_verify.test.py @@ -0,0 +1,110 @@ +''' +Test SERVER_VERIFY_HOOK +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import re + +Test.Summary = ''' +Test different combinations of TLS handshake hooks to ensure they are applied consistently. +''' + +Test.SkipUnless(Condition.HasProgram("grep", "grep needs to be installed on system for this test to work")) + +ts = Test.MakeATSProcess("ts", select_ports=False) +server = Test.MakeOriginServer("server", ssl=True) +request_header = {"headers": "GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""} +# desired response form the origin server +response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""} +server.addResponse("sessionlog.json", request_header, response_header) + +ts.addSSLfile("ssl/server.pem") +ts.addSSLfile("ssl/server.key") + +ts.Variables.ssl_port = 4443 +ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'ssl_verify_test', + 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir), + 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir), + # enable ssl port + 'proxy.config.http.server_ports': '{0}:ssl'.format(ts.Variables.ssl_port), + 'proxy.config.ssl.server.cipher_suite': 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:AES128-GCM-SHA256:AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:RC4-SHA:RC4-MD5:AES128-SHA:AES256-SHA:DES-CBC3-SHA!SRP:!DSS:!PSK:!aNULL:!eNULL:!SSLv2', + 'proxy.config.ssl.client.verify.server.policy': 'ENFORCED', + 'proxy.config.ssl.client.verify.server.properties': 'NONE', + 'proxy.config.url_remap.pristine_host_hdr': 1 +}) + +ts.Disk.ssl_multicert_config.AddLine( + 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' +) + +ts.Disk.remap_config.AddLine( + 'map https://foo.com:{1}/ https://127.0.0.1:{0}'.format(server.Variables.Port,ts.Variables.ssl_port) +) +ts.Disk.remap_config.AddLine( + 'map https://bar.com:{1}/ https://127.0.0.1:{0}'.format(server.Variables.Port,ts.Variables.ssl_port) +) +ts.Disk.remap_config.AddLine( + 'map https://random.com:{1}/ https://127.0.0.1:{0}'.format(server.Variables.Port,ts.Variables.ssl_port) +) + +ts.Disk.ssl_server_name_yaml.AddLine( + '- fqdn: bar.com') +ts.Disk.ssl_server_name_yaml.AddLine( + ' verify_server_policy: PERMISSIVE') + +Test.PreparePlugin(os.path.join(Test.Variables.AtsTestToolsDir, 'plugins', 'ssl_verify_test.cc'), ts, '-count=2 -bad=random.com -bad=bar.com') + +tr = Test.AddTestRun("request good name") +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port)) +tr.StillRunningAfter = ts +tr.StillRunningAfter = server +tr.Processes.Default.Command = "curl --resolve \"foo.com:{0}:127.0.0.1\" -k https://foo.com:{0}".format(ts.Variables.ssl_port) +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have failed") + + +tr2 = Test.AddTestRun("request bad name") +tr2.StillRunningAfter = ts +tr2.StillRunningAfter = server +tr2.Processes.Default.Command = "curl --resolve \"random.com:{0}:127.0.0.1\" -k https://random.com:{0}".format(ts.Variables.ssl_port) +tr2.Processes.Default.ReturnCode = 0 +tr2.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Curl attempt should have failed") + +tr3 = Test.AddTestRun("request bad name permissive") +tr3.StillRunningAfter = ts +tr3.StillRunningAfter = server +tr3.Processes.Default.Command = "curl --resolve \"bar.com:{0}:127.0.0.1\" -k https://bar.com:{0}".format(ts.Variables.ssl_port) +tr3.Processes.Default.ReturnCode = 0 +tr3.Processes.Default.Streams.stdout = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have failed") + +# Over riding the built in ERROR check since we expect tr2 to fail +ts.Disk.diags_log.Content = Testers.ContainsExpression("WARNING: TS_EVENT_SSL_VERIFY_SERVER plugin failed the origin certificate check for 127.0.0.1. Action=Terminate SNI=random.com", "random.com should fail") +ts.Disk.diags_log.Content += Testers.ContainsExpression("WARNING: TS_EVENT_SSL_VERIFY_SERVER plugin failed the origin certificate check for 127.0.0.1. Action=Continue SNI=bar.com", "bar.com should fail but continue") +ts.Disk.diags_log.Content += Testers.ContainsExpression("ERROR: SSL connection failed for 'random.com': .+?:certificate verify failed", "random.com should really fail") +ts.Disk.diags_log.Content += Testers.ExcludesExpression("SNI=foo.com", "foo.com should not fail in any way") + +ts.Streams.All += Testers.ContainsExpression("Server verify callback 0 [\da-fx]+? - event is good SNI=foo.com good HS", "verify callback happens 2 times") +ts.Streams.All += Testers.ContainsExpression("Server verify callback 1 [\da-fx]+? - event is good SNI=foo.com good HS", "verify callback happens 2 times") +ts.Streams.All += Testers.ContainsExpression("Server verify callback 0 [\da-fx]+? - event is good SNI=random.com error HS", "verify callback happens 2 times") +ts.Streams.All += Testers.ContainsExpression("Server verify callback 1 [\da-fx]+? - event is good SNI=random.com error HS", "verify callback happens 2 times") +ts.Streams.All += Testers.ContainsExpression("Server verify callback 0 [\da-fx]+? - event is good SNI=bar.com error HS", "verify callback happens 2 times") +ts.Streams.All += Testers.ContainsExpression("Server verify callback 1 [\da-fx]+? - event is good SNI=bar.com error HS", "verify callback happens 2 times") + diff --git a/tests/tools/plugins/ssl_verify_test.cc b/tests/tools/plugins/ssl_verify_test.cc new file mode 100644 index 00000000000..4599db99fd1 --- /dev/null +++ b/tests/tools/plugins/ssl_verify_test.cc @@ -0,0 +1,125 @@ +/** @file + + SSL Preaccept test plugin + Implements blind tunneling based on the client IP address + The client ip addresses are specified in the plugin's + config file as an array of IP addresses or IP address ranges under the + key "client-blind-tunnel" + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define PN "ssl_verify_test" +#define PCP "[" PN " Plugin] " + +std::map bad_names; + +int +CB_server_verify(TSCont cont, TSEvent event, void *edata) +{ + TSVConn ssl_vc = reinterpret_cast(edata); + + int count = reinterpret_cast(TSContDataGet(cont)); + + // Is this a good name or not? + TSEvent reenable_event = TS_EVENT_CONTINUE; + TSSslConnection sslobj = TSVConnSSLConnectionGet(ssl_vc); + SSL *ssl = (SSL *)sslobj; + const char *sni_name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); + if (sni_name) { + std::string sni_string(sni_name); + if (bad_names.find(sni_string) != bad_names.end()) { + reenable_event = TS_EVENT_ERROR; + } + } + + TSDebug(PN, "Server verify callback %d %p - event is %s SNI=%s %s", count, ssl_vc, + event == TS_EVENT_SSL_VERIFY_SERVER ? "good" : "bad", sni_name, + reenable_event == TS_EVENT_ERROR ? "error HS" : "good HS"); + + // All done, reactivate things + TSVConnReenableEx(ssl_vc, reenable_event); + return TS_SUCCESS; +} + +void +parse_callbacks(int argc, const char *argv[], int &count) +{ + int i = 0; + const char *ptr; + for (i = 0; i < argc; i++) { + if (argv[i][0] == '-') { + switch (argv[i][1]) { + case 'c': + ptr = index(argv[i], '='); + if (ptr) { + count = atoi(ptr + 1); + } + break; + case 'b': + ptr = index(argv[i], '='); + if (ptr) { + bad_names.insert(std::pair(std::string(ptr + 1), 1)); + } + break; + } + } + } +} + +void +setup_callbacks(int count) +{ + TSCont cb = nullptr; + int i; + + TSDebug(PN, "Setup callbacks count=%d", count); + for (i = 0; i < count; i++) { + cb = TSContCreate(&CB_server_verify, TSMutexCreate()); + TSContDataSet(cb, (void *)(intptr_t)i); + TSHttpHookAdd(TS_SSL_VERIFY_SERVER_HOOK, cb); + } + return; +} + +// Called by ATS as our initialization point +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info; + info.plugin_name = const_cast("SSL verify server test"); + info.vendor_name = const_cast("apache"); + info.support_email = const_cast("shinrich@apache.org"); + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PN); + } + + int verify_count = 0; + parse_callbacks(argc, argv, verify_count); + setup_callbacks(verify_count); + return; +}