Updated Branches: refs/heads/master 2089e76a4 -> d43b5d685
TS-1467: Disable client initiated renegotiation (SSL) DDoS by default Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/d43b5d68 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/d43b5d68 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/d43b5d68 Branch: refs/heads/master Commit: d43b5d685e55795a755413b92d3a8827b86c4a03 Parents: 2089e76 Author: Bryan Call <[email protected]> Authored: Thu Jan 30 16:31:23 2014 -0800 Committer: Bryan Call <[email protected]> Committed: Thu Jan 30 16:31:23 2014 -0800 ---------------------------------------------------------------------- CHANGES | 2 ++ .../configuration/records.config.en.rst | 6 ++++ iocore/net/P_SSLConfig.h | 1 + iocore/net/P_SSLNetVConnection.h | 11 +++++++ iocore/net/SSLConfig.cc | 4 ++- iocore/net/SSLNetVConnection.cc | 9 ++++++ iocore/net/SSLUtils.cc | 33 ++++++++++++++++++-- mgmt/RecordsConfig.cc | 3 +- 8 files changed, 65 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 2be803f..3877c68 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ -*- coding: utf-8 -*- Changes with Apache Traffic Server 4.2.0 + *) [TS-1467] Disable client initiated renegotiation (SSL) DDoS by default + *) [TS-2538] Cleanup of ProcessMutex (unused) and InkMutex (dupe of ink_mutex). We now use ink_mutex consistently. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/doc/reference/configuration/records.config.en.rst ---------------------------------------------------------------------- diff --git a/doc/reference/configuration/records.config.en.rst b/doc/reference/configuration/records.config.en.rst index c0d78a5..96e4a99 100644 --- a/doc/reference/configuration/records.config.en.rst +++ b/doc/reference/configuration/records.config.en.rst @@ -2061,6 +2061,12 @@ SSL Termination to the Strict-Transport-Security header. proxy.config.ssl.hsts_max_age needs to be set to a non ``-1`` value for this configuration to take effect. +.. ts:cv:: CONFIG proxy.config.ssl.allow_client_renegotiation INT 0 + + This configuration specifies whether the client is able to initiate + renegotiation of the SSL connection. The default of ``0``, means + the client can't initiate renegotiation. + Client-Related Configuration ---------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/iocore/net/P_SSLConfig.h ---------------------------------------------------------------------- diff --git a/iocore/net/P_SSLConfig.h b/iocore/net/P_SSLConfig.h index b258b6c..d7d98ce 100644 --- a/iocore/net/P_SSLConfig.h +++ b/iocore/net/P_SSLConfig.h @@ -77,6 +77,7 @@ struct SSLConfigParams : public ConfigInfo long ssl_ctx_options; static int ssl_maxrecord; + static bool ssl_allow_client_renegotiation; void initialize(); void cleanup(); http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/iocore/net/P_SSLNetVConnection.h ---------------------------------------------------------------------- diff --git a/iocore/net/P_SSLNetVConnection.h b/iocore/net/P_SSLNetVConnection.h index 990a8dc..e4734b2 100644 --- a/iocore/net/P_SSLNetVConnection.h +++ b/iocore/net/P_SSLNetVConnection.h @@ -110,12 +110,23 @@ public: return npnEndpoint; } + bool getSSLClientRenegotiationAbort() const + { + return sslClientRenegotiationAbort; + }; + + void setSSLClientRenegotiationAbort(bool state) + { + sslClientRenegotiationAbort = state; + }; + private: SSLNetVConnection(const SSLNetVConnection &); SSLNetVConnection & operator =(const SSLNetVConnection &); bool sslHandShakeComplete; bool sslClientConnection; + bool sslClientRenegotiationAbort; const SSLNextProtocolSet * npnSet; Continuation * npnEndpoint; }; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/iocore/net/SSLConfig.cc ---------------------------------------------------------------------- diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc index e35bd58..9a20883 100644 --- a/iocore/net/SSLConfig.cc +++ b/iocore/net/SSLConfig.cc @@ -42,6 +42,7 @@ int SSLConfig::configid = 0; int SSLCertificateConfig::configid = 0; int SSLConfigParams::ssl_maxrecord = 0; +bool SSLConfigParams::ssl_allow_client_renegotiation = false; static ConfigUpdateHandler<SSLCertificateConfig> * sslCertUpdate; @@ -232,11 +233,12 @@ SSLConfigParams::initialize() ats_free_null(ssl_client_private_key_filename); ats_free_null(ssl_client_private_key_path); - REC_ReadConfigStringAlloc(clientCACertFilename, "proxy.config.ssl.client.CA.cert.filename"); REC_ReadConfigStringAlloc(clientCACertRelativePath, "proxy.config.ssl.client.CA.cert.path"); set_paths_helper(clientCACertRelativePath, clientCACertFilename, &clientCACertPath, &clientCACertFilename); ats_free(clientCACertRelativePath); + + REC_ReadConfigInt32(ssl_allow_client_renegotiation, "proxy.config.ssl.allow_client_renegotiation"); } void http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/iocore/net/SSLNetVConnection.cc ---------------------------------------------------------------------- diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc index 0dd3f07..199b6ce 100644 --- a/iocore/net/SSLNetVConnection.cc +++ b/iocore/net/SSLNetVConnection.cc @@ -191,6 +191,13 @@ SSLNetVConnection::net_read_io(NetHandler *nh, EThread *lthread) MIOBufferAccessor &buf = s->vio.buffer; int64_t ntodo = s->vio.ntodo(); + if (sslClientRenegotiationAbort == true) { + this->read.triggered = 0; + readSignalError(nh, (int)r); + Debug("ssl", "[SSLNetVConnection::net_read_io] client renegotiation setting read signal error"); + return; + } + MUTEX_TRY_LOCK_FOR(lock, s->vio.mutex, lthread, s->vio._cont); if (!lock) { readReschedule(nh); @@ -435,6 +442,7 @@ SSLNetVConnection::load_buffer_and_write(int64_t towrite, int64_t &wattempted, i SSLNetVConnection::SSLNetVConnection(): sslHandShakeComplete(false), sslClientConnection(false), + sslClientRenegotiationAbort(false), npnSet(NULL), npnEndpoint(NULL) { @@ -465,6 +473,7 @@ SSLNetVConnection::free(EThread * t) { } sslHandShakeComplete = false; sslClientConnection = false; + sslClientRenegotiationAbort = false; npnSet = NULL; npnEndpoint= NULL; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/iocore/net/SSLUtils.cc ---------------------------------------------------------------------- diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc index 68f017a..eeb1fbc 100644 --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -170,7 +170,14 @@ ssl_servername_callback(SSL * ssl, int * ad, void * arg) const char * servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); SSLNetVConnection * netvc = (SSLNetVConnection *)SSL_get_app_data(ssl); - Debug("ssl", "ssl=%p ad=%d lookup=%p server=%s", ssl, *ad, lookup, servername); + Debug("ssl", "ssl_servername_callback ssl=%p ad=%d lookup=%p server=%s handshake_complete=%d", ssl, *ad, lookup, servername, + netvc->getSSLHandShakeComplete()); + + // catch the client renegotiation early on + if (SSLConfigParams::ssl_allow_client_renegotiation == false && netvc->getSSLHandShakeComplete()) { + Debug("ssl", "ssl_servername_callback trying to renegotiate from the client"); + return SSL_TLSEXT_ERR_ALERT_FATAL; + } // The incoming SSL_CTX is either the one mapped from the inbound IP address or the default one. If we // don't find a name-based match at this point, we *do not* want to mess with the context because we've @@ -193,7 +200,7 @@ ssl_servername_callback(SSL * ssl, int * ad, void * arg) } ctx = SSL_get_SSL_CTX(ssl); - Debug("ssl", "found SSL context %p for requested name '%s'", ctx, servername); + Debug("ssl", "ssl_servername_callback found SSL context %p for requested name '%s'", ctx, servername); if (ctx == NULL) { return SSL_TLSEXT_ERR_NOACK; @@ -662,6 +669,26 @@ ssl_index_certificate(SSLCertLookup * lookup, SSL_CTX * ctx, const char * certfi X509_free(cert); } +// This callback function is executed while OpenSSL processes the SSL +// handshake and does SSL record layer stuff. It's used to trap +// client-initiated renegotiations +static void +ssl_callback_info(const SSL *ssl, int where, int ret) +{ + Debug("ssl", "ssl_callback_info ssl: %p where: %d ret: %d", ssl, where, ret); + SSLNetVConnection * netvc = (SSLNetVConnection *)SSL_get_app_data(ssl); + + if ((where & SSL_CB_ACCEPT_LOOP) && netvc->getSSLHandShakeComplete() == true && + SSLConfigParams::ssl_allow_client_renegotiation == false) { + int state = SSL_get_state(ssl); + + if (state == SSL3_ST_SR_CLNT_HELLO_A || state == SSL23_ST_SR_CLNT_HELLO_A) { + netvc->setSSLClientRenegotiationAbort(true); + Debug("ssl", "ssl_callback_info trying to renegotiate from the client"); + } + } +} + static bool ssl_store_ssl_context( const SSLConfigParams * params, @@ -682,6 +709,8 @@ ssl_store_ssl_context( return false; } + SSL_CTX_set_info_callback(ctx, ssl_callback_info); + #if TS_USE_TLS_NPN SSL_CTX_set_next_protos_advertised_cb(ctx, SSLNetVConnection::advertise_next_protocol, NULL); #endif /* TS_USE_TLS_NPN */ http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d43b5d68/mgmt/RecordsConfig.cc ---------------------------------------------------------------------- diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index abae558..76028c9 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1279,7 +1279,8 @@ RecordElement RecordsConfig[] = { , {RECT_CONFIG, "proxy.config.ssl.hsts_include_subdomains", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} , - + {RECT_CONFIG, "proxy.config.ssl.allow_client_renegotiation", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} + , //############################################################################## //# ICP Configuration //##############################################################################
