You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ig...@apache.org on 2012/10/09 21:43:52 UTC
[1/4] git commit: [TS-1484] SSL-crashed every now and then with 3.2.0
+ SNI-fixes
Updated Branches:
refs/heads/3.2.x fd0fc8624 -> e640fd9fa
[TS-1484] SSL-crashed every now and then with 3.2.0 + SNI-fixes
Trunk: d6d07d8c43c084d5683b45c1efe9ed2bf9ef8635
3.2.x Patch: https://issues.apache.org/jira/secure/attachment/12546611/TS-1484-3.2.x.patch
review/test: jpeach, igalic, zwoop
backport: igalic
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/e640fd9f
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/e640fd9f
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/e640fd9f
Branch: refs/heads/3.2.x
Commit: e640fd9fab76737546c7bd2c38ebc5b647d2d2c3
Parents: fb6cf1c
Author: Igor Galić <i....@brainsware.org>
Authored: Tue Oct 9 21:42:58 2012 +0200
Committer: Igor Galić <i....@brainsware.org>
Committed: Tue Oct 9 21:42:58 2012 +0200
----------------------------------------------------------------------
CHANGES | 2 +
STATUS | 6 --
iocore/net/P_SSLCertLookup.h | 8 ++--
iocore/net/SSLCertLookup.cc | 98 ++++++++++++++++++++++++++-------
iocore/net/SSLNetVConnection.cc | 55 -------------------
lib/ts/Trie.h | 7 +--
6 files changed, 84 insertions(+), 92 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e640fd9f/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 82f0953..ee1240d 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 3.2.3
+ *) [TS-1484] SSL-crashed every now and then with 3.2.0 + SNI-fixes
+
*) [TS-1514] Fix collation in custom logging.
Author: bettydramit
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e640fd9f/STATUS
----------------------------------------------------------------------
diff --git a/STATUS b/STATUS
index 97ce02e..5d101fd 100644
--- a/STATUS
+++ b/STATUS
@@ -41,12 +41,6 @@ A list of all bugs open for the next development release can be found at
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
- *) SSL-crashed every now and then with 3.2.0 + SNI-fixes
- Trunk: d6d07d8c43c084d5683b45c1efe9ed2bf9ef8635
- 3.2.x Patch: https://issues.apache.org/jira/secure/attachment/12546611/TS-1484-3.2.x.patch
- Jira: https://issues.apache.org/jira/browse/TS-1484
- +1: jpeach, igalic, zwoop
-
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
[ New proposals should be added at the end of the list ]
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e640fd9f/iocore/net/P_SSLCertLookup.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLCertLookup.h b/iocore/net/P_SSLCertLookup.h
index da18345..ae65fd8 100644
--- a/iocore/net/P_SSLCertLookup.h
+++ b/iocore/net/P_SSLCertLookup.h
@@ -36,12 +36,12 @@ class SSLCertLookup
bool addInfoToHash(
const char *strAddr, const char *cert, const char *ca, const char *serverPrivateKey);
- char config_file_path[PATH_NAME_MAX];
- SslConfigParams *param;
- bool multipleCerts;
+ char config_file_path[PATH_NAME_MAX];
+ SslConfigParams * param;
+ bool multipleCerts;
SSLContextStorage * ssl_storage;
- SSL_CTX * ssl_default;
+ SSL_CTX * ssl_default;
public:
bool hasMultipleCerts() const { return multipleCerts; }
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e640fd9f/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 8438ef2..bce1124 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -45,6 +45,64 @@ typedef const SSL_METHOD * ink_ssl_method_t;
typedef SSL_METHOD * ink_ssl_method_t;
#endif
+#if TS_USE_TLS_SNI
+
+static int
+ssl_servername_callback(SSL * ssl, int * ad, void * arg)
+{
+ SSL_CTX * ctx = NULL;
+ SSLCertLookup * lookup = (SSLCertLookup *) arg;
+ const char * servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
+
+ Debug("ssl", "ssl=%p ad=%d lookup=%p server=%s", ssl, *ad, lookup, servername);
+
+ if (likely(servername)) {
+ ctx = lookup->findInfoInHash((char *)servername);
+ }
+
+ if (ctx == NULL) {
+ ctx = lookup->defaultContext();
+ }
+
+ if (ctx != NULL) {
+ SSL_set_SSL_CTX(ssl, ctx);
+ }
+
+ // At this point, we might have updated ctx based on the SNI lookup, or we might still have the
+ // original SSL context that we set when we accepted the connection.
+ ctx = SSL_get_SSL_CTX(ssl);
+ Debug("ssl", "found SSL context %p for requested name '%s'", ctx, servername);
+
+ if (ctx == NULL) {
+ return SSL_TLSEXT_ERR_NOACK;
+ }
+
+ // We need to return one of the SSL_TLSEXT_ERR constants. If we return an
+ // error, we can fill in *ad with an alert code to propgate to the
+ // client, see SSL_AD_*.
+ return SSL_TLSEXT_ERR_OK;
+}
+
+#endif /* TS_USE_TLS_SNI */
+
+static SSL_CTX *
+make_ssl_context(void * arg)
+{
+ SSL_CTX * ctx = NULL;
+ ink_ssl_method_t meth = NULL;
+
+ meth = SSLv23_server_method();
+ ctx = SSL_CTX_new(meth);
+
+#if TS_USE_TLS_SNI
+ Debug("ssl", "setting SNI callbacks with for ctx %p", ctx);
+ SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback);
+ SSL_CTX_set_tlsext_servername_arg(ctx, arg);
+#endif /* TS_USE_TLS_SNI */
+
+ return ctx;
+}
+
class SSLContextStorage
{
@@ -52,7 +110,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);
@@ -112,7 +170,14 @@ void
SSLCertLookup::init(SslConfigParams * p)
{
param = p;
- multipleCerts = buildTable();
+
+ this->multipleCerts = buildTable();
+
+ // We *must* have a default context even if it can't possibly work. The default context is used to bootstrap the SSL
+ // handshake so that we can subsequently do the SNI lookup to switch to the real context.
+ if (this->ssl_default == NULL) {
+ this->ssl_default = make_ssl_context(this);
+ }
}
bool
@@ -192,17 +257,6 @@ SSLCertLookup::buildTable()
line = tokLine(NULL, &tok_state);
} // while(line != NULL)
-/* if(num_el == 0)
- {
- Warning("%s No entries in %s. Using default server cert for all connections",
- moduleName, configFilePath);
- }
-
- if(is_debug_tag_set("ssl"))
- {
- Print();
- }
-*/
ats_free(file_buf);
return ret;
}
@@ -272,10 +326,9 @@ SSLCertLookup::addInfoToHash(
const char *strAddr, const char *cert,
const char *caCert, const char *serverPrivateKey)
{
- ink_ssl_method_t meth = NULL;
+ SSL_CTX * ctx;
- meth = SSLv23_server_method();
- SSL_CTX *ctx = SSL_CTX_new(meth);
+ ctx = make_ssl_context(this);
if (!ctx) {
SSLNetProcessor::logSSLError("Cannot create new server contex.");
return (false);
@@ -493,7 +546,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);
@@ -522,6 +575,7 @@ SSLContextStorage::lookup(const char * name) const
return NULL;
}
+ Debug("ssl", "attempting wildcard match for %s", reversed);
entry = this->wildcards.Search(reversed);
if (entry) {
return entry->ctx;
@@ -537,17 +591,18 @@ 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);
+ SSL_CTX * wild = make_ssl_context(NULL);
+ SSL_CTX * notwild = make_ssl_context(NULL);
+ SSL_CTX * b_notwild = make_ssl_context(NULL);
+ SSL_CTX * foo = make_ssl_context(NULL);
*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");
+ 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");
@@ -557,6 +612,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/e640fd9f/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index fd89cba..1df458e 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -24,10 +24,6 @@
#include "P_Net.h"
#include "P_SSLNextProtocolSet.h"
-#if HAVE_OPENSSL_TLS1_H
-#include <openssl/tls1.h>
-#endif
-
#define SSL_READ_ERROR_NONE 0
#define SSL_READ_ERROR 1
#define SSL_READ_READY 2
@@ -46,48 +42,6 @@ ClassAllocator<SSLNetVConnection> sslNetVCAllocator("sslNetVCAllocator");
// Private
//
-static SSL_CTX * ssl_default = SSL_CTX_new(SSLv23_server_method());
-
-#if TS_USE_TLS_SNI
-
-static int
-ssl_servername_callback(SSL * ssl, int * ad, void * arg)
-{
- SSL_CTX * ctx = NULL;
- SSLCertLookup * lookup = (SSLCertLookup *) arg;
- const char * servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
-
- Debug("ssl", "ssl=%p ad=%d lookup=%p server=%s", ssl, *ad, lookup, servername);
-
- if (likely(servername)) {
- ctx = lookup->findInfoInHash((char *)servername);
- }
-
- if (ctx == NULL) {
- ctx = lookup->defaultContext();
- }
-
- if (ctx != NULL) {
- SSL_set_SSL_CTX(ssl, ctx);
- }
-
- // At this point, we might have updated ctx based on the SNI lookup, or we might still have the
- // original SSL context that we set when we accepted the connection.
- ctx = SSL_get_SSL_CTX(ssl);
- Debug("ssl", "found SSL context %p for requested name '%s'", ctx, servername);
-
- if (ctx == NULL) {
- return SSL_TLSEXT_ERR_NOACK;
- }
-
- // We need to return one of the SSL_TLSEXT_ERR constants. If we return an
- // error, we can fill in *ad with an alert code to propgate to the
- // client, see SSL_AD_*.
- return SSL_TLSEXT_ERR_OK;
-}
-
-#endif /* TS_USE_TLS_SNI */
-
static SSL *
make_ssl_connection(SSL_CTX * ctx, SSLNetVConnection * netvc)
{
@@ -503,15 +457,6 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
if (ctx == NULL) {
ctx = sslCertLookup.defaultContext();
}
- if (ctx == NULL) {
- ctx = ssl_default;
- }
-
-#if TS_USE_TLS_SNI
- Debug("ssl", "setting SNI callbacks with initial ctx %p", ctx);
- SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback);
- SSL_CTX_set_tlsext_servername_arg(ctx, &sslCertLookup);
-#endif /* TS_USE_TLS_SNI */
this->ssl = make_ssl_connection(ctx, this);
if (this->ssl == NULL) {
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e640fd9f/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;