trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bc...@apache.org
Subject git commit: TS-1467: Disable client initiated renegotiation (SSL) DDoS by default
Date Fri, 31 Jan 2014 00:32:11 GMT
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 <bcall@apache.org>
Authored: Thu Jan 30 16:31:23 2014 -0800
Committer: Bryan Call <bcall@apache.org>
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
   //##############################################################################


Mime
View raw message