git commit: TS-1392: Fix SNI certificate fallback path

[jp...@apache.org]

Updated Branches:
  refs/heads/master a2b709167 -> 9c3bebd88


TS-1392: Fix SNI certificate fallback path

When the SNI lookup fails, we fall back to a bad default SSL context
instead of the context that we selected when we accepted the TCP
connection. Make sure that we don't clobber a SSL context if the
SNI lookup fails.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/9c3bebd8
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/9c3bebd8
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/9c3bebd8

Branch: refs/heads/master
Commit: 9c3bebd88eecf6aee1ce346b67460b8e1787752d
Parents: a2b7091
Author: James Peach <jp...@apache.org>
Authored: Mon Aug 6 20:42:43 2012 -0700
Committer: James Peach <jp...@apache.org>
Committed: Mon Aug 6 20:42:43 2012 -0700

----------------------------------------------------------------------
 CHANGES                         |    2 ++
 iocore/net/SSLCertLookup.cc     |    7 -------
 iocore/net/SSLNetVConnection.cc |   17 ++++++++++++++---
 3 files changed, 16 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9c3bebd8/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index cacaa22..6399c3c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.3.0
 
+  *) [TS-1392] Fix SNI certificate fallback path
+
   *) [TS-1385] generic atomic operations API
 
   *) [TS-1380] SSL wildcard lookup doesn't find the longest match

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9c3bebd8/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index b5392a7..3f2a29e 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -113,13 +113,6 @@ SSLCertLookup::init(SslConfigParams * p)
 {
   param = p;
   multipleCerts = buildTable();
-
-  // If there wasn't a default SSL context, make a default one. We need this to bootstrap
-  // the SNI process and also to avoid crashing (which is generaly frowned upon).
-  if (!this->ssl_default) {
-    // XXX this leaks, but we're a singleton, so ....
-    this->ssl_default = SSL_CTX_new(SSLv23_server_method());
-  }
 }
 
 bool

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9c3bebd8/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index e9372e9..fd89cba 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -46,6 +46,8 @@ ClassAllocator<SSLNetVConnection> sslNetVCAllocator("sslNetVCAllocator");
 // Private
 //
 
+static SSL_CTX * ssl_default = SSL_CTX_new(SSLv23_server_method());
+
 #if TS_USE_TLS_SNI
 
 static int
@@ -65,12 +67,18 @@ ssl_servername_callback(SSL * ssl, int * ad, void * arg)
     ctx = lookup->defaultContext();
   }
 
-  if (ctx == NULL) {
-    return SSL_TLSEXT_ERR_NOACK;
+  if (ctx != NULL) {
+    SSL_set_SSL_CTX(ssl, ctx);
   }
 
+  // At this point, we might have updated ctx based on the SNI lookup, or we might still have the
+  // original SSL context that we set when we accepted the connection.
+  ctx = SSL_get_SSL_CTX(ssl);
   Debug("ssl", "found SSL context %p for requested name '%s'", ctx, servername);
-  SSL_set_SSL_CTX(ssl, ctx);
+
+  if (ctx == NULL) {
+    return SSL_TLSEXT_ERR_NOACK;
+  }
 
   // We need to return one of the SSL_TLSEXT_ERR constants. If we return an
   // error, we can fill in *ad with an alert code to propgate to the
@@ -495,6 +503,9 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
       if (ctx == NULL) {
         ctx = sslCertLookup.defaultContext();
       }
+      if (ctx == NULL) {
+        ctx = ssl_default;
+      }
 
 #if TS_USE_TLS_SNI
       Debug("ssl", "setting SNI callbacks with initial ctx %p", ctx);