------------------------------------------------------------ revno: 14138 revision-id: squid3@treenet.co.nz-20170127161419-onou0izf4ednolnx parent: squid3@treenet.co.nz-20170127150005-d27qg46z0lm5ye8c author: Christos Tsantilas committer: Amos Jeffries branch nick: 3.5 timestamp: Sat 2017-01-28 05:14:19 +1300 message: Mitigate DoS attacks that use client-initiated SSL/TLS renegotiation. There is a well-known DoS attack using client-initiated SSL/TLS renegotiation. The severety or uniqueness of this attack method is disputed, but many believe it is serious/real. There is even a (disputed) CVE 2011-1473: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-1473 The old Squid code tried to disable client-initiated renegotiation, but it did not work reliably (or at all), depending on Squid version, due to OpenSSL API changes and conflicting SslBump callbacks. That code is now removed and client-initiated renegotiations are allowed. With this change, Squid aborts the TLS connection, with a level-1 ERROR message if the rate of client-initiated renegotiate requests exceeds 5 requests in 10 seconds (approximately). This protection and the rate limit are currently hard-coded but the rate is not expected to be exceeded under normal circumstances. This is a Measurement Factory project. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20170127161419-onou0izf4ednolnx # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: 127588acf702109066efb3a48fe6bac9b5d5cc10 # timestamp: 2017-01-27 16:30:20 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squid3@treenet.co.nz-20170127150005-\ # d27qg46z0lm5ye8c # # Begin patch === modified file 'src/Makefile.am' --- src/Makefile.am 2017-01-01 00:16:45 +0000 +++ src/Makefile.am 2017-01-27 16:14:19 +0000 @@ -1673,6 +1673,7 @@ tests/stub_ETag.cc \ EventLoop.cc \ event.cc \ + FadingCounter.cc \ fatal.h \ tests/stub_fatal.cc \ fd.h \ @@ -3177,6 +3178,7 @@ store_rebuild.h \ tests/stub_store_rebuild.cc \ tests/stub_store_stats.cc \ + FadingCounter.cc \ fatal.h \ tests/stub_fatal.cc \ fd.h \ @@ -3359,6 +3361,7 @@ ETag.cc \ EventLoop.cc \ event.cc \ + FadingCounter.cc \ fatal.h \ fatal.cc \ fd.h \ === modified file 'src/ssl/bio.cc' --- src/ssl/bio.cc 2017-01-01 00:16:45 +0000 +++ src/ssl/bio.cc 2017-01-27 16:14:19 +0000 @@ -175,6 +175,16 @@ rbuf.init(4096, 65536); } +Ssl::ClientBio::ClientBio(const int anFd): + Bio(anFd), + holdRead_(false), + holdWrite_(false), + helloState(atHelloNone), + abortReason(nullptr) +{ + renegotiations.configure(10*1000); +} + bool Ssl::ClientBio::isClientHello(int state) { @@ -194,11 +204,32 @@ Ssl::ClientBio::stateChanged(const SSL *ssl, int where, int ret) { Ssl::Bio::stateChanged(ssl, where, ret); + // detect client-initiated renegotiations DoS (CVE-2011-1473) + if (where & SSL_CB_HANDSHAKE_START) { + const int reneg = renegotiations.count(1); + + if (abortReason) + return; // already decided and informed the admin + + if (reneg > RenegotiationsLimit) { + abortReason = "renegotiate requests flood"; + debugs(83, DBG_IMPORTANT, "Terminating TLS connection [from " << fd_table[fd_].ipaddr << "] due to " << abortReason << ". This connection received " << + reneg << " renegotiate requests in the last " << + RenegotiationsWindow << " seconds (and " << + renegotiations.remembered() << " requests total)."); + } + } } int Ssl::ClientBio::write(const char *buf, int size, BIO *table) { + if (abortReason) { + debugs(83, 3, "BIO on FD " << fd_ << " is aborted"); + BIO_clear_retry_flags(table); + return -1; + } + if (holdWrite_) { BIO_set_retry_write(table); return 0; @@ -222,6 +253,12 @@ int Ssl::ClientBio::read(char *buf, int size, BIO *table) { + if (abortReason) { + debugs(83, 3, "BIO on FD " << fd_ << " is aborted"); + BIO_clear_retry_flags(table); + return -1; + } + if (helloState < atHelloReceived) { int bytes = readAndBuffer(buf, size, table, "TLS client Hello"); if (bytes <= 0) === modified file 'src/ssl/bio.h' --- src/ssl/bio.h 2017-01-01 00:16:45 +0000 +++ src/ssl/bio.h 2017-01-27 16:14:19 +0000 @@ -9,6 +9,7 @@ #ifndef SQUID_SSL_BIO_H #define SQUID_SSL_BIO_H +#include "FadingCounter.h" #include "fd.h" #include "SBuf.h" @@ -134,7 +135,7 @@ public: /// The ssl hello message read states typedef enum {atHelloNone = 0, atHelloStarted, atHelloReceived} HelloReadState; - explicit ClientBio(const int anFd): Bio(anFd), holdRead_(false), holdWrite_(false), helloState(atHelloNone) {} + explicit ClientBio(const int anFd); /// The ClientBio version of the Ssl::Bio::stateChanged method /// When the client hello message retrieved, fill the @@ -156,11 +157,22 @@ private: /// True if the SSL state corresponds to a hello message bool isClientHello(int state); + + /// approximate size of a time window for computing client-initiated renegotiation rate (in seconds) + static const time_t RenegotiationsWindow = 10; + + /// the maximum tolerated number of client-initiated renegotiations in RenegotiationsWindow + static const int RenegotiationsLimit = 5; + /// The futures retrieved from client SSL hello message Bio::sslFeatures features; bool holdRead_; ///< The read hold state of the bio. bool holdWrite_; ///< The write hold state of the bio. HelloReadState helloState; ///< The SSL hello read state + FadingCounter renegotiations; ///< client requested renegotiations limit control + + /// why we should terminate the connection during next TLS operation (or nil) + const char *abortReason; }; /// BIO node to handle socket IO for squid server side === modified file 'src/ssl/support.cc' --- src/ssl/support.cc 2017-01-01 00:16:45 +0000 +++ src/ssl/support.cc 2017-01-27 16:14:19 +0000 @@ -848,18 +848,6 @@ return dh; } -#if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) -static void -ssl_info_cb(const SSL *ssl, int where, int ret) -{ - (void)ret; - if ((where & SSL_CB_HANDSHAKE_DONE) != 0) { - // disable renegotiation (CVE-2009-3555) - ssl->s3->flags |= SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS; - } -} -#endif - static bool configureSslEECDH(SSL_CTX *sslContext, const char *curve) { @@ -889,10 +877,6 @@ int ssl_error; SSL_CTX_set_options(sslContext, port.sslOptions); -#if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) - SSL_CTX_set_info_callback(sslContext, ssl_info_cb); -#endif - if (port.sslContextSessionId) SSL_CTX_set_session_id_context(sslContext, (const unsigned char *)port.sslContextSessionId, strlen(port.sslContextSessionId)); @@ -1261,10 +1245,6 @@ SSL_CTX_set_options(sslContext, Ssl::parse_options(options)); -#if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) - SSL_CTX_set_info_callback(sslContext, ssl_info_cb); -#endif - if (cipher) { debugs(83, 5, "Using chiper suite " << cipher << ".");