At 06:43 AM 7/14/2005, Jeff Trawick wrote:
>On 7/13/05, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:
>
>> How can I fix thee? let me count the ways...
>>
>> * pass a chunked body always (no-body requests don't go chunked).
>
>We tried to send C-L whenever practical because it is common for a
>origin server not to be able to handle chunked. It looks like the
>default configuration will be broken for some folks with input
>filters, such as origin server = Apache mod_cgi, or origin server =
>IIS (somebody else's comments).
Good points... however...
>So previously we optimized for
>fool-proof operation instead of efficiency, and the administrator
>could tune for efficiency if they could determine that there was no
>issue with the back-end server handling chunked.
Well, not fool proof, here were the faults with the old logic;
* "c-l: 0" can still have a request body, the RB_SPOOL_CL is
now used in all but the 'r->input_filt == r->proto_input_filt'
case, for paranoia that the given cl can mismatch the input
length read from the filter chain. Adding a test against the
value reinforces our faith that the request isn't split.
* I'd agree to drop the RB_STREAM_CHUNKED from the old_cl_val test
path, and always fall back on RB_SPOOL_CL instead. Costly but,
as you point out, not designed for efficiency. And manditory
to change for httpd-2.0, I agree.
* In the old_te_val case, I'd also agree to probe the server first
with an OPTIONS to determine if it is an HTTP/1.1 server before
we choose RB_SPOOL_CL. Connection persistence makes this more
cost-effective if we cache the result for the specific backend,
at least for the lifetime of this request. But for the normal
scenario, the origin server will return a 411 Length Required,
and the client will retry falling into the old_cl_val case.
* RFC 2616 says
"All HTTP/1.1 applications MUST be able to receive and decode the
"chunked" transfer-coding, and MUST ignore chunk-extension extensions
they do not understand."
I read this as "an HTTP/1.1 server must accept "chunked" or quit
reporting it complies with the HTTP/1.1 specification".
However, I agree this might be insufficient. We simply will have
to be prepared for the request to be rejected. I'm thinking that
an initial probe of the origin server would work, especially if we
can then keep a cache of backend server capabilities.
We might even send the headers and an Expect: 100-continue, wait
for our continue, and send the chunked body. If that fails, or
sends nothing after 2 sec or whatever, send as C-L request instead.
We still have the headers and haven't consumed (burned) the client
request body.
That thought is something to contemplate for 2.1 not 2.0.
>> * validate that the C-L counted body length doesn't change.
>
>It looks like we are counting bytes after they go through input
>filters here, and then comparing that byte count with the specified
>C-L header field. That doesn't have to match since filters can change
>the size. (Admittedly I'm probably misunderstanding something.)
Yes! We only hit that STREAM_CL choice if there are no body filters,
only protocol filters. We just reported to the origin server that
the C-L was X - and now we are sending the server Y. This is the
very definition of response splitting, this time at the hands of
an httpd filter.
We will simply kill the request if this happens. That's why I'd
killed the 'please prefer STREAM_CL' flag and moved the C-L "0"
case over to RB_SPOOL_CL. The flag was beyond the administrators
knowledge (and the developer who recommends it can't be certain
the admin hasn't added another filter), while the C-L "0" case
is just as efficient passing through the spool_reqbody_cl path.
>> * follow RFC 2616 for C-L / T-E in the request body C-L / T-E
>> election logic.
>
>Can you be more specific about what exactly had to be done for RFC
>2616 conformance?
If T-E is given, ignore C-L. The old code didn't do so for the
repeated request. This is exactly what Watchfire observed.
Watchfire also observed that all body headers would disappear
entirely under certain scenarios, leaving the body to appear to
the origin server as a new request.
>> * do not forward HTTP/1.0 requests as HTTP/1.1, unless the admin
>> configures force-proxy-request-1.1
>
>What is the harm? The potential value is that the administrator can
>tell us to use chunked iin the case where there is an input filter and
>the origin server can handle chunked.
Because header results may change, rendering headers instead that
the client does not understand.
>> + *) SECURITY: CAN-2005-2088
>> + proxy: Correctly handle the Transfer-Encoding and Content-Length
>> + headers, discard the request Content-Length whenever T-E: chunked
>> + is used, always passing one of either C-L or T-E: chunked whenever
>> + the request includes a request body, and no longer upgrade HTTP/1.0
>> + requests to the origin server as HTTP/1.1. Resolves an entire class
>> + of proxy HTTP Request Splitting/Spoofing attacks. [William Rowe]
>
>I'm so confused while trying to draw the line between
>alternate RFC-compliant philosophy
>fixes for actual RFC violations
>fixes for security issues
There's no distinction. The security issue *is* RFC non-compliance
in handling C-L and T-E headers. The RB_SPOOL/STREAM_CL/CHUNKED
choose-one logic violated that principal. The only reason you no
longer see it is the change to protocol.c - back out that patch
(which doesn't affect internal actions by modules or filters) and
you can observe exactly what I mean. Additional errors in not
sending empty bodies, etc, compounded the issue.
>I think CHANGES should be crystal clear on what change has a security
>implication.
All of it, except for the preference to RB_STREAM_CHUNKED when,
perhaps, we could be more sub-optimal, falling back on RB_SPOOL_CL.
Many RB_STREAM_CL choices, before, were equally dangerous, and that
C-L == length_read test in the stream_reqbody_chunked() was meant
to exclude future abuse.
So how is this;
* proxy: Correctly handle the Transfer-Encoding and Content-Length
request body headers. Discards the request Content-Length whenever
Transfer-Encoding: chunked is used; ensures that any request which
includes a body (even zero length) passes on that request body to
the origin server with either the T-E: chunked -or- C-L: header as
appropriate. Resolves an entire class of proxy HTTP Request
Splitting / Spoofing attacks. [William Rowe]
* No longer upgrade HTTP/1.0 requests to the origin server as
HTTP/1.1 unless the force-proxy-request-1.1 envvar is set.
[William Rowe]
Bill
