[squid-dev] [PATCH] Mitigate DoS attacks that use client-initiated SSL/TLS renegotiation.

[Amos Jeffries]

On 23/01/2017 1:03 a.m., Christos Tsantilas wrote:
> 
> There is a well-known DoS attack using client-initiated SSL/TLS
> renegotiation. The severity 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
> 


Thank you.

In Ssl::ClientBio::stateChanged:

* please make the initial comment:
   // detect client-initiated renegotiation DoS (CVE-2011-1473)

* The counting logic does not seem right:

> +        const time_t currentTime = getCurrentTime();
> +        if (windowRenegotiationsStart + RenegotiationsWindow < currentTime) {
> +            windowRenegotiationsStart = currentTime;
> +            windowRenegotiations = 1;
> +        } else {

... each attempt, the start timer is moved forward to the current
timestamp. So you are not counting 5 per 10sec, you are rejecing is
>10sec between attempts (which is okay I think, but still not what is
intended).

I think the FadingCounter class should be used here instead.


Amos