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
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
