You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2005/07/14 07:21:32 UTC

[patch 2.0] http body request/response/trace conformance

Attached is the rollup of everything that we have collected
to address c-l/t-e conformance in the httpd-2.1 trunk, which
should apply clean to httpd-2.0 branch.  I'll commit in 24 hrs
by lazy consensus, but would rather see a few +1's first.

Jeff's patch (without the protocol.c patch) demonstrated to me
how broken 2.1 really was.  I've refactored that code entirely.
After this refactoring (and still without the protocol.c patch)
the attached chunked.req works correctly against an array of 
Apache versions.  I totally agree with Jeff's plan, the only
way to address this is to backport the corrected 2.1 proxy
request handler.

The 'TraceEnable extended' option has really proved invaluable
for this testing; I would encourage anyone investigating these
issues to check it out.

Bill

p.s. I've found this mini-configlet very useful for my test array;

ProxyPass /13/ http://localhost:8013/
ProxyPassReverse /13/ http://localhost:8013/
ProxyPass /20/ http://localhost:8020/
ProxyPassReverse /20/ http://localhost:8020/
ProxyPass /21/ http://localhost:8021/
ProxyPassReverse /21/ http://localhost:8021/

ProxyPass /99/ http://localhost:8099/
ProxyPassReverse /99/ http://localhost:8099/

TraceEnable extended

Re: [patch 2.0] http body request/response/trace conformance

Posted by Jim Jagielski <ji...@jaguNET.com>.
+1

Re: [patch 2.0] http body request/response/trace conformance

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 03:03 PM 7/14/2005, William A. Rowe, Jr. wrote:
>To simplify - Jeff Joe and I reviewed two of the patches, and they
>are committed.  Two patches are available for comment;

changelog;

  *) Added TraceEnable [on|off|extended] per-server directive to alter
     the behavior of the TRACE method.  This addresses a flaw in proxy
     conformance to RFC 2616 - previously the proxy server would accept
     a TRACE request body although the RFC prohibited it.  The default
     remains 'TraceEnable on'.  [William Rowe]


>http://people.apache.org/~wrowe/httpd-2.0-trace.patch

and changelog;


  *) 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.  Resolves an entire class of
     proxy HTTP Request Splitting/Spoofing attacks.  [William Rowe]


The newest flavor based on my most recent commits from Roy and Jeff's
feedback is available at;

http://people.apache.org/~wrowe/httpd-2.0-proxy-request-2.patch

and 2.0 STATUS is updated accordingly.  Votes/Comments please?

>Although proxy-request.patch will evolve as this discussion
>continues; Jeff caused me to look, again, at the code and
>recognize another edge case already committed to trunk 
>(and also in the patch.)  proxy-request.patch will ultimately
>mirror what we agree to on trunk.
>
>And FYI, revert r219061 (below) from 2.1 or 2.0 to see the
>continued misbehavior of proxy without the proxy-request.patch.
>
>Bill
>
>--- httpd/httpd/branches/2.0.x/server/protocol.c (original)
>+++ httpd/httpd/branches/2.0.x/server/protocol.c Thu Jul 14 09:51:55 2005
>@@ -885,6 +885,15 @@
>             apr_brigade_destroy(tmp_bb);
>             return r;
>         }
>+
>+        if (apr_table_get(r->headers_in, "Transfer-Encoding")
>+            && apr_table_get(r->headers_in, "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_in, "Content-Length");
>+        }
>     }
>     else {
>         if (r->header_only) {



Re: [patch 2.0] http body request/response/trace conformance

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 06:14 AM 7/15/2005, Roy T. Fielding wrote:
>-1  This is a violation of HTTP:
>
>+    if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")
>+         || ((r->proto_num < HTTP_VERSION(1,1))
>+             && !apr_table_get(r->subprocess_env, "force-proxy-request-1.1"))) {
>
>In all cases except when specifically configured otherwise,
>the proxy must send HTTP/1.1 if it supports responses in HTTP/1.1.

Thanks for the clarification, Roy!

Bill



Re: [patch 2.0] http body request/response/trace conformance

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
-1  This is a violation of HTTP:

+    if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")
+         || ((r->proto_num < HTTP_VERSION(1,1))
+             && !apr_table_get(r->subprocess_env, 
"force-proxy-request-1.1"))) {

In all cases except when specifically configured otherwise,
the proxy must send HTTP/1.1 if it supports responses in HTTP/1.1.

....Roy


Re: [patch 2.0] http body request/response/trace conformance

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
To simplify - Jeff Joe and I reviewed two of the patches, and they
are committed.  Two patches are available for comment;

http://people.apache.org/~wrowe/httpd-2.0-trace.patch
http://people.apache.org/~wrowe/httpd-2.0-proxy-request.patch

Although proxy-request.patch will evolve as this discussion
continues; Jeff caused me to look, again, at the code and
recognize another edge case already committed to trunk 
(and also in the patch.)  proxy-request.patch will ultimately
mirror what we agree to on trunk.

And FYI, revert r219061 (below) from 2.1 or 2.0 to see the
continued misbehavior of proxy without the proxy-request.patch.

Bill

--- httpd/httpd/branches/2.0.x/server/protocol.c (original)
+++ httpd/httpd/branches/2.0.x/server/protocol.c Thu Jul 14 09:51:55 2005
@@ -885,6 +885,15 @@
             apr_brigade_destroy(tmp_bb);
             return r;
         }
+
+        if (apr_table_get(r->headers_in, "Transfer-Encoding")
+            && apr_table_get(r->headers_in, "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_in, "Content-Length");
+        }
     }
     else {
         if (r->header_only) {



Re: [patch 2.0] http body request/response/trace conformance

Posted by Jeff Trawick <tr...@gmail.com>.
On 7/14/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> Attached is the rollup of everything that we have collected
> to address c-l/t-e conformance in the httpd-2.1 trunk, which
> should apply clean to httpd-2.0 branch.  I'll commit in 24 hrs
> by lazy consensus, but would rather see a few +1's first.

What's the deal with "lazy concensus" here?  If you don't have the
reviews, you don't commit.

> ...  I totally agree with Jeff's plan, the only
> way to address this is to backport the corrected 2.1 proxy
> request handler.

that design point is not something I can confirm or deny at this
point; all I remember noting was that I couldn't test something
successfully with 2.1 proxy