Re: [PATCH] Compliance: Allow valid requests with benign CRs

Hi all,

I found this patch in a bug report for debian (http://bugs.debian.org/669148) and wondered why it was not merged. Is there any reason I should not add it to current package?

Thanks,

L

> From: Alex Rousskov <rousskov_at_measurement-factory.com>
> Date: Thu, 29 Jul 2010 14:13:40 -0600
> Hello,
>
> The attached patch improves request smuggling attack detection and
> makes Squid more tolerable to valid HTTP headers with benign CR+
> sequences. This change may be useful in general (I believe I have seen
> benign requests rejected by Squid exposed to a large variety of client
> software) and also removes several Co-Advisor-detected HTTP/1.1 test
> case failures and violations.
>
> Lower-level details are quoted below. Please review.
>
> Thank you,
>
> Alex.
>
> Removed "double CR" check from parseHttpRequest() for several reasons:
>
> 1) The check was most likely introduced as a short-term defense
> against "HTTP request smuggling" attacks identified in an
> influential 2004 paper. The paper documented certain
> vulnerabilities related to requests with "double CR" sequences, and
> Squid was quickly hacked to prohibit such requests as
> malformed. However, a more careful reading of the paper indicates
> that only LF CR CR LF (a.k.a. "CR header") sequences were
> identified as dangerous (note the leading LF). The quick fix was
> too aggressive and blocked _all_ requests with CR CR LF sequences,
> including benign requests.
>
> 2) The check duplicated a HttpHeader::parse() check.
>
> 3) The check was slower than the code it duplicated.
>
> Improved "double CR" handling in HttpHeader::parse() to detect
> potentially dangerous "empty headers", that is header fields that
> contain nothing but CR character(s). Requests with such headers are
> rejected as malformed. We used to reject similar requests (and more)
> in parseHttpRequest() as described above.
>
> After the change, potentially malicious requests with CR+ headers are
> still denied. Other, benign headers ending with CRs are now allowed.
>
> If the HTTP header parser is not "relaxed", benign and valid requests
> with extra CR characters are blocked as before.
>
> • text/x-diff attachment: allow-benign-CRs-t3.patch

--
Luigi Gangitano -- <luigi_at_debian.org> -- <gangitano_at_lugroma3.org>
GPG: 1024D/924C0C26: 12F8 9C03 89D3 DB4A 9972  C24A F19B A618 924C 0C26
GPG: 4096R/2BA97CED: 8D48 5A35 FF1E 6EB7 90E5  0F6D 0284 F20C 2BA9 7CED