Re: [RFC] drop Request-Range header support

On Thu, Feb 27, 2014 at 8:58 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On 27/02/2014 6:21 p.m., Alex Rousskov wrote:
>> On 02/26/2014 09:51 PM, Amos Jeffries wrote:
>>> While checking the code against HTTPbis Range draft I think we have a
>>> possible smuggling avenue being enabled by Squid in the form of the
>>> Request-Range header.
>>>
>>> What I can see there in Squid is parsing Request-Range into the same
>>> memory structures as Range. Each time teh header is encountered erasign
>>> prior headers data. Then forwarding the results upstream under the name
>>> Range.
>>>
>>> Only one range header is specified. Multiple is udnefined behaviour. So
>>> the following request may be interpreted differently by other
>>> participants seeing:
>>>
>>> GET / HTTP/1.1
>>> Range: bytes=0-1000
>>> Request-Range: bytes=257-1000
>>>
>>>
>>> * Squid will effectively strip the Range: header and relay the
>>> Request-Range value in Range: upstream.
>>>
>>> * agents closer to the client may be interpreting that as only Range:
>>> header plus unknown header.
>>>
>>> Content-Range on the reply should be sorting out what is actually
>>> delivered so no problems are visible. But any protection based on the
>>> request could be confused or tricked into not processing the reply if it
>>> was only checking based on the RFC standard Range header.
>>>
>>>
>>> This type of crafted request will also have the same behaviour in Squid:
>>>
>>> GET / HTTP/1.1
>>> Range: bytes=0-1000
>>> Range: bytes=257-1000
>>>
>>>
>>> The proposal(s):
>>>
>>> A) treating the Request-Range header as obsolete and erasing it without
>>> processing?
>>>
>>> The only mentions I can find online are CVE-2011-3192 about a
>>> Flash+Apache vulnerability meaning the header had to be blocked from
>>> sending by Flash and XHR mechanisms plus documentation relating. And
>>> that its a legacy header from the days of MSIE 3.0 and Nescape.
>>> So it looks like a candidate for dropping some more code.
>>>
>>>
>>> B) treating multiple Range headers as an error and dropping them all.
>>
>> Unless they are the same perhaps? I am worried about existing HTTP
>> clients (unknown to me) sending double Ranges (for no good reason).
>> There is no compelling reason to drop all Ranges if they are identical,
>> I think. We can leave and honor one of them.
>>
>>
>>> This would result in Request-Range being treasted somewhat like
>>> Proxy-Connection has been for some time. Either upgraded when its alone,
>>> or dropped if there is a collision.
>>>
>>>
>>> FWIW: the specification says any proxy MAY drop Range at its choice. So
>>> we are permitted to erase this traffic brokenness in either of the above
>>> ways.
>>
>> Do you propose to hard-code actions (a) and (b)? Will some existing or
>> new option control this behavior?
>
>
> (A) can be hard-coded in the form of either:
> i) dropping it from the header registry + using delByName() on every
> request, OR
> ii) dropping explicit mentions of it outside the header registry,
> setting its field type to generic String, and adding it to the
> connection-headers mask.
>
> (B) the putRange() can detect multiples and set the range to a special
> (empty header?) value that gets stripped at the end of parsing. In order
> not to reject even counts of Range headers while allowing odd-counts.
>
>
>
> I kind of favour (A), but we may end up having to do both eventually
> anyway to enforce correct Range compliance.

(A) looks better to me.
(B) as expressed doesn't look right to me. I don't know what the
standard says; in my opinion it's not worth the (code and
computational) effort needed to try and reconcile broken Range
requests; the safest response is to drop them all in case of
duplicates, and I'd suggest we do just that.

-- 
    Kinkie