git commit: TS-1536: SNI support breaks IP-based lookup

[ig...@apache.org]

Updated Branches:
  refs/heads/3.2.x 2b805c173 -> d8212abfe


TS-1536: SNI support breaks IP-based lookup

The OpenSSL SNI callback will revert to the default context if the
name-based lookup fails even if we already did a successful
address-based context lookup. In this case, we clobber the address-based
context with a default context.

review/test: jpeach, igalic, oschaaf
backport: igalic


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

Branch: refs/heads/3.2.x
Commit: d8212abfed857582944e9a626801acfe9b59366f
Parents: 2b805c1
Author: James Peach <jp...@apache.org>
Authored: Sat Oct 13 21:44:21 2012 -0700
Committer: Igor Galić <i....@brainsware.org>
Committed: Wed Oct 17 13:08:26 2012 +0200

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


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d8212abf/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index c045145..ce545b8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,7 +1,7 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.2.3
 
-  *) [TS-1524] fix signed/unsigned compilation issues in Vec
+  *) [TS-1536] SNI support breaks IP-based lookup
 
   *) [TS-1523] High CPU on *BSD
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d8212abf/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index bce1124..021e1a5 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -56,20 +56,17 @@ ssl_servername_callback(SSL * ssl, int * ad, void * arg)
 
   Debug("ssl", "ssl=%p ad=%d lookup=%p server=%s", ssl, *ad, lookup, servername);
 
+  // The incoming SSL_CTX is either the one mapped from the inbound IP address or the default one. If we don't find a
+  // name-based match at this point, we *do not* want to mess with the context because we've already made a best effort
+  // to find the best match.
   if (likely(servername)) {
     ctx = lookup->findInfoInHash((char *)servername);
   }
 
-  if (ctx == NULL) {
-    ctx = lookup->defaultContext();
-  }
-
   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);
 
@@ -549,6 +546,7 @@ SSLContextStorage::insert(SSL_CTX * ctx, const char * name)
     Debug("ssl", "indexed wildcard certificate for '%s' as '%s' with SSL_CTX %p", name, reversed, ctx);
     return this->wildcards.Insert(reversed, new SslEntry(ctx), 0 /* rank */, -1 /* keylen */);
   } else {
+    Debug("ssl", "indexed '%s' with SSL_CTX %p", name, ctx);
     ink_hash_table_insert(this->hostnames, name, (void *)ctx);
   }
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d8212abf/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 1df458e..201e346 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -454,6 +454,7 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
       safe_getsockname(get_socket(), &ip.sa, &namelen);
       ats_ip_ntop(&ip.sa, buff, sizeof(buff));
       ctx = sslCertLookup.findInfoInHash(buff);
+      Debug("ssl", "IP context is %p, default context %p", ctx, sslCertLookup.defaultContext());
       if (ctx == NULL) {
         ctx = sslCertLookup.defaultContext();
       }