Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (asf.osuosl.org: local policy)
Errors-To: <wrowe@rowe-clan.net>
Message-Id: <6.2.1.2.2.20050622220324.0cc106c0@pop3.rowe-clan.net>
Date: Wed, 22 Jun 2005 22:51:06 -0500
To: dev@httpd.apache.org
From: "William A. Rowe, Jr." <wrowe@rowe-clan.net>
Subject: Re: 2.1.5 available for testing
In-Reply-To: <cc67648e0506221640630ff644@mail.gmail.com>
References: <6.2.1.2.2.20050622145657.06df4eb0@pop3.rowe-clan.net>
 <20050622211223.GA4142@redhat.com>
 <42B9D66F.8020704@force-elite.com>
 <cc67648e0506221640630ff644@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"

++1 To Joe's comments.

Jeff's fix is technically right, but scares the nibbles out
of me.  If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not 
trust the origin server at all.

For origin servers (as opposed to clients) make this choice
between ignore C-L, or fail, configurable?

My observation is that there are far more varied clients in
the world than servers, each with unique faults.  But the
RFC 2616 is clear...

   Messages MUST NOT include both a Content-Length header field and a
   non-identity transfer-coding. If the message does include a non-
   identity transfer-coding, the Content-Length MUST be ignored.

   When a Content-Length is given in a message where a message-body is
   allowed, its field value MUST exactly match the number of OCTETs in
   the message-body. HTTP/1.1 user agents MUST notify the user when an
   invalid length is received and detected.

...and server authors can be expected to be less buggy than clients.
"Permissive in what we accept, strict in what we send" implies some
strictness in what we trust to pass on to the client.

So +.5 to Jeff's patch, and let's discuss if the proxy response should 
then be trusted at all with T-E and C-L, in httpd-2.x where we support 
keepalives.

[FYI +1 in httpd-1.3, that proxy does not use keepalives.]

Bill


Bill

At 06:40 PM 6/22/2005, Jeff Trawick wrote:
>On 6/22/05, Paul Querna <chip@force-elite.com> wrote:
>> Joe Orton wrote:
>> 
>> >On Wed, Jun 22, 2005 at 03:02:50PM -0500, William Rowe wrote:
>> >
>> >
>> >>Prior to either patch we totally mishandled such requests.  So the
>> >>only question which remains is; which behavior do we prefer?
>> >>As the RFC states this is not acceptable, my gut says reject ANY
>> >>request with both C-L and T-E of non-identity.
>> >>
>> >>
>> >
>> >I don't see any reason to reject anything, 2616 dictates precisely how
>> >to handle requests which are malformed in this way.  I do think the
>> >check against "identity" is actually redundant, though; not least
>> >because the 2616 errata remove all references to the word.
>> >
>> >I think correct behaviour is to just follow the letter of Section 4.4,
>> >point 3, as below:
>> >
>> >                               If a message is received with both a
>> >     Transfer-Encoding header field and a Content-Length header field,
>> >     the latter MUST be ignored.
>> >
>> >(and it's also going to be better to check for T-E before C-L since
>> >99.99% of requests received are not going to have a T-E header so it'll
>> >fall through slightly quicker)
>> >
>> >
>> >
>> +1 to the posted patch.
>
>+1 here as well
>
>What about proxy reading from origin server?
>
>Origin server sends this to Apache acting as proxy:
>HTTP/1.1 200 OK\r\n
>Content-Length: 99\r\n
>Transfer-Encoding: chunked\r\n
>\r\n
>0A\r\n
>aaaaaaaaaa\r\n
>00\r\n
>\r\n
>
>Client receives this:
>
>HTTP/1.1 200 OK
>Date: Wed, 22 Jun 2005 23:12:31 GMT
>Content-Length: 99
>Content-Type: text/plain; charset=ISO-8859-1
>
>aaaaaaaaaa(END)
>
>Add this patch:
>
>Index: modules/proxy/mod_proxy_http.c
>===================================================================
>--- modules/proxy/mod_proxy_http.c      (revision 191655)
>+++ modules/proxy/mod_proxy_http.c      (working copy)
>@@ -1128,7 +1128,17 @@
>                                                        r->headers_out,
>                                                        save_table);
>                 }
>-
>+
>+                /* can't have both Content-Length and Transfer-Encoding */
>+                if (apr_table_get(r->headers_out, "Transfer-Encoding")
>+                    && apr_table_get(r->headers_out, "Content-Length")) {
>+                    /* 2616 section 4.4, point 3: "if both Transfer-Encoding
>+                     * and Content-Length are received, the latter MUST be
>+                     * ignored"; so unset it here to prevent any confusion
>+                     * later. */
>+                    apr_table_unset(r->headers_out, "Content-Length");
>+                }
>+
>                 /* strip connection listed hop-by-hop headers from response */
>                 backend->close +=
>ap_proxy_liststr(apr_table_get(r->headers_out,
>                                                                  "Connection"),
>
>Client now gets this:
>
>HTTP/1.1 200 OK
>Date: Wed, 22 Jun 2005 23:22:19 GMT
>Transfer-Encoding: chunked
>Content-Type: text/plain; charset=ISO-8859-1
>
>a
>aaaaaaaaaa
>0