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 <[email protected]> Authored: Tue Jan 28 09:40:48 2014 -0800 Committer: James Peach <[email protected]> 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 <[email protected]> + *) [TS-2501] Refactor and improve performance for the case without expansions. Review: Alexey Ivanov <[email protected]>. 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();
