trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [05/12] git commit: TS-2031: Prevent duplicate SSL SNI name registration
Date Thu, 30 Jan 2014 23:31:18 GMT
TS-2031: Prevent duplicate SSL SNI name registration

If you have two certs that has the same CNs, the last one wins in
the SNI negotiation. We not detect this case, issue a warning and
preserve the existing registration.


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

Branch: refs/heads/5.0.x
Commit: 063b320a72bafe837ff018a04912b6632984084d
Parents: e8f18ea
Author: Feifei Cai <ffcai@yahoo-inc.com>
Authored: Tue Jan 28 09:40:48 2014 -0800
Committer: James Peach <jpeach@apache.org>
Committed: Tue Jan 28 09:40:48 2014 -0800

----------------------------------------------------------------------
 CHANGES                       |  3 +++
 iocore/net/SSLCertLookup.cc   | 30 +++++++++++++++++++++++++-----
 iocore/net/SSLUtils.cc        |  4 ++--
 iocore/net/test_certlookup.cc | 13 +++++++------
 4 files changed, 37 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/063b320a/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index f71c4a2..7fddf93 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 4.2.0
 
+  *) [TS-2031] Prevent duplicate SSL SNI name registration.
+   Author: Feifei Cai <ffcai@yahoo-inc.com>
+
   *) [TS-2501] Refactor and improve performance for the case without
    expansions. Review: Alexey Ivanov <aivanov@linkedin.com>.
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/063b320a/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index fe14369..df77418 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -216,7 +216,7 @@ bool
 SSLContextStorage::insert(SSL_CTX * ctx, const char * name)
 {
   ats_wildcard_matcher wildcard;
-  bool inserted = true;
+  bool inserted = false;
 
   if (wildcard.match(name)) {
     // We turn wildcards into the reverse DNS form, then insert them into the trie
@@ -231,17 +231,37 @@ SSLContextStorage::insert(SSL_CTX * ctx, const char * name)
       return false;
     }
 
-    Debug("ssl", "indexed wildcard certificate for '%s' as '%s' with SSL_CTX %p", name, reversed,
ctx);
     entry = new SSLEntry(ctx);
     inserted = this->wildcards.Insert(reversed, entry, 0 /* rank */, -1 /* keylen */);
-    if (inserted) {
-      entry.release();
+    if (!inserted) {
+      SSLEntry * found;
+
+      // We fail to insert, so the longest wildcard match search should return the full match
value.
+      found = this->wildcards.Search(reversed);
+      if (found != NULL && found->ctx != ctx) {
+        Warning("previously indexed wildcard certificate for '%s' as '%s', cannot index it
with SSL_CTX %p now",
+            name, reversed, ctx);
+      }
+
+      goto done;
     }
+
+    Debug("ssl", "indexed wildcard certificate for '%s' as '%s' with SSL_CTX %p", name, reversed,
ctx);
+    entry.release();
   } else {
-    Debug("ssl", "indexed '%s' with SSL_CTX %p", name, ctx);
+    InkHashTableValue value;
+
+    if (ink_hash_table_lookup(this->hostnames, name, &value) && (void *)ctx
!= value) {
+      Warning("previously indexed '%s' with SSL_CTX %p, cannot index it with SSL_CTX %p now",
name, value, ctx);
+      goto done;
+    }
+
+    inserted = true;
     ink_hash_table_insert(this->hostnames, name, (void *)ctx);
+    Debug("ssl", "indexed '%s' with SSL_CTX %p", name, ctx);
   }
 
+done:
   // Keep a unique reference to the SSL_CTX, so that we can free it later. Since we index
by name, multiple
   // certificates can be indexed for the same name. If this happens, we will overwrite the
previous pointer
   // and leak a context. So if we insert a certificate, keep an ownership reference to it.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/063b320a/iocore/net/SSLUtils.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 833ac7d..68f017a 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -611,8 +611,8 @@ asn1_strdup(ASN1_STRING * s)
 }
 
 // Given a certificate and it's corresponding SSL_CTX context, insert hash
-// table aliases for all of the subject and subjectAltNames. Note that we don't
-// deal with wildcards (yet).
+// table aliases for subject CN and subjectAltNames DNS without wildcard,
+// insert trie aliases for those with wildcard.
 static void
 ssl_index_certificate(SSLCertLookup * lookup, SSL_CTX * ctx, const char * certfile)
 {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/063b320a/iocore/net/test_certlookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/test_certlookup.cc b/iocore/net/test_certlookup.cc
index e0ba053..9a78a5f 100644
--- a/iocore/net/test_certlookup.cc
+++ b/iocore/net/test_certlookup.cc
@@ -56,12 +56,12 @@ REGRESSION_TEST(SSLCertificateLookup)(RegressionTest* t, int /* atype
ATS_UNUSED
   box.check(lookup.insert(notwild, "*.notwild.com"), "insert wildcard context");
   box.check(lookup.insert(b_notwild, "*.b.notwild.com"), "insert wildcard context");
 
-  // XXX inserting the same certificate multiple times ought to always fail, but it doesn't
-  // when we store a hash value.
-  lookup.insert(foo, "www.foo.com");
-  lookup.insert(wild, "*.wild.com");
-  lookup.insert(notwild, "*.notwild.com");
-  lookup.insert(b_notwild, "*.b.notwild.com");
+  // To test name collisions, we need to shuffle the SSL_CTX's so that we try to
+  // index the same name with a different SSL_CTX.
+  box.check(lookup.insert(wild, "www.foo.com") == false, "insert host duplicate");
+  box.check(lookup.insert(foo, "*.wild.com") == false, "insert wildcard duplicate");
+  box.check(lookup.insert(b_notwild, "*.notwild.com") == false, "insert wildcard conext duplicate");
+  box.check(lookup.insert(notwild, "*.b.notwild.com") == false, "insert wildcard conext duplicate");
 
   // Basic wildcard cases.
   box.check(lookup.findInfoInHash("a.wild.com") == wild, "wildcard lookup for a.wild.com");
@@ -186,6 +186,7 @@ SSLReleaseContext(SSL_CTX * ctx)
 
 int main(int argc, const char ** argv)
 {
+  diags = new Diags(NULL, NULL, stdout);
   res_track_memory = 1;
 
   SSL_library_init();


Mime
View raw message