trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iga...@apache.org
Subject [18/21] git commit: TS-1471: Fix SNI certificate fallback path
Date Fri, 21 Sep 2012 10:04:40 GMT
TS-1471: 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.

Trunk: 9c3bebd88eecf6aee1ce346b67460b8e1787752d
Test/Review: jpeach, igalic, zwoop
Backport: igalic


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

Branch: refs/heads/3.2.x
Commit: ee5d47718af15ed2389110ffc34b1d4721c9c442
Parents: 040e174
Author: James Peach <jpeach@apache.org>
Authored: Mon Aug 6 20:42:43 2012 -0700
Committer: Igor Galić <i.galic@brainsware.org>
Committed: Fri Sep 21 08:47:21 2012 +0200

----------------------------------------------------------------------
 CHANGES                         |    6 +++++-
 STATUS                          |    5 -----
 iocore/net/SSLCertLookup.cc     |    7 -------
 iocore/net/SSLNetVConnection.cc |   17 ++++++++++++++---
 4 files changed, 19 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ee5d4771/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 57ed1dc..00395be 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,9 +1,13 @@
                                                          -*- coding: utf-8 -*-
-Changes with Apache Traffic Server 3.2.1
+Changes with Apache Traffic Server 3.2.2
+
+  *) [TS-1392] Fix SNI certificate fallback path
 
   *)  Fix cache sizes > 16TB
    Author: Jan Van Doorn <Jan_VanDoorn@cable.comcast.com>
 
+Changes with Apache Traffic Server 3.2.1
+
   *) [TS-1358] Don't link libreadline with all binaries and plugins.
 
   *) [TS-1374] Cert path not working using intermdiate certificate

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ee5d4771/STATUS
----------------------------------------------------------------------
diff --git a/STATUS b/STATUS
index 383ba66..78e13ba 100644
--- a/STATUS
+++ b/STATUS
@@ -41,11 +41,6 @@ A list of all bugs open for the next development release can be found at
 
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
 
-  *) Fix SNI certificate fallback path
-   Trunk: 9c3bebd88eecf6aee1ce346b67460b8e1787752d
-   Jira: https://issues.apache.jira/browse/TS-1471
-   +1: jpeach, igalic, zwoop
-
   *) Fix build on amd64 Debian/Ubuntu
    Trunk: b21ccf7 c00f951 02506c0 fa61f83 7fc244e 0d85fe4 44052e9
    Jira: https://issues.apache.jira/browse/TS-1430

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ee5d4771/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 8c323a5..8438ef2 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/ee5d4771/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);


Mime
View raw message