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 2014/01/28 18:43:21 UTC
git commit: TS-2031: Prevent duplicate SSL SNI name registration
Updated Branches:
refs/heads/master e8f18ea04 -> 063b320a7
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/master
Commit: 063b320a72bafe837ff018a04912b6632984084d
Parents: e8f18ea
Author: Feifei Cai <ff...@yahoo-inc.com>
Authored: Tue Jan 28 09:40:48 2014 -0800
Committer: James Peach <jp...@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 <ff...@yahoo-inc.com>
+
*) [TS-2501] Refactor and improve performance for the case without
expansions. Review: Alexey Ivanov <ai...@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();