apache · shinrich · Oct 29, 2018 · Oct 22, 2018
diff --git a/doc/developer-guide/api/functions/TSVConnReenable.en.rst b/doc/developer-guide/api/functions/TSVConnReenable.en.rst
@@ -26,7 +26,7 @@ Synopsis
 
 `#include <ts/ts.h>`
 
-.. 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 <ts/ts.h>`
+
+.. 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<NetVConnection *>(vconn);
   SSLNetVConnection *ssl_vc = dynamic_cast<SSLNetVConnection *>(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
@@ -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")
+