trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpe...@apache.org
Subject git commit: TS-1484: Fix SNI crashes where there is no default certificate
Date Fri, 21 Sep 2012 00:15:43 GMT
Updated Branches:
  refs/heads/master 319d54d4c -> d6d07d8c4


TS-1484: Fix SNI crashes where there is no default certificate

Remove global default SSL_CTX that just didn't work. Move all the
SNI callbacks into the certificate lookup store so that the callbacks
only get set once. Make sure that we always have a default SSL
context that can bootstrap SNI. Minor code cleanup.


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

Branch: refs/heads/master
Commit: d6d07d8c43c084d5683b45c1efe9ed2bf9ef8635
Parents: 319d54d
Author: James Peach <jpeach@apache.org>
Authored: Thu Sep 20 17:13:51 2012 -0700
Committer: James Peach <jpeach@apache.org>
Committed: Thu Sep 20 17:13:51 2012 -0700

----------------------------------------------------------------------
 CHANGES                         |    2 +
 iocore/net/P_SSLCertLookup.h    |    8 ++--
 iocore/net/SSLCertLookup.cc     |   92 ++++++++++++++++++++++++++-------
 iocore/net/SSLNetVConnection.cc |   55 --------------------
 4 files changed, 78 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d6d07d8c/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 86997af..6b5145c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.3.1
 
+  *) [TS-1484] Fix SNI crashes where there is no default certificate
+
   *) [TS-1473] Fix header_filter plugin for ARM.
 
   *) [TS-1457] Change chunking output to avoid massive memory use by transforms.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d6d07d8c/iocore/net/P_SSLCertLookup.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLCertLookup.h b/iocore/net/P_SSLCertLookup.h
index da18345..ae65fd8 100644
--- a/iocore/net/P_SSLCertLookup.h
+++ b/iocore/net/P_SSLCertLookup.h
@@ -36,12 +36,12 @@ class SSLCertLookup
   bool addInfoToHash(
     const char *strAddr, const char *cert, const char *ca, const char *serverPrivateKey);
 
-  char config_file_path[PATH_NAME_MAX];
-  SslConfigParams *param;
-  bool multipleCerts;
+  char              config_file_path[PATH_NAME_MAX];
+  SslConfigParams * param;
+  bool              multipleCerts;
 
   SSLContextStorage * ssl_storage;
-  SSL_CTX * ssl_default;
+  SSL_CTX *           ssl_default;
 
 public:
   bool hasMultipleCerts() const { return multipleCerts; }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d6d07d8c/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 3f2a29e..bce1124 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -45,6 +45,64 @@ typedef const SSL_METHOD * ink_ssl_method_t;
 typedef SSL_METHOD * ink_ssl_method_t;
 #endif
 
+#if TS_USE_TLS_SNI
+
+static int
+ssl_servername_callback(SSL * ssl, int * ad, void * arg)
+{
+  SSL_CTX *       ctx = NULL;
+  SSLCertLookup * lookup = (SSLCertLookup *) arg;
+  const char *    servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
+
+  Debug("ssl", "ssl=%p ad=%d lookup=%p server=%s", ssl, *ad, lookup, servername);
+
+  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);
+
+  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
+  // client, see SSL_AD_*.
+  return SSL_TLSEXT_ERR_OK;
+}
+
+#endif /* TS_USE_TLS_SNI */
+
+static SSL_CTX *
+make_ssl_context(void * arg)
+{
+  SSL_CTX *         ctx = NULL;
+  ink_ssl_method_t  meth = NULL;
+
+  meth = SSLv23_server_method();
+  ctx = SSL_CTX_new(meth);
+
+#if TS_USE_TLS_SNI
+  Debug("ssl", "setting SNI callbacks with for ctx %p", ctx);
+  SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback);
+  SSL_CTX_set_tlsext_servername_arg(ctx, arg);
+#endif /* TS_USE_TLS_SNI */
+
+  return ctx;
+}
+
 class SSLContextStorage
 {
 
@@ -112,7 +170,14 @@ void
 SSLCertLookup::init(SslConfigParams * p)
 {
   param = p;
-  multipleCerts = buildTable();
+
+  this->multipleCerts = buildTable();
+
+  // We *must* have a default context even if it can't possibly work. The default context
is used to bootstrap the SSL
+  // handshake so that we can subsequently do the SNI lookup to switch to the real context.
+  if (this->ssl_default == NULL) {
+    this->ssl_default = make_ssl_context(this);
+  }
 }
 
 bool
@@ -192,17 +257,6 @@ SSLCertLookup::buildTable()
     line = tokLine(NULL, &tok_state);
   }                             //  while(line != NULL)
 
-/*  if(num_el == 0)
-  {
-    Warning("%s No entries in %s. Using default server cert for all connections",
-	    moduleName, configFilePath);
-  }
-
-  if(is_debug_tag_set("ssl"))
-  {
-    Print();
-  }
-*/
   ats_free(file_buf);
   return ret;
 }
@@ -272,10 +326,9 @@ SSLCertLookup::addInfoToHash(
     const char *strAddr, const char *cert,
     const char *caCert, const char *serverPrivateKey)
 {
-  ink_ssl_method_t meth = NULL;
+  SSL_CTX * ctx;
 
-  meth = SSLv23_server_method();
-  SSL_CTX *ctx = SSL_CTX_new(meth);
+  ctx = make_ssl_context(this);
   if (!ctx) {
     SSLNetProcessor::logSSLError("Cannot create new server contex.");
     return (false);
@@ -538,12 +591,11 @@ REGRESSION_TEST(SslHostLookup)(RegressionTest* t, int atype, int * pstatus)
 {
   TestBox           tb(t, pstatus);
   SSLContextStorage storage;
-  ink_ssl_method_t  methods = SSLv23_server_method();
 
-  SSL_CTX * wild = SSL_CTX_new(methods);
-  SSL_CTX * notwild = SSL_CTX_new(methods);
-  SSL_CTX * b_notwild = SSL_CTX_new(methods);
-  SSL_CTX * foo = SSL_CTX_new(methods);
+  SSL_CTX * wild = make_ssl_context(NULL);
+  SSL_CTX * notwild = make_ssl_context(NULL);
+  SSL_CTX * b_notwild = make_ssl_context(NULL);
+  SSL_CTX * foo = make_ssl_context(NULL);
 
   *pstatus = REGRESSION_TEST_PASSED;
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d6d07d8c/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index fd89cba..1df458e 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -24,10 +24,6 @@
 #include "P_Net.h"
 #include "P_SSLNextProtocolSet.h"
 
-#if HAVE_OPENSSL_TLS1_H
-#include <openssl/tls1.h>
-#endif
-
 #define SSL_READ_ERROR_NONE	  0
 #define SSL_READ_ERROR		  1
 #define SSL_READ_READY		  2
@@ -46,48 +42,6 @@ ClassAllocator<SSLNetVConnection> sslNetVCAllocator("sslNetVCAllocator");
 // Private
 //
 
-static SSL_CTX * ssl_default = SSL_CTX_new(SSLv23_server_method());
-
-#if TS_USE_TLS_SNI
-
-static int
-ssl_servername_callback(SSL * ssl, int * ad, void * arg)
-{
-  SSL_CTX *       ctx = NULL;
-  SSLCertLookup * lookup = (SSLCertLookup *) arg;
-  const char *    servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
-
-  Debug("ssl", "ssl=%p ad=%d lookup=%p server=%s", ssl, *ad, lookup, servername);
-
-  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);
-
-  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
-  // client, see SSL_AD_*.
-  return SSL_TLSEXT_ERR_OK;
-}
-
-#endif /* TS_USE_TLS_SNI */
-
 static SSL *
 make_ssl_connection(SSL_CTX * ctx, SSLNetVConnection * netvc)
 {
@@ -503,15 +457,6 @@ 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);
-      SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback);
-      SSL_CTX_set_tlsext_servername_arg(ctx, &sslCertLookup);
-#endif /* TS_USE_TLS_SNI */
 
       this->ssl = make_ssl_connection(ctx, this);
       if (this->ssl == NULL) {


Mime
View raw message