URI lossage with ProxyPass

[Francois-Rene Rideau]

Dear mod_proxy developers,

I have experienced quite some trouble due to design bugs in ProxyPass,
and have proposed a patch for apache 1.3.
The very same bugs are present in apache 2.0, and a similar fix could be used.
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=29554

Can you tell me if you'll fix the official mod_proxy,
either using my patch or otherwise?


The bug symptoms are that
(1) when a request to a ProxyPass host contains %3A, the %3A is expanded
to a colon, which yields an incorrect HTTP URL that confuses the remote host.
(2) when a request to a ProxyPass host contains %2F, apache rejects the
request with a 404 without even contacting the remote host.

The bug causes are that
(1) function modules/proxy/mod_proxy.c:proxy_fixup() makes a misguided attempt
at URI canonicalization. It should definitely not try to
when using PROXY_PASS, and probably not in STD_PROXY mode either.
Since I don't understand all the ins and outs, my patch only adds a bypass
in the case of PROXY_PASS, but I believe the whole function should be
scrapped altogether (whoever checks in the patch should ponder that).
(2) r->proxyreq=PROXY_PASS is declared too late, only in
modules/proxy/mod_proxy.c:proxy_trans(), so that
main/http_request.c:process_request_internal() already messed up
with the URL, not realizing there is a proxy request going on.
Consequently, the ProxyPass alias detection MUST happen not in
modules/proxy/mod_proxy.c:proxy_trans() but in
modules/proxy/mod_proxy.c:proxy_detect().
This may or may not interfere with funky rewrites that some people
may want to do before or after a ProxyPass is used. Someone who understands
such issues should step in and tell. Maybe my change introduces some
subtle incompatibilities in *actually deployed* setups, but I would bet not,
and some mechanism could be devised to restore proper behaviour
for those who would need such a feature.

I hope my patch doesn't break any expected behaviour, but I can't be sure.
What I'm certain of is that ProxyPass is quite broken without my patch.
Please consider merging this patch into apache, and tell me when it's done.

Cheers,

[ François-René ÐVB Rideau | Reflection&Cybernethics | http://fare.tunes.org ]
[ TUNES project for a Free Reflective Computing System | http://tunes.org ]
The last good thing written in C was Franz Schubert's Symphony number 9.
-- Erwin Dieterich <***@cvt12.verfahrenstechnik.uni-stuttgart.de>

[Graham Leggett]

Post by Francois-Rene Rideau
The bug symptoms are that
(1) when a request to a ProxyPass host contains %3A, the %3A is expanded
to a colon, which yields an incorrect HTTP URL that confuses the remote host.
(2) when a request to a ProxyPass host contains %2F, apache rejects the
request with a 404 without even contacting the remote host.

In theory the proxy should pass the URL as is to the backend server,
without any canonicalisation at all.

Can anyone see any reason why a proxy request should have the URL changed?

Regards,
Graham
--

[Nick Kew]

Post by Graham Leggett

Post by Francois-Rene Rideau
The bug symptoms are that
(1) when a request to a ProxyPass host contains %3A, the %3A is expande=

d

Post by Graham Leggett

Post by Francois-Rene Rideau
to a colon, which yields an incorrect HTTP URL that confuses the remot=

e host.

Post by Graham Leggett

Post by Francois-Rene Rideau
(2) when a request to a ProxyPass host contains %2F, apache rejects the
request with a 404 without even contacting the remote host.

In theory the proxy should pass the URL as is to the backend server,
without any canonicalisation at all.
Can anyone see any reason why a proxy request should have the URL changed=

?

For a forward proxy, that's right. For a reverse proxy, no.
Of course ProxyPass and family modify URLs.

Far=E9: do you have a testcase that demonstrates this? I'll be committing
my fix to bug 10722 soon, and if I'm satisfied with your patch I could
probably roll it in together. A testcase will help there:-)

--=20
Nick Kew

[Francois-Rene Rideau]

Post by Nick Kew

Post by Graham Leggett
Can anyone see any reason why a proxy request should have the URL changed?

For a forward proxy, that's right. For a reverse proxy, no.
Of course ProxyPass and family modify URLs.

OK, I mean, "changes that require further canonicalization".
And why does that further canonicalization require to de-encode anything?
If de-encoding there must be because of intermediate modifications,
shouldn't there be a preliminary encoding in an early pass to preserve
the encoded parts of the original URI?

Post by Nick Kew
Faré: do you have a testcase that demonstrates this?

Try any URI with a %3A, and any URL with a %2F inside. 100% reproducible.
i.e. try to access through ProxyPass the following URI:
http://cliki.tunes.org/CP%2FM
http://cliki.tunes.org/Word%20Processors%3A%20Stupid%20and%20Inefficient
For instance add a
ProxyPass /cto/ http://cliki.tunes.org/
to one of your servers, and see (e.g. with tcpdump
or by logging proxy behaviour) what happens.

The real cliki.tunes.org actually sits behind a (now fixed) apache proxypass
that takes care of handling HTTP problems with bad clients and bad connections.
Without the fix, these two pages (created before the use of apache)
were unaccessible, each in a different way.

Post by Nick Kew
I'll be committing
my fix to bug 10722 soon, and if I'm satisfied with your patch I could
probably roll it in together. A testcase will help there:-)

nagoya.apache.org is unaccessible right now,
so I can't see what 10722 is about.

[ François-René ÐVB Rideau | Reflection&Cybernethics | http://fare.tunes.org ]
[ TUNES project for a Free Reflective Computing System | http://tunes.org ]
If it isn't source, it isn't software.
-- NASA

[Graham Leggett]

Post by Nick Kew

Post by Graham Leggett
Can anyone see any reason why a proxy request should have the URL changed?

For a forward proxy, that's right. For a reverse proxy, no.
Of course ProxyPass and family modify URLs.

I am not sure I understand why a forward or a reverse proxy should
modify the URL that is sends to the downstream server. I see no
difference between the two - I think the unmodified URL should be passed
as is.

As for mod_proxy's internal handling of the URL, there is a need to
decode the URL, as requests for /%41%42%43 and /ABC must both match the
ProxyPass defined for /ABC which they only will do if decoded first.

Regards,
Graham
--

[Francois-Rene Rideau]

Post by Graham Leggett
As for mod_proxy's internal handling of the URL, there is a need to
decode the URL, as requests for /%41%42%43 and /ABC must both match the
ProxyPass defined for /ABC which they only will do if decoded first.

Well, then, functions to decode the URL should be done in a buffer
used merely for checking, that does NOT change the URL actually used
for the request. It should NOT die when %2F is found in the URI,
and it should NOT mishandle %3A in the URI.

In any case, current code is broken.
If you have a better fix, please step in.

[ François-René ÐVB Rideau | Reflection&Cybernethics | http://fare.tunes.org ]
[ TUNES project for a Free Reflective Computing System | http://tunes.org ]
It is proof of a base and low mind for one to wish to think with the masses or
majority, merely because the majority is the majority. Truth does not change
because it is, or is not, believed by a majority of the people.
-- Giordano Bruno (1548-burned at the stake, 1600)

[Nick Kew]

Post by Francois-Rene Rideau
In any case, current code is broken.
If you have a better fix, please step in.

OK, I've run your testcase, and can confirm it's partly broken in HEAD.
The config option to support %2F works, but the decoded colon causes
an error.

I note your patch is for 1.3 but you claim it's equivalent in 2.0.

[Nick Kew]

Post by Francois-Rene Rideau
I have experienced quite some trouble due to design bugs in ProxyPass,
and have proposed a patch for apache 1.3.
The very same bugs are present in apache 2.0, and a similar fix could be =

used.

Post by Francois-Rene Rideau
=09http://nagoya.apache.org/bugzilla/show_bug.cgi?id=3D29554

I've reviewed this in the context of httpd-2.1, and it looks good to me
with essentially the same patch. It works on your testcase, and I'm
99% satisfied that it doesn't break anything. Ready to commit if we
can answer the remaining question: should proxy_fixup be removed

Post by Francois-Rene Rideau
Can you tell me if you'll fix the official mod_proxy,
either using my patch or otherwise?
The bug symptoms are that
(1) when a request to a ProxyPass host contains %3A, the %3A is expanded
to a colon, which yields an incorrect HTTP URL that confuses the remote =

host.

Post by Francois-Rene Rideau
(2) when a request to a ProxyPass host contains %2F, apache rejects the
request with a 404 without even contacting the remote host.
The bug causes are that
(1) function modules/proxy/mod_proxy.c:proxy_fixup() makes a misguided at=

tempt

Post by Francois-Rene Rideau
at URI canonicalization. It should definitely not try to
when using PROXY_PASS, and probably not in STD_PROXY mode either.
Since I don't understand all the ins and outs, my patch only adds a bypa=

ss

Post by Francois-Rene Rideau
in the case of PROXY_PASS, but I believe the whole function should be
scrapped altogether (whoever checks in the patch should ponder that).

Graham Leggett's reply seems to support that, and having figured out
what you are talking about, I agree.

Can anyone see why proxy_fixup should not be removed altogether?

Post by Francois-Rene Rideau
(2) r->proxyreq=3DPROXY_PASS is declared too late, only in
modules/proxy/mod_proxy.c:proxy_trans(), so that
main/http_request.c:process_request_internal() already messed up
with the URL, not realizing there is a proxy request going on.
Consequently, the ProxyPass alias detection MUST happen not in
modules/proxy/mod_proxy.c:proxy_trans() but in
modules/proxy/mod_proxy.c:proxy_detect().
This may or may not interfere with funky rewrites that some people
may want to do before or after a ProxyPass is used. Someone who understa=

nds

Post by Francois-Rene Rideau
such issues should step in and tell. Maybe my change introduces some
subtle incompatibilities in *actually deployed* setups, but I would bet =

not,

Post by Francois-Rene Rideau
and some mechanism could be devised to restore proper behaviour
for those who would need such a feature.
I hope my patch doesn't break any expected behaviour, but I can't be sure=

=2E

Post by Francois-Rene Rideau
What I'm certain of is that ProxyPass is quite broken without my patch.
Please consider merging this patch into apache, and tell me when it's don=

e.

Post by Francois-Rene Rideau
Cheers,
[ Fran=E7ois-Ren=E9 =D0VB Rideau | Reflection&Cybernethics | http://fare.=

tunes.org ]

Post by Francois-Rene Rideau
[ TUNES project for a Free Reflective Computing System | http://tunes.o=

rg ]

Post by Francois-Rene Rideau
The last good thing written in C was Franz Schubert's Symphony number 9.

e>
--=20
Nick Kew

[Dirk-Willem van Gulik]

Post by Nick Kew
I've reviewed this in the context of httpd-2.1, and it looks good to me
with essentially the same patch. It works on your testcase, and I'm
99% satisfied that it doesn't break anything. Ready to commit if we

+1 here too - we had something ourselfs which did this in a dirty/specific
ase; and this patch makes that unessesary and is more general.

Dw.

[Graham Leggett]

Post by Nick Kew
Can anyone see why proxy_fixup should not be removed altogether?

Proxy fixup seems to do the job of making sure the URL "/%41%42%43"
matches "ProxyPass /ABC http://xxx/ABC", so I don't think it should be
removed altogether.

The URL that is sent to the backend server however has no business being
touched at all en route - a copy of the untouched URL should be kept
alongside the canonicalised URL, and the untouched URL should be sent to
the backend server just as the client sent it.

Regards,
Graham
--

[Nick Kew]

Post by Graham Leggett

Post by Nick Kew
Can anyone see why proxy_fixup should not be removed altogether?

Proxy fixup seems to do the job of making sure the URL "/%41%42%43"
matches "ProxyPass /ABC http://xxx/ABC", so I don't think it should be
removed altogether.

I don't think that's right. Both proxy_detect and proxy_trans happen
before proxy_fixup, and the comment in proxy_fixup refers to its
relationship with mod_rewrite.

The patched apache fails that test, but simply reinstating proxy_fixup
makes no difference to that. Now I'm confused.

I think you're right in your other post: separate patches for separate
bugs. And not necessarily at 4 a.m. ....

But having come this far, I want to see both fixed:-) And a trawl of
bugzilla tells me that the URI Lossage is bug #15207 and probably others,
while bug #16812 is a trivial corollary to the cookie patch.

[Nick Kew]

Post by Graham Leggett

Post by Nick Kew
Can anyone see why proxy_fixup should not be removed altogether?

Proxy fixup seems to do the job of making sure the URL "/%41%42%43"
matches "ProxyPass /ABC http://xxx/ABC", so I don't think it should be
removed altogether.

OK, the reason for that is that the patch moved ProxyPass-ing from
proxy_trans to proxy_detect. The latter happens before canonicalisation,
which is both why the patch works and why it breaks the above.

A fix is for alias_match() to recognise %xx sequences. I've now
implemented it, but also separated out the URI-trouble stuff with
#ifdef FIX_15207
on the grounds that it's still subject to debate.

That still leaves us a proxy_fixup with no purpose I can see.
Perhaps someone who uses it with mod_rewrite can say if it
does anything for you?

[Dirk-Willem van Gulik]

Did anyone in mod_proxy land look at this ? I've got more or less the same
questions - but am also a bit unsure what collateral damage the soltuion
outlined below would do (though it 'works for me gov').

Dw

Post by Francois-Rene Rideau
Dear mod_proxy developers,
I have experienced quite some trouble due to design bugs in ProxyPass,
and have proposed a patch for apache 1.3.
The very same bugs are present in apache 2.0, and a similar fix could be used.
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=29554
Can you tell me if you'll fix the official mod_proxy,
either using my patch or otherwise?
The bug symptoms are that
(1) when a request to a ProxyPass host contains %3A, the %3A is expanded
to a colon, which yields an incorrect HTTP URL that confuses the remote host.
(2) when a request to a ProxyPass host contains %2F, apache rejects the
request with a 404 without even contacting the remote host.
The bug causes are that
(1) function modules/proxy/mod_proxy.c:proxy_fixup() makes a misguided attempt
at URI canonicalization. It should definitely not try to
when using PROXY_PASS, and probably not in STD_PROXY mode either.
Since I don't understand all the ins and outs, my patch only adds a bypass
in the case of PROXY_PASS, but I believe the whole function should be
scrapped altogether (whoever checks in the patch should ponder that).
(2) r->proxyreq=PROXY_PASS is declared too late, only in
modules/proxy/mod_proxy.c:proxy_trans(), so that
main/http_request.c:process_request_internal() already messed up
with the URL, not realizing there is a proxy request going on.
Consequently, the ProxyPass alias detection MUST happen not in
modules/proxy/mod_proxy.c:proxy_trans() but in
modules/proxy/mod_proxy.c:proxy_detect().
This may or may not interfere with funky rewrites that some people
may want to do before or after a ProxyPass is used. Someone who understands
such issues should step in and tell. Maybe my change introduces some
subtle incompatibilities in *actually deployed* setups, but I would bet not,
and some mechanism could be devised to restore proper behaviour
for those who would need such a feature.
I hope my patch doesn't break any expected behaviour, but I can't be sure.
What I'm certain of is that ProxyPass is quite broken without my patch.
Please consider merging this patch into apache, and tell me when it's done.
Cheers,
[ François-René ÐVB Rideau | Reflection&Cybernethics | http://fare.tunes.org ]
[ TUNES project for a Free Reflective Computing System | http://tunes.org ]
The last good thing written in C was Franz Schubert's Symphony number 9.

[Nick Kew]

Post by Dirk-Willem van Gulik
Did anyone in mod_proxy land look at this ? I've got more or less the same
questions - but am also a bit unsure what collateral damage the soltuion
outlined below would do (though it 'works for me gov').

I've updated this patch to compile conditionally with an #ifdef for 2.1.
Planning to commit tomorrow unless something new crops up. The #ifdef
should make it easy to test whether it breaks anything, and can be
chopped if all is well.