From: Christos Tsantilas Date: Tue, 24 Jan 2017 12:09:25 +0000 (+0200) Subject: Mitigate DoS attacks that use client-initiated SSL/TLS renegotiation. X-Git-Tag: M-staged-PR71~302 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=edb876ab9348039f9c709703104b6836f08c6edf;p=thirdparty%2Fsquid.git 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. --- diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc index b016085a75..3a6e0a48f2 100644 --- a/src/ssl/bio.cc +++ b/src/ssl/bio.cc @@ -20,6 +20,7 @@ #include "globals.h" #include "ip/Address.h" #include "parser/BinaryTokenizer.h" +#include "SquidTime.h" #include "ssl/bio.h" #if HAVE_OPENSSL_SSL_H @@ -167,15 +168,46 @@ Ssl::Bio::stateChanged(const SSL *ssl, int where, int ret) SSL_state_string(ssl) << " (" << SSL_state_string_long(ssl) << ")"); } +Ssl::ClientBio::ClientBio(const int anFd): + Bio(anFd), + holdRead_(false), + holdWrite_(false), + helloSize(0), + abortReason(nullptr) +{ + renegotiations.configure(10*1000); +} + void 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; @@ -187,6 +219,12 @@ Ssl::ClientBio::write(const char *buf, int size, BIO *table) 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 (holdRead_) { debugs(83, 7, "Hold flag is set, retry latter. (Hold " << size << "bytes)"); BIO_set_retry_read(table); diff --git a/src/ssl/bio.h b/src/ssl/bio.h index 9d7d261c69..f8df982bf3 100644 --- a/src/ssl/bio.h +++ b/src/ssl/bio.h @@ -9,6 +9,7 @@ #ifndef SQUID_SSL_BIO_H #define SQUID_SSL_BIO_H +#include "FadingCounter.h" #include "fd.h" #include "security/Handshake.h" @@ -69,7 +70,7 @@ protected: class ClientBio: public Bio { public: - explicit ClientBio(const int anFd): Bio(anFd), holdRead_(false), holdWrite_(false), helloSize(0) {} + explicit ClientBio(const int anFd); /// The ClientBio version of the Ssl::Bio::stateChanged method /// When the client hello message retrieved, fill the @@ -89,9 +90,19 @@ public: /// by the caller. void setReadBufData(SBuf &data) {rbuf = data;} private: + /// 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; + bool holdRead_; ///< The read hold state of the bio. bool holdWrite_; ///< The write hold state of the bio. int helloSize; ///< The SSL hello message sent by client size + 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 diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 08a9eb1b01..8cecb599c8 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -501,34 +501,12 @@ Ssl::Initialize(void) ssl_ex_index_ssl_untrusted_chain = SSL_get_ex_new_index(0, (void *) "ssl_untrusted_chain", NULL, NULL, &ssl_free_CertChain); } -#if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) && (OPENSSL_VERSION_NUMBER < 0x10100000L) -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 void -maybeDisableRenegotiate(Security::ContextPointer &ctx) -{ -#if defined(SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) && (OPENSSL_VERSION_NUMBER < 0x10100000L) - SSL_CTX_set_info_callback(ctx.get(), ssl_info_cb); -#endif -} - static bool configureSslContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) { int ssl_error; SSL_CTX_set_options(ctx.get(), port.secure.parsedOptions); - maybeDisableRenegotiate(ctx); - if (port.sslContextSessionId) SSL_CTX_set_session_id_context(ctx.get(), (const unsigned char *)port.sslContextSessionId, strlen(port.sslContextSessionId)); @@ -656,8 +634,6 @@ Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &pee SSL_CTX_set_options(ctx.get(), options); - maybeDisableRenegotiate(ctx); - if (!peer.sslCipher.isEmpty()) { debugs(83, 5, "Using chiper suite " << peer.sslCipher << ".");