You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2012/03/28 05:33:11 UTC

[2/5] git commit: TS-1135: index wildcard certificates for ServerNameIndication

TS-1135: index wildcard certificates for ServerNameIndication

Add a SSLContextStorage support class. This objects owns all the
SSL contexts and indexes them by both name and (stringified) address.
Both the CN from the certificate subject name and the DNS
subjectAlternativeNames are indexed.

If we find a wildcard name, we use the trie for indexing so that
we get longest match semantics. We only support wildcards of the
form '*.foo.org' because that's the only useful form, and the code
for supporting the general case would be a bit hairy.


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

Branch: refs/heads/master
Commit: 0dcad6eaf692e6acee68685c08788bc42e8e15b3
Parents: 313c224
Author: James Peach <jp...@apache.org>
Authored: Thu Mar 22 21:19:37 2012 -0700
Committer: James Peach <jp...@apache.org>
Committed: Tue Mar 27 20:32:26 2012 -0700

----------------------------------------------------------------------
 CHANGES                         |    2 +
 iocore/net/P_SSLCertLookup.h    |    7 +-
 iocore/net/SSLCertLookup.cc     |  246 ++++++++++++++++++++++++++++++----
 iocore/net/SSLNetVConnection.cc |    2 +-
 4 files changed, 225 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0dcad6ea/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 0ff37ea..0611a24 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,7 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.1.4
+  *) [TS-1135] support wildcard certificates for ServerNameIndication (SNI)
+
   *) [TS-1140] Combine IP Allow and QuickFilter.
 
   *) [TS-1159] Add compiler hints to debug logging

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0dcad6ea/iocore/net/P_SSLCertLookup.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLCertLookup.h b/iocore/net/P_SSLCertLookup.h
index c9ea033..b09a8d1 100644
--- a/iocore/net/P_SSLCertLookup.h
+++ b/iocore/net/P_SSLCertLookup.h
@@ -26,6 +26,8 @@
 #include "libts.h"
 #include "P_SSLNetProcessor.h"
 
+class SSLContextStorage;
+
 class SSLCertLookup
 {
   bool buildTable();
@@ -34,16 +36,17 @@ class SSLCertLookup
   bool addInfoToHash(
     const char *strAddr, const char *cert, const char *ca, const char *serverPrivateKey) const;
 
-  InkHashTable *SSLCertLookupHashTable;
   char config_file_path[PATH_NAME_MAX];
   SslConfigParams *param;
   bool multipleCerts;
 
+  SSLContextStorage * ssl_storage;
+
 public:
   bool hasMultipleCerts() const { return multipleCerts; }
 
   void init(SslConfigParams * param);
-  SSL_CTX *findInfoInHash(char *strAddr) const;
+  SSL_CTX *findInfoInHash(const char * address) const;
 
   SSLCertLookup();
   ~SSLCertLookup();

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0dcad6ea/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 5d4a3ff..5792051 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -26,6 +26,11 @@
 #include "P_SSLCertLookup.h"
 #include "P_UnixNet.h"
 #include "I_Layout.h"
+#include "Regex.h"
+#include "Trie.h"
+#include "ts/TestBox.h"
+
+#include <algorithm>
 
 #include <openssl/bio.h>
 #include <openssl/pem.h>
@@ -42,10 +47,40 @@ typedef const SSL_METHOD * ink_ssl_method_t;
 typedef SSL_METHOD * ink_ssl_method_t;
 #endif
 
+class SSLContextStorage
+{
+
+  struct SslEntry
+  {
+    explicit SslEntry(SSL_CTX * c) : ctx(c) {}
+
+    void Print() const { printf("%p/%p", this, ctx); }
+
+    SSL_CTX * ctx;
+    LINK(SslEntry, link);
+  };
+
+  Trie<SslEntry>  wildcards;
+  InkHashTable *  hostnames;
+
+public:
+  SSLContextStorage();
+  ~SSLContextStorage();
+
+  bool insert(SSL_CTX * ctx, const char * name);
+  SSL_CTX * lookup(const char * name) const;
+
+  // XXX although we take "ownership" of the SSL_CTX, we never actually free them when we are
+  // done. Currently, this doesn't matter because altering the set of SSL certificates requires
+  // a restart. When this changes, we will need to keep track of the SSL_CTX pointers exactly
+  // once and call SSL_CTX_free() on them.
+
+};
+
 SSLCertLookup sslCertLookup;
 
 static void
-insert_ssl_certificate(InkHashTable *, SSL_CTX *, const char *);
+insert_ssl_certificate(SSLContextStorage *, SSL_CTX *, const char *);
 
 #define SSL_IP_TAG "dest_ip"
 #define SSL_CERT_TAG "ssl_cert_name"
@@ -57,13 +92,23 @@ const matcher_tags sslCertTags = {
   NULL, NULL, SSL_IP_TAG, NULL, NULL, false
 };
 
-SSLCertLookup::SSLCertLookup():
-param(NULL), multipleCerts(false)
+SSLCertLookup::SSLCertLookup()
+  : param(NULL), multipleCerts(false), ssl_storage(NEW(new SSLContextStorage()))
 {
-  SSLCertLookupHashTable = ink_hash_table_create(InkHashTableKeyType_String);
   *config_file_path = '\0';
 }
 
+SSLCertLookup::~SSLCertLookup()
+{
+  delete this->ssl_storage;
+}
+
+SSL_CTX *
+SSLCertLookup::findInfoInHash(const char * address) const
+{
+  return this->ssl_storage->lookup(address);
+}
+
 void
 SSLCertLookup::init(SslConfigParams * p)
 {
@@ -243,12 +288,14 @@ SSLCertLookup::addInfoToHash(
 
   if (ssl_NetProcessor.initSSLServerCTX(ctx, this->param, cert, caCert, serverPrivateKey, false) == 0) {
     char * certpath = Layout::relative_to(this->param->getServerCertPathOnly(), cert);
-    ink_hash_table_insert(SSLCertLookupHashTable, strAddr, (void *) ctx);
+
+    // Index this certificate by the specified IP(v6) address;
+    this->ssl_storage->insert(ctx, strAddr);
 
     // Insert additional mappings. Note that this maps multiple keys to the same value, so when
     // this code is updated to reconfigure the SSL certificates, it will need some sort of
     // refcounting or alternate way of avoiding double frees.
-    insert_ssl_certificate(SSLCertLookupHashTable, ctx, certpath);
+    insert_ssl_certificate(this->ssl_storage, ctx, certpath);
 
     ats_free(certpath);
     return (true);
@@ -258,26 +305,6 @@ SSLCertLookup::addInfoToHash(
   return (false);
 }
 
-SSL_CTX *
-SSLCertLookup::findInfoInHash(char *strAddr) const
-{
-
-  InkHashTableValue hash_value;
-  if (ink_hash_table_lookup(SSLCertLookupHashTable, strAddr, &hash_value) == 0) {
-    return NULL;
-  } else {
-    return (SSL_CTX *) hash_value;
-  }
-}
-
-SSLCertLookup::~SSLCertLookup()
-{
-  // XXX This is completely broken. You can't use ats_free to free
-  // a SSL_CTX *, you have to use SSL_CTX_free(). It doesn't matter
-  // right now because sslCertLookup is a singleton and never destroyed.
-  ink_hash_table_destroy_and_xfree_values(SSLCertLookupHashTable);
-}
-
 struct ats_x509_certificate
 {
   explicit ats_x509_certificate(X509 * x) : x509(x) {}
@@ -312,6 +339,25 @@ private:
     ats_file_bio& operator=(const ats_file_bio&);
 };
 
+struct ats_wildcard_matcher
+{
+  ats_wildcard_matcher() {
+    if (regex.compile("^\\*\\.[^\\*.]+") != 0) {
+      Fatal("failed to compile TLS wildcard matching regex");
+    }
+  }
+
+  ~ats_wildcard_matcher() {
+  }
+
+  bool match(const char * hostname) const {
+    return regex.match(hostname) != -1;
+  }
+
+private:
+  DFA regex;
+};
+
 static char *
 asn1_strdup(ASN1_STRING * s)
 {
@@ -327,9 +373,10 @@ asn1_strdup(ASN1_STRING * s)
 // table aliases for all of the subject and subjectAltNames. Note that we don't
 // deal with wildcards (yet).
 static void
-insert_ssl_certificate(InkHashTable * htable, SSL_CTX * ctx, const char * certfile)
+insert_ssl_certificate(SSLContextStorage * storage, SSL_CTX * ctx, const char * certfile)
 {
   X509_NAME * subject = NULL;
+  ats_wildcard_matcher wildcard;
 
   ats_file_bio bio(certfile, "r");
   ats_x509_certificate certificate(PEM_read_bio_X509_AUX(bio.bio, NULL, NULL, NULL));
@@ -349,7 +396,7 @@ insert_ssl_certificate(InkHashTable * htable, SSL_CTX * ctx, const char * certfi
       char * name = asn1_strdup(cn);
 
       Debug("ssl", "mapping '%s' to certificate %s", name, certfile);
-      ink_hash_table_insert(htable, name, (void *)ctx);
+      storage->insert(ctx, name);
       ats_free(name);
     }
   }
@@ -368,7 +415,7 @@ insert_ssl_certificate(InkHashTable * htable, SSL_CTX * ctx, const char * certfi
       case GEN_DNS:
         dns = asn1_strdup(name->d.dNSName);
         Debug("ssl", "mapping '%s' to certificate %s", dns, certfile);
-        ink_hash_table_insert(htable, dns, (void *)ctx);
+        storage->insert(ctx, dns);
         ats_free(dns);
         break;
       }
@@ -379,3 +426,144 @@ insert_ssl_certificate(InkHashTable * htable, SSL_CTX * ctx, const char * certfi
 #endif // HAVE_OPENSSL_TS_H
 
 }
+
+static char *
+reverse_dns_name(const char * hostname, char (&reversed)[TS_MAX_HOST_NAME_LEN+1])
+{
+  char * ptr = reversed + sizeof(reversed);
+  const char * part = hostname;
+
+  *(--ptr) = '\0'; // NUL-terminate
+
+  while (*part) {
+    size_t len = strcspn(part, ".");
+
+    // We are going to put the '.' separator back for all components except the first.
+    if (*ptr == '\0') {
+      if (std::distance(reversed, ptr) < len) {
+        return NULL;
+      }
+    } else {
+      if (std::distance(reversed, ptr) < len + 1) {
+        return NULL;
+      }
+      *(--ptr) = '.';
+    }
+
+    ptr -= len;
+    memcpy(ptr, part, len);
+
+    // Skip to the next domain component. This will take us to either a '.' or a NUL.
+    // If it's a '.' we need to skip over it.
+    part += len;
+    if (*part == '.') {
+      ++part;
+    }
+  }
+
+  return ptr;
+}
+
+SSLContextStorage::SSLContextStorage()
+  :wildcards(), hostnames(ink_hash_table_create(InkHashTableKeyType_String))
+{
+}
+
+SSLContextStorage::~SSLContextStorage()
+{
+  ink_hash_table_destroy(this->hostnames);
+}
+
+bool
+SSLContextStorage::insert(SSL_CTX * ctx, const char * name)
+{
+  ats_wildcard_matcher wildcard;
+
+  if (wildcard.match(name)) {
+    // We turn wildcards into the reverse DNS form, then insert them into the trie
+    // so that we can do a longest match lookup.
+    char namebuf[TS_MAX_HOST_NAME_LEN + 1];
+    char * reversed;
+
+    reversed = reverse_dns_name(name + 2, namebuf);
+    if (!reversed) {
+      Error("wildcard name '%s' is too long", name);
+      return false;
+    }
+
+    Debug("indexed wildcard certificate for '%s' as '%s'", name, reversed);
+    this->wildcards.Insert(reversed, new SslEntry(ctx), 0 /* rank */, -1 /* keylen */);
+  } else {
+    ink_hash_table_insert(this->hostnames, name, (void *)ctx);
+  }
+
+  return true;
+}
+
+SSL_CTX *
+SSLContextStorage::lookup(const char * name) const
+{
+  InkHashTableValue value;
+
+  if (ink_hash_table_lookup(const_cast<InkHashTable *>(this->hostnames), name, &value)) {
+    return (SSL_CTX *)value;
+  }
+
+  if (!this->wildcards.Empty()) {
+    char namebuf[TS_MAX_HOST_NAME_LEN + 1];
+    char * reversed;
+    SslEntry * entry;
+
+    reversed = reverse_dns_name(name, namebuf);
+    if (!reversed) {
+      Error("failed to reverse hostname name '%s' is too long", name);
+      return NULL;
+    }
+
+    entry = this->wildcards.Search(reversed);
+    if (entry) {
+      return entry->ctx;
+    }
+  }
+
+  return NULL;
+}
+
+#if TS_HAS_TESTS
+
+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 * foo = SSL_CTX_new(methods);
+
+  *pstatus = REGRESSION_TEST_PASSED;
+
+  tb.check(storage.insert(foo, "www.foo.com"), "insert host context");
+  tb.check(storage.insert(wild, "*.wild.com"), "insert wildcard context");
+  tb.check(storage.insert(notwild, "*.notwild.com"), "insert wildcard context");
+
+  // Basic wildcard cases.
+  tb.check(storage.lookup("a.wild.com") == wild, "wildcard lookup for a.wild.com");
+  tb.check(storage.lookup("b.wild.com") == wild, "wildcard lookup for b.wild.com");
+  tb.check(storage.lookup("wild.com") == wild, "wildcard lookup for wild.com");
+
+  // Varify that wildcard does longest match.
+  tb.check(storage.lookup("a.notwild.com") == notwild, "wildcard lookup for a.notwild.com");
+  tb.check(storage.lookup("notwild.com") == notwild, "wildcard lookup for notwild.com");
+
+  // Basic hostname cases.
+  tb.check(storage.lookup("www.foo.com") == foo, "host lookup for www.foo.com");
+  tb.check(storage.lookup("www.bar.com") == NULL, "host lookup for www.bar.com");
+
+  // XXX Stop free'ing these once SSLContextStorage does it.
+  SSL_CTX_free(wild);
+  SSL_CTX_free(notwild);
+  SSL_CTX_free(foo);
+}
+
+#endif // TS_HAS_TESTS

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0dcad6ea/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 727e170..33ebe64 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -494,7 +494,7 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
         Debug("ssl", "setting SNI callbacks");
         SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback);
         SSL_CTX_set_tlsext_servername_arg(ctx, &sslCertLookup);
-#endif
+#endif /* TS_USE_TLS_SNI */
       }
 
       ssl = make_ssl_connection(ctx, this);