Updated Branches:
  refs/heads/master 492936c6c -> 758d8657b

TS-1380: SSL wildcard lookup doesn't find the longest match

Make sure that the Trie returns the longest match (where the rank
is of equal or lesser priority). Add a SSLcontextStorage unit test
to verify this.


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

Branch: refs/heads/master
Commit: 758d8657b012a671940f54951d5c6bc3be6056ec
Parents: 492936c
Author: James Peach <[email protected]>
Authored: Mon Jul 23 22:23:39 2012 -0700
Committer: James Peach <[email protected]>
Committed: Mon Jul 23 22:23:39 2012 -0700

----------------------------------------------------------------------
 CHANGES                     |    2 ++
 iocore/net/SSLCertLookup.cc |   10 +++++++---
 lib/ts/Trie.h               |    7 +------
 3 files changed, 10 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/758d8657/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 3eeedb9..052de74 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,7 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.3.0
+  *) [TS-1380] SSL wildcard lookup doesn't find the longest match
+
   *) [TS-1315] Fix URL parsing to handle non-HTTP schemes correctly.
 
   *) [TS-1322] CONNECT to parent proxy has URL with a trailing slash

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/758d8657/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 8c323a5..72b7bb7 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -52,7 +52,7 @@ class SSLContextStorage
   {
     explicit SslEntry(SSL_CTX * c) : ctx(c) {}
 
-    void Print() const { printf("%p/%p", this, ctx); }
+    void Print() const { Debug("ssl", "SslEntry=%p SSL_CTX=%p", this, ctx); }
 
     SSL_CTX * ctx;
     LINK(SslEntry, link);
@@ -500,7 +500,7 @@ SSLContextStorage::insert(SSL_CTX * ctx, const char * name)
       return false;
     }
 
-    Debug("indexed wildcard certificate for '%s' as '%s'", name, reversed);
+    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 {
     ink_hash_table_insert(this->hostnames, name, (void *)ctx);
@@ -529,7 +529,8 @@ SSLContextStorage::lookup(const char * name) const
       return NULL;
     }
 
-    entry = this->wildcards.Search(reversed);
+    Debug("ssl", "attempting wildcard match for %s", reversed);
+    entry = this->wildcards.Search(reversed, strlen(reversed) + 1);
     if (entry) {
       return entry->ctx;
     }
@@ -548,6 +549,7 @@ REGRESSION_TEST(SslHostLookup)(RegressionTest* t, int 
atype, int * pstatus)
 
   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);
 
   *pstatus = REGRESSION_TEST_PASSED;
@@ -555,6 +557,7 @@ REGRESSION_TEST(SslHostLookup)(RegressionTest* t, int 
atype, int * pstatus)
   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");
+  tb.check(storage.insert(b_notwild, "*.b.notwild.com"), "insert wildcard 
context");
 
   // Basic wildcard cases.
   tb.check(storage.lookup("a.wild.com") == wild, "wildcard lookup for 
a.wild.com");
@@ -564,6 +567,7 @@ REGRESSION_TEST(SslHostLookup)(RegressionTest* t, int 
atype, int * pstatus)
   // 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");
+  tb.check(storage.lookup("c.b.notwild.com") == b_notwild, "wildcard lookup 
for c.b.notwild.com");
 
   // Basic hostname cases.
   tb.check(storage.lookup("www.foo.com") == foo, "host lookup for 
www.foo.com");

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/758d8657/lib/ts/Trie.h
----------------------------------------------------------------------
diff --git a/lib/ts/Trie.h b/lib/ts/Trie.h
index eed9207..eb5363b 100644
--- a/lib/ts/Trie.h
+++ b/lib/ts/Trie.h
@@ -170,14 +170,9 @@ Trie<T>::Search(const char *key, int key_len /* = -1 */) 
const
       curr_node->Print("Trie::Search");
     }
     if (curr_node->occupied) {
-      if (!found_node) {
+      if (!found_node || curr_node->rank <= found_node->rank) {
         found_node = curr_node;
       }
-      else {
-        if (curr_node->rank < found_node->rank) {
-          found_node = curr_node;
-        }
-      }
     }
     if (i == key_len) {
       break;

Reply via email to