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 2012/11/17 07:23:21 UTC
[4/4] git commit: TS-1551: reload ssl_multicert.config at runtime
TS-1551: reload ssl_multicert.config at runtime
Reload ssl_multicert.config and the corresponding SSL certificates
in response to traffic_line -x.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/c63c4dd2
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/c63c4dd2
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/c63c4dd2
Branch: refs/heads/master
Commit: c63c4dd225d1fa05d2735d5a8b100d07cd80a29d
Parents: 54b82f2
Author: Ethan Lai <yz...@yahoo.com>
Authored: Mon Nov 12 10:29:31 2012 -0800
Committer: James Peach <jp...@apache.org>
Committed: Fri Nov 16 22:22:13 2012 -0800
----------------------------------------------------------------------
CHANGES | 3 +
iocore/net/P_SSLCertLookup.h | 40 +++++-
iocore/net/SSLCertLookup.cc | 245 ++++++++++++++++++++++++++++++---
iocore/net/SSLNetProcessor.cc | 8 +-
iocore/net/SSLNetVConnection.cc | 8 +-
mgmt/Main.cc | 2 +-
6 files changed, 268 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c63c4dd2/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 902edb3..80bb024 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 3.3.1
+ *) [TS-1551] reload ssl_multicert.config at runtime
+ Author: Ethan Lai <yz...@yahoo.com>
+
*) [TS-1572] Plugin response status change can trigger ATS assertion
Author: Uri Shachar
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c63c4dd2/iocore/net/P_SSLCertLookup.h
----------------------------------------------------------------------
diff --git a/iocore/net/P_SSLCertLookup.h b/iocore/net/P_SSLCertLookup.h
index a13ff8b..cd2e7dc 100644
--- a/iocore/net/P_SSLCertLookup.h
+++ b/iocore/net/P_SSLCertLookup.h
@@ -26,27 +26,50 @@
#include "libts.h"
#include "P_SSLNetProcessor.h"
+struct SSLCertConfigInfo
+{
+ volatile int m_refcount;
+
+ virtual ~SSLCertConfigInfo()
+ { }
+};
+
+
+class SSLCertConfigProcessor
+{
+public:
+ SSLCertConfigProcessor();
+
+ unsigned int set(unsigned int id, SSLCertConfigInfo * info);
+ SSLCertConfigInfo *get(unsigned int id);
+ void release(unsigned int id, SSLCertConfigInfo * data);
+
+public:
+ SSLCertConfigInfo *infos[MAX_CONFIGS];
+ int ninfos;
+};
+
class SSLContextStorage;
-class SSLCertLookup
+class SSLCertLookup : public SSLCertConfigInfo
{
- bool buildTable(const SSLConfigParams * param);
const char *extractIPAndCert(
matcher_line * line_info, char **addr, char **cert, char **ca, char **priKey) const;
bool addInfoToHash(
const SSLConfigParams * param,
const char *strAddr, const char *cert, const char *ca, const char *serverPrivateKey);
- char config_file_path[PATH_NAME_MAX];
- bool multipleCerts;
+ char config_file_path[PATH_NAME_MAX];
SSLContextStorage * ssl_storage;
SSL_CTX * ssl_default;
+ static int id;
+
public:
- bool hasMultipleCerts() const { return multipleCerts; }
+ bool buildTable(const SSLConfigParams * param);
+ void checkDefaultContext();
- void init(const SSLConfigParams * param);
SSL_CTX *findInfoInHash(const char * address) const;
// Return the last-resort default TLS context if there is no name or address match.
@@ -54,6 +77,11 @@ public:
SSLCertLookup();
~SSLCertLookup();
+
+ static void startup();
+ static void reconfigure();
+ static SSLCertLookup * acquire();
+ static void release(SSLCertLookup *p);
};
extern SSLCertLookup sslCertLookup;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c63c4dd2/iocore/net/SSLCertLookup.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLCertLookup.cc b/iocore/net/SSLCertLookup.cc
index 9a74215..62e4b2f 100644
--- a/iocore/net/SSLCertLookup.cc
+++ b/iocore/net/SSLCertLookup.cc
@@ -45,6 +45,12 @@ typedef const SSL_METHOD * ink_ssl_method_t;
typedef SSL_METHOD * ink_ssl_method_t;
#endif
+#define SSL_MULTICERT_TIMEOUT HRTIME_SECOND * 5
+SSLCertConfigProcessor sslCertConfigProcessor;
+static Ptr<ProxyMutex> ssl_reconfig_mutex = NULL;
+int SSLCertLookup::id = 0;
+int sslCertFile_CB(const char *name, RecDataT data_type, RecData data, void *cookie);
+
#if TS_USE_TLS_SNI
static int
@@ -122,16 +128,8 @@ public:
bool insert(SSL_CTX * ctx, const char * name);
SSL_CTX * lookup(const char * name) const;
-
- // XXX although we take "ownership" of the SSL_CTX, we never actually free them when we are
- // done. Currently, this doesn't matter because altering the set of SSL certificates requires
- // a restart. When this changes, we will need to keep track of the SSL_CTX pointers exactly
- // once and call SSL_CTX_free() on them.
-
};
-SSLCertLookup sslCertLookup;
-
static void
insert_ssl_certificate(SSLContextStorage *, SSL_CTX *, const char *);
@@ -147,7 +145,7 @@ static const matcher_tags sslCertTags = {
};
SSLCertLookup::SSLCertLookup()
- : multipleCerts(false), ssl_storage(NEW(new SSLContextStorage())), ssl_default(NULL)
+ : ssl_storage(NEW(new SSLContextStorage())), ssl_default(NULL)
{
*config_file_path = '\0';
}
@@ -164,15 +162,40 @@ SSLCertLookup::findInfoInHash(const char * address) const
}
void
-SSLCertLookup::init(const SSLConfigParams * param)
+SSLCertLookup::startup()
{
- this->multipleCerts = buildTable(param);
+ ssl_reconfig_mutex = new_ProxyMutex();
+ reconfigure();
- // 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);
- }
+ REC_RegisterConfigUpdateFunc("proxy.config.ssl.server.multicert.filename", sslCertFile_CB, NULL);
+ REC_RegisterConfigUpdateFunc("proxy.config.ssl.server.cert.path", sslCertFile_CB, NULL);
+ REC_RegisterConfigUpdateFunc("proxy.config.ssl.server.private_key.path", sslCertFile_CB, NULL);
+ REC_RegisterConfigUpdateFunc("proxy.config.ssl.server.cert_chain.filename", sslCertFile_CB, NULL);
+}
+
+void
+SSLCertLookup::reconfigure()
+{
+ SSLConfig::startup();
+ SSLConfig::scoped_config param;
+
+ SSLCertLookup *lookup = NEW(new SSLCertLookup());
+ lookup->buildTable(param);
+ lookup->checkDefaultContext();
+
+ id = sslCertConfigProcessor.set(id, lookup);
+}
+
+SSLCertLookup *
+SSLCertLookup::acquire()
+{
+ return ((SSLCertLookup *) sslCertConfigProcessor.get(id));
+}
+
+void
+SSLCertLookup::release(SSLCertLookup *sslCertTable)
+{
+ sslCertConfigProcessor.release(id, sslCertTable);
}
bool
@@ -256,6 +279,16 @@ SSLCertLookup::buildTable(const SSLConfigParams * param)
return ret;
}
+void
+SSLCertLookup::checkDefaultContext()
+{
+ // 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);
+ }
+}
+
const char *
SSLCertLookup::extractIPAndCert(matcher_line * line_info, char **addr, char **cert, char **ca, char **priKey) const
{
@@ -464,8 +497,10 @@ insert_ssl_certificate(SSLContextStorage * storage, SSL_CTX * ctx, const char *
switch (name->type) {
case GEN_DNS:
dns = asn1_strdup(name->d.dNSName);
- Debug("ssl", "mapping '%s' to certificate %s", dns, certfile);
- storage->insert(ctx, dns);
+ if (!storage->lookup(dns)) {
+ Debug("ssl", "mapping '%s' to certificate %s", dns, certfile);
+ storage->insert(ctx, dns);
+ }
ats_free(dns);
break;
}
@@ -522,6 +557,33 @@ SSLContextStorage::SSLContextStorage()
SSLContextStorage::~SSLContextStorage()
{
+ InkHashTableEntry *e;
+ InkHashTableIteratorState state;
+ InkHashTable *ht_ptr, *uniqueCtx;
+
+ uniqueCtx = ink_hash_table_create(InkHashTableKeyType_String);
+
+ // SSL_CTX_free decrease SSL_CTX internal reference count
+ // could NOT assign NULL to *value since this ctx might be used by some other live SSL instances
+
+ // Traverse hash table to find unique SSL context pointers first
+ ht_ptr = this->hostnames;
+ for (e = ink_hash_table_iterator_first(ht_ptr, &state); e != NULL; e = ink_hash_table_iterator_next(ht_ptr, &state)) {
+ InkHashTableValue value = ink_hash_table_entry_value(ht_ptr, e);
+ char ptr[32];
+ snprintf(ptr, 31, "%p", value);
+ ink_hash_table_insert(uniqueCtx, ptr, (void *)value);
+ }
+
+ // release these unique SSL contexts
+ ht_ptr = uniqueCtx;
+ for (e = ink_hash_table_iterator_first(ht_ptr, &state); e != NULL; e = ink_hash_table_iterator_next(ht_ptr, &state)) {
+ InkHashTableValue value = ink_hash_table_entry_value(ht_ptr, e);
+ if (value != NULL && *(InkHashTableValue *)value) {
+ SSL_CTX_free((SSL_CTX *)value);
+ }
+ }
+ ink_hash_table_destroy(uniqueCtx);
ink_hash_table_destroy(this->hostnames);
}
@@ -582,6 +644,148 @@ SSLContextStorage::lookup(const char * name) const
return NULL;
}
+class SSLCertConfigInfoReleaser:public Continuation
+{
+public:
+ SSLCertConfigInfoReleaser(unsigned int id, SSLCertConfigInfo * info)
+ : Continuation(new_ProxyMutex()), m_id(id), m_info(info)
+ {
+ SET_HANDLER(&SSLCertConfigInfoReleaser::handle_event);
+ }
+
+ int handle_event(int event, void *edata)
+ {
+ NOWARN_UNUSED(event);
+ NOWARN_UNUSED(edata);
+ sslCertConfigProcessor.release(m_id, m_info);
+ delete this;
+ return 0;
+ }
+
+public:
+ unsigned int m_id;
+ SSLCertConfigInfo *m_info;
+};
+
+SSLCertConfigProcessor::SSLCertConfigProcessor()
+ : ninfos(0)
+{
+ for (int i = 0; i < MAX_CONFIGS; i++) {
+ infos[i] = NULL;
+ }
+}
+
+unsigned int
+SSLCertConfigProcessor::set(unsigned int id, SSLCertConfigInfo * info)
+{
+ SSLCertConfigInfo *old_info;
+ int idx;
+
+ if (id == 0) {
+ id = ink_atomic_increment((int *) &ninfos, 1) + 1;
+ ink_assert(id != 0);
+ ink_assert(id <= MAX_CONFIGS);
+ }
+
+ info->m_refcount = 1;
+
+ if (id > MAX_CONFIGS) {
+ // invalid index
+ Error("[SSLCertConfigProcessor::set] invalid index");
+ return 0;
+ }
+
+ idx = id - 1;
+
+ do {
+ old_info = infos[idx];
+ } while (!ink_atomic_cas( &infos[idx], old_info, info));
+
+ if (old_info) {
+ eventProcessor.schedule_in(NEW(new SSLCertConfigInfoReleaser(id, old_info)), SSL_MULTICERT_TIMEOUT, ET_CACHE);
+ }
+
+ return id;
+}
+
+SSLCertConfigInfo *
+SSLCertConfigProcessor::get(unsigned int id)
+{
+ SSLCertConfigInfo *info;
+ int idx;
+
+ ink_assert(id != 0);
+ ink_assert(id <= MAX_CONFIGS);
+
+ if (id == 0 || id > MAX_CONFIGS) {
+ // return NULL, because we of an invalid index
+ return NULL;
+ }
+
+ idx = id - 1;
+ info = (SSLCertConfigInfo *) infos[idx];
+ if (ink_atomic_increment((int *) &info->m_refcount, 1) < 0) {
+ ink_assert(!"not reached");
+ }
+
+ return info;
+}
+
+void
+SSLCertConfigProcessor::release(unsigned int id, SSLCertConfigInfo * info)
+{
+ int val;
+ int idx;
+
+ ink_assert(id != 0);
+ ink_assert(id <= MAX_CONFIGS);
+
+ if (id == 0 || id > MAX_CONFIGS) {
+ // nothing to delete since we have an invalid index
+ return;
+ }
+
+ idx = id - 1;
+ val = ink_atomic_increment((int *) &info->m_refcount, -1);
+ if ((infos[idx] != info) && (val == 1)) {
+ delete info;
+ }
+}
+
+// struct SSLCert_UpdateContinuation
+//
+// Used to read the ssl_multicert.config file after the manager signals
+// a change
+//
+struct SSLCert_UpdateContinuation;
+typedef int (SSLCert_UpdateContinuation::*SSLCert_UpdContHandler) (int, void *);
+struct SSLCert_UpdateContinuation: public Continuation
+{
+ int file_update_handler(int etype, void *data)
+ {
+ NOWARN_UNUSED(etype);
+ NOWARN_UNUSED(data);
+ SSLCertLookup::reconfigure();
+ delete this;
+ return EVENT_DONE;
+ }
+ SSLCert_UpdateContinuation(ProxyMutex * m):Continuation(m)
+ {
+ SET_HANDLER((SSLCert_UpdContHandler) & SSLCert_UpdateContinuation::file_update_handler);
+ }
+};
+
+int
+sslCertFile_CB(const char *name, RecDataT data_type, RecData data, void *cookie)
+{
+ NOWARN_UNUSED(name);
+ NOWARN_UNUSED(data_type);
+ NOWARN_UNUSED(data);
+ NOWARN_UNUSED(cookie);
+ eventProcessor.schedule_imm(NEW(new SSLCert_UpdateContinuation(ssl_reconfig_mutex)), ET_CACHE);
+ return 0;
+}
+
#if TS_HAS_TESTS
REGRESSION_TEST(SSLHostLookup)(RegressionTest* t, int atype, int * pstatus)
@@ -614,11 +818,6 @@ REGRESSION_TEST(SSLHostLookup)(RegressionTest* t, int atype, int * pstatus)
// Basic hostname cases.
tb.check(storage.lookup("www.foo.com") == foo, "host lookup for www.foo.com");
tb.check(storage.lookup("www.bar.com") == NULL, "host lookup for www.bar.com");
-
- // XXX Stop free'ing these once SSLContextStorage does it.
- SSL_CTX_free(wild);
- SSL_CTX_free(notwild);
- SSL_CTX_free(foo);
}
#endif // TS_HAS_TESTS
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c63c4dd2/iocore/net/SSLNetProcessor.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetProcessor.cc b/iocore/net/SSLNetProcessor.cc
index 3c0bf55..fe4be05 100644
--- a/iocore/net/SSLNetProcessor.cc
+++ b/iocore/net/SSLNetProcessor.cc
@@ -133,19 +133,17 @@ SSLNetProcessor::reconfigure(void)
initSSLLocks();
}
- SSLConfig::scoped_config param;
-
- ink_assert(param);
-
if (HttpProxyPort::hasSSL()) {
// Only init server stuff if SSL is enabled in the config file
- sslCertLookup.init(param);
+ SSLCertLookup::startup();
}
// Enable client regardless of config file setttings as remap file
// can cause HTTP layer to connect using SSL. But only if SSL
// initialization hasn't failed already.
if (err == 0) {
+ SSLConfig::scoped_config param;
+ ink_assert(param);
err = initSSLClient(param);
if (err != 0)
logSSLError("Can't initialize the SSL client, HTTPS in remap rules will not function");
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c63c4dd2/iocore/net/SSLNetVConnection.cc
----------------------------------------------------------------------
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 6c3f45e..3f4e8ca 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -456,13 +456,15 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
safe_getsockname(get_socket(), &ip.sa, &namelen);
ats_ip_ntop(&ip.sa, buff, sizeof(buff));
- ctx = sslCertLookup.findInfoInHash(buff);
- Debug("ssl", "IP context is %p, default context %p", ctx, sslCertLookup.defaultContext());
+ SSLCertLookup *lookup = SSLCertLookup::acquire();
+ ctx = lookup->findInfoInHash(buff);
+ Debug("ssl", "IP context is %p, default context %p", ctx, lookup->defaultContext());
if (ctx == NULL) {
- ctx = sslCertLookup.defaultContext();
+ ctx = lookup->defaultContext();
}
this->ssl = make_ssl_connection(ctx, this);
+ SSLCertLookup::release(lookup);
if (this->ssl == NULL) {
Debug("ssl", "SSLNetVConnection::sslServerHandShakeEvent, ssl create failed");
SSLNetProcessor::logSSLError("SSL_StartHandShake");
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c63c4dd2/mgmt/Main.cc
----------------------------------------------------------------------
diff --git a/mgmt/Main.cc b/mgmt/Main.cc
index 06294e6..4247c84 100644
--- a/mgmt/Main.cc
+++ b/mgmt/Main.cc
@@ -1108,7 +1108,7 @@ fileUpdated(char *fname)
mgmt_log(stderr, "[fileUpdated] plugin.config file has been modified\n");
} else if (strcmp(fname, "ssl_multicert.config") == 0) {
- mgmt_log(stderr, "[fileUpdated] ssl_multicert.config file has been modified\n");
+ lmgmt->signalFileChange("proxy.config.ssl.server.multicert.filename");
} else if (strcmp(fname, "proxy.config.body_factory.template_sets_dir") == 0) {
lmgmt->signalFileChange("proxy.config.body_factory.template_sets_dir");