Mailing List Archive

On 6/9/20 12:05 PM, jean-frederic clere wrote:
> Hi,
>
> Basically it adds servletnormalizecheck to mod_proxy for ProxyPass/ProxyPassMatch and mod_rewrite when using P
> I have tested the following uses:
> #ProxyPass  /docs ajp://localhost:8009/docs secret=%A1b2!@ servletnormalizecheck
>
> #ProxyPassMatch  "^/docs(.*)$" "ajp://localhost:8009/docs$1" secret=%A1b2!@ servletnormalizecheck
>
> #RewriteEngine On
> #RewriteRule "^/docs(.*)$" "ajp://localhost:8009/docs$1" [P,SNC]
> #<Proxy "ajp://localhost:8009/docs">
> #ProxySet connectiontimeout=5 timeout=30 secret=%A1b2!@
> #</Proxy>
>
> #<Location "/docs">
> #  ProxyPass  ajp://localhost:8009/docs secret=%A1b2!@ servletnormalizecheck
> #</Location>
>
> What is not supported is
> curl -v --path-as-is "http://localhost:8000/docs/..;foo=bar/;foo=bar/test/index.jsp"
>
> that could be remapped to
> ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@ servletnormalizecheck
> or a <location test/>
>
> Comments?

I understood from Mark that the request you do above with curl should not be denied but just mapped to /test.
But rethinking that, it becomes real fun: For mapping we should use the URI stripped off path parameters and then having done the
shrinking operation (servlet normalized) but we should use the original URI having done the shrinking operation with path
parameters to sent to the backend. That might work for a simple prefix matching, but it seems to be very difficult for regular
expression scenarios where you might use complex captures from the matching to build the result. But if the matching was done
against the servlet normalized URI the captures might be different, than the ones you would have got when doing the same against
not normalized URI. So I am little bit lost here.
What if we just have an option on virtual host base to drop path parameters of the following kind

s#/([.]{0,2})(;[^/]*)/#/$1/g

do the usual shrinking operation afterwards and just process them afterwards as usual.

Regards

Rüdiger

On 10/06/2020 11:53, Ruediger Pluem wrote:
>
>
> On 6/9/20 12:05 PM, jean-frederic clere wrote:
>> Hi,
>>
>> Basically it adds servletnormalizecheck to mod_proxy for ProxyPass/ProxyPassMatch and mod_rewrite when using P
>> I have tested the following uses:
>> #ProxyPass  /docs ajp://localhost:8009/docs secret=%A1b2!@ servletnormalizecheck
>>
>> #ProxyPassMatch  "^/docs(.*)$" "ajp://localhost:8009/docs$1" secret=%A1b2!@ servletnormalizecheck
>>
>> #RewriteEngine On
>> #RewriteRule "^/docs(.*)$" "ajp://localhost:8009/docs$1" [P,SNC]
>> #<Proxy "ajp://localhost:8009/docs">
>> #ProxySet connectiontimeout=5 timeout=30 secret=%A1b2!@
>> #</Proxy>
>>
>> #<Location "/docs">
>> #  ProxyPass  ajp://localhost:8009/docs secret=%A1b2!@ servletnormalizecheck
>> #</Location>
>>
>> What is not supported is
>> curl -v --path-as-is "http://localhost:8000/docs/..;foo=bar/;foo=bar/test/index.jsp"
>>
>> that could be remapped to
>> ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@ servletnormalizecheck
>> or a <location test/>
>>
>> Comments?
>
> I understood from Mark that the request you do above with curl should not be denied but just mapped to /test.
> But rethinking that, it becomes real fun: For mapping we should use the URI stripped off path parameters and then having done the
> shrinking operation (servlet normalized) but we should use the original URI having done the shrinking operation with path
> parameters to sent to the backend. That might work for a simple prefix matching, but it seems to be very difficult for regular
> expression scenarios where you might use complex captures from the matching to build the result. But if the matching was done
> against the servlet normalized URI the captures might be different, than the ones you would have got when doing the same against
> not normalized URI. So I am little bit lost here.
> What if we just have an option on virtual host base to drop path parameters of the following kind
>
> s#/([.]{0,2})(;[^/]*)/#/$1/g
>
> do the usual shrinking operation afterwards and just process them afterwards as usual.

I think it makes sense to have it there but separated from the
servletnormalizecheck because that changes the whole <VirtualHost/> mapping
So I will add something like MergeSlashes which will map
http://localhost:8000/docs/..;foo=bar/;foo=bar/test/index.jsp
to /test
And arrange the proxy so that /docs/..;foo=bar/;foo=bar/test/index.jsp
is sent to the back-end.

Should I commit my first proposal (it is easily backportable to 2.4.x)
and later work on the next one?

>
> Regards
>
> Rüdiger
>
>


--
Cheers

Jean-Frederic

On Thu, Jun 11, 2020 at 8:52 AM jean-frederic clere <jfclere@gmail.com> wrote:
>
> Should I commit my first proposal (it is easily backportable to 2.4.x)
> and later work on the next one?

How about something like the attached patch?

It's a new single ap_normalize_path() helper with options (like
AP_NORMALIZE_MERGE_SLASHES and AP_NORMALIZE_PATH_PARAMETERS), which we
can use in multiple places to replace ap_getparents(). The patch is
without the mod_rewrite bits (for now), and uses the "mapping=servlet"
syntax for the new proxypass parameter (which I found more
extensible).

The issue with re-calling ap_getparents() in ap_proxy_trans_match()
(or ap_normalize_path() in my patch still) is that it happens after
%-decoding, so bets are off.
For instance "/docs/..%3Bfoo=bar/%3Bfoo=bar/test/index.jsp" is not the
same as "http://localhost:8000/docs/..;foo=bar/;foo=bar/test/index.jsp"
and shouldn't be matched the same by mapping=servlet.

We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
if we want to normalize a second time..


Regards;
Yann.

On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
> if we want to normalize a second time..

IOW, this block in ap_process_request_internal():

/* Ignore URL unescaping for proxy requests */
if (!r->proxyreq && r->parsed_uri.path) {
d = ap_get_core_module_config(r->per_dir_config);
if (d->allow_encoded_slashes) {
access_status = ap_unescape_url_keep2f(r->parsed_uri.path,
d->decode_encoded_slashes);
}
else {
access_status = ap_unescape_url(r->parsed_uri.path);
}
if (access_status) {
if (access_status == HTTP_NOT_FOUND) {
if (! d->allow_encoded_slashes) {
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00026)
"found %%2f (encoded '/') in URI "
"(decoded='%s'), returning 404",
r->uri);
}
}
return access_status;
}
}

Should go _after_ the following:

if ((access_status = ap_run_translate_name(r))) {
return decl_die(access_status, "translate", r);
}


But it looks like a breaking change for 2.4.x..

On 11/06/2020 07:51, jean-frederic clere wrote:
> On 10/06/2020 11:53, Ruediger Pluem wrote:
>>
>>
>> On 6/9/20 12:05 PM, jean-frederic clere wrote:
>>> Hi,
>>>
>>> Basically it adds servletnormalizecheck to mod_proxy for
>>> ProxyPass/ProxyPassMatch and mod_rewrite when using P
>>> I have tested the following uses:
>>> #ProxyPass  /docs ajp://localhost:8009/docs secret=%A1b2!@
>>> servletnormalizecheck
>>>
>>> #ProxyPassMatch  "^/docs(.*)$" "ajp://localhost:8009/docs$1"
>>> secret=%A1b2!@ servletnormalizecheck
>>>
>>> #RewriteEngine On
>>> #RewriteRule "^/docs(.*)$" "ajp://localhost:8009/docs$1" [P,SNC]
>>> #<Proxy "ajp://localhost:8009/docs">
>>> #ProxySet connectiontimeout=5 timeout=30 secret=%A1b2!@
>>> #</Proxy>
>>>
>>> #<Location "/docs">
>>> #  ProxyPass  ajp://localhost:8009/docs secret=%A1b2!@
>>> servletnormalizecheck
>>> #</Location>
>>>
>>> What is not supported is
>>> curl -v --path-as-is
>>> "http://localhost:8000/docs/..;foo=bar/;foo=bar/test/index.jsp"
>>>
>>> that could be remapped to
>>> ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@
>>> servletnormalizecheck
>>> or a <location test/>
>>>
>>> Comments?
>>
>> I understood from Mark that the request you do above with curl should
>> not be denied but just mapped to /test.
>> But rethinking that, it becomes real fun: For mapping we should use
>> the URI stripped off path parameters and then having done the
>> shrinking operation (servlet normalized) but we should use the
>> original URI having done the shrinking operation with path
>> parameters to sent to the backend. That might work for a simple prefix
>> matching, but it seems to be very difficult for regular
>> expression scenarios where you might use complex captures from the
>> matching to build the result. But if the matching was done
>> against the servlet normalized URI the captures might be different,
>> than the ones you would have got when doing the same against
>> not normalized URI. So I am little bit lost here.

I can see how this gets complicated for regular expression scenarios.

Since the servlet specification doesn't have the concept of regular
expression mapping, I don't think the rationale for servletnormalize
applies in that case. There is no expectation of how the mapping will
occur from a servlet perspective so the httpd behaviour cannot be
unexpected.

Coming from a servlet perspective I have no view on what the 'correct'
behaviour is in this case. I'll happily support whatever the httpd
community thinks is best.

>> What if we just have an option on virtual host base to drop path
>> parameters of the following kind
>>
>> s#/([.]{0,2})(;[^/]*)/#/$1/g
>>
>> do the usual shrinking operation afterwards and just process them
>> afterwards as usual.
>
> I think it makes sense to have it there but separated from the
> servletnormalizecheck because that changes the whole <VirtualHost/> mapping
> So I will add something like MergeSlashes which will map
> http://localhost:8000/docs/..;foo=bar/;foo=bar/test/index.jsp
> to /test
> And arrange the proxy so that /docs/..;foo=bar/;foo=bar/test/index.jsp
> is sent to the back-end.

That sounds good to me. That is the expected mapping from a servlet
perspective.

Thanks for all your efforts on this.

Mark

On Thu, Jun 11, 2020 at 9:57 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >
> > We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
> > if we want to normalize a second time..
>
> IOW, this block in ap_process_request_internal():
[snip]
> Should go _after_ the following:
[snip]

Or we could introduce a new pre_translate_name hook which would
execute before %-decoding, and be used by mod_proxy when
"ProxyPreTranslation on" is configured, and be a prerequisite for
mapping=servlet.

I find ProxyPreTranslation also useful for the non-servlet case btw.

Something like this attached v2 patch.

Regards;
Yann.

On Thu, Jun 11, 2020 at 1:22 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Thu, Jun 11, 2020 at 9:57 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >
> > On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> > >
> > > We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
> > > if we want to normalize a second time..
> >
> > IOW, this block in ap_process_request_internal():
> [snip]
> > Should go _after_ the following:
> [snip]
>
> Or we could introduce a new pre_translate_name hook which would
> execute before %-decoding, and be used by mod_proxy when
> "ProxyPreTranslation on" is configured, and be a prerequisite for
> mapping=servlet.
>
> I find ProxyPreTranslation also useful for the non-servlet case btw.
>
> Something like this attached v2 patch.

Here is a v3 with the relevant pre_translate_name hooks only and
ap_getparents() preserved when the URI does not start with '/' (which
makes the patch read better too).

>
> Regards;
> Yann.

On 6/11/20 9:50 AM, Yann Ylavic wrote:
> On Thu, Jun 11, 2020 at 8:52 AM jean-frederic clere <jfclere@gmail.com> wrote:
>>
>> Should I commit my first proposal (it is easily backportable to 2.4.x)
>> and later work on the next one?
>
> How about something like the attached patch?

Looks good in general, but

1. Why do we need to switch to the new API in other parts of the code?
2. It doesn't tackle the mod_rewrite issue (but I think you mentioned this somewhere).
3. It doesn't tackle Location / LocationMatch / if issues.

If you did something like

<Location /admin>

do some auth

</Location>

ProxyPass / http://backend/

then

/app/..;foo=bar/admin

would bypass this auth on Apache reverse proxy layer.
This convinces me even more that we need to fix this by a virtual host specific settings that
strips off all path parameters at least for '.' and '..' segments unless it is on the last segment and this segment is NOT '..' or
'.'.
Of course this might cause issues if other parts of this virtual host need these path parameters and
deal with them in the HTTP sense, but guess in practice this case is rather rare.

The next question is: All the patches here deal with ';' as an indicator for a path parameter. What about the the other sub-delims
specified in RFC3986?

sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="

Regards

Rüdiger

On 11/06/2020 13:50, Yann Ylavic wrote:
> On Thu, Jun 11, 2020 at 1:22 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>
>> On Thu, Jun 11, 2020 at 9:57 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>>
>>> On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>>>
>>>> We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
>>>> if we want to normalize a second time..
>>>
>>> IOW, this block in ap_process_request_internal():
>> [snip]
>>> Should go _after_ the following:
>> [snip]
>>
>> Or we could introduce a new pre_translate_name hook which would
>> execute before %-decoding, and be used by mod_proxy when
>> "ProxyPreTranslation on" is configured, and be a prerequisite for
>> mapping=servlet.
>>
>> I find ProxyPreTranslation also useful for the non-servlet case btw.
>>
>> Something like this attached v2 patch.
>
> Here is a v3 with the relevant pre_translate_name hooks only and
> ap_getparents() preserved when the URI does not start with '/' (which
> makes the patch read better too).

with this patch, how to I get:
curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp

Mapped to
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
Or rejected in case I have only:
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@


>
>>
>> Regards;
>> Yann.


--
Cheers

Jean-Frederic

On Sat, Jun 13, 2020 at 11:18 AM jean-frederic clere <jfclere@gmail.com> wrote:
>
> On 11/06/2020 13:50, Yann Ylavic wrote:
> > On Thu, Jun 11, 2020 at 1:22 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >>
> >> On Thu, Jun 11, 2020 at 9:57 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >>>
> >>> On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >>>>
> >>>> We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
> >>>> if we want to normalize a second time..
> >>>
> >>> IOW, this block in ap_process_request_internal():
> >> [snip]
> >>> Should go _after_ the following:
> >> [snip]
> >>
> >> Or we could introduce a new pre_translate_name hook which would
> >> execute before %-decoding, and be used by mod_proxy when
> >> "ProxyPreTranslation on" is configured, and be a prerequisite for
> >> mapping=servlet.
> >>
> >> I find ProxyPreTranslation also useful for the non-servlet case btw.
> >>
> >> Something like this attached v2 patch.
> >
> > Here is a v3 with the relevant pre_translate_name hooks only and
> > ap_getparents() preserved when the URI does not start with '/' (which
> > makes the patch read better too).
>
> with this patch, how to I get:
> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp
>
> Mapped to
> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
> Or rejected in case I have only:
> ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@

Right sorry, it does not work with patch v3, I mainly focused on the
"decode at the right place" part of the issue, which is not your
point..

I just staged a more complete proposal in
https://github.com/apache/httpd/pull/128

For the proxy servlet part, I think that we need a dedicated
alias_match() for servlet mapping (called alias_match_servlet() in the
PR), we can't normalize and match separately or the matched length is
completely off wrt the original URI-path.

Can you please try with the patches there? (the last is not really
necessary, it's just to complete the PR should this be merged).

You need to set:
ProxyMappingDecoded off
in your vhost (or directory) for servlet mapping to be active, with a
ProxyPass like:
ProxyPass /good/ http://127.0.0.1:80/good/ mapping=servlet

I tried with paths like
"/bad/..;foo=bar/.;foo=bar//other;foo=bar//..;foo=bar/good;foo1=bar1/;foo2=bar2/.;foo3=bar3///./index.html"
which results in "/good/;foo2=bar2/.;foo3=bar3///./index.html" being
forwarded, still things that shouldn't be seem to be declined.

The code in alias_match_servlet() is not really simple, but neither is
servlet mapping..


Regards;
Yann.

On 17/06/2020 13:26, Yann Ylavic wrote:
> On Sat, Jun 13, 2020 at 11:18 AM jean-frederic clere <jfclere@gmail.com> wrote:
>>
>> On 11/06/2020 13:50, Yann Ylavic wrote:
>>> On Thu, Jun 11, 2020 at 1:22 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>>>
>>>> On Thu, Jun 11, 2020 at 9:57 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>>>>
>>>>> On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>>>>>
>>>>>> We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
>>>>>> if we want to normalize a second time..
>>>>>
>>>>> IOW, this block in ap_process_request_internal():
>>>> [snip]
>>>>> Should go _after_ the following:
>>>> [snip]
>>>>
>>>> Or we could introduce a new pre_translate_name hook which would
>>>> execute before %-decoding, and be used by mod_proxy when
>>>> "ProxyPreTranslation on" is configured, and be a prerequisite for
>>>> mapping=servlet.
>>>>
>>>> I find ProxyPreTranslation also useful for the non-servlet case btw.
>>>>
>>>> Something like this attached v2 patch.
>>>
>>> Here is a v3 with the relevant pre_translate_name hooks only and
>>> ap_getparents() preserved when the URI does not start with '/' (which
>>> makes the patch read better too).
>>
>> with this patch, how to I get:
>> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp
>>
>> Mapped to
>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
>> Or rejected in case I have only:
>> ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@
>
> Right sorry, it does not work with patch v3, I mainly focused on the
> "decode at the right place" part of the issue, which is not your
> point..
>
> I just staged a more complete proposal in
> https://github.com/apache/httpd/pull/128
>
> For the proxy servlet part, I think that we need a dedicated
> alias_match() for servlet mapping (called alias_match_servlet() in the
> PR), we can't normalize and match separately or the matched length is
> completely off wrt the original URI-path.
>
> Can you please try with the patches there? (the last is not really
> necessary, it's just to complete the PR should this be merged).
>
> You need to set:
> ProxyMappingDecoded off
> in your vhost (or directory) for servlet mapping to be active, with a
> ProxyPass like:
> ProxyPass /good/ http://127.0.0.1:80/good/ mapping=servlet
>
> I tried with paths like
> "/bad/..;foo=bar/.;foo=bar//other;foo=bar//..;foo=bar/good;foo1=bar1/;foo2=bar2/.;foo3=bar3///./index.html"
> which results in "/good/;foo2=bar2/.;foo3=bar3///./index.html" being
> forwarded, still things that shouldn't be seem to be declined.
>
> The code in alias_match_servlet() is not really simple, but neither is
> servlet mapping..

OK we are going forward:
ProxyMappingDecoded Off
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
and curl -v --path-as-is
"http://localhost:8000/docs/..;food=bar/test/index.jsp 404 httpd.

ProxyMappingDecoded Off
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
and curl -v --path-as-is
"http://localhost:8000/docs/..;food=bar/test/index.jsp 200 tc URL:
http://localhost:8000/test/index.jsp
but curl -v --path-as-is
"http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp" 404 httpd
what is going wrong with
"http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp"
same for "curl -v --path-as-is
"http://localhost:8000/test;food=bar/index.jsp"

ProxyMappingDecoded On
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
mapping=servlet 404 httpd.

ProxyMappingDecoded On
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ 404 httpd.

Comments?

>
>
> Regards;
> Yann.
>


--
Cheers

Jean-Frederic

On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere <jfclere@gmail.com> wrote:
>
> ProxyMappingDecoded Off
> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
[]
> what is going wrong with
> "http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp"
> same for "curl -v --path-as-is
> "http://localhost:8000/test;food=bar/index.jsp"

Good catch, should be fixed with
https://github.com/apache/httpd/compare/491a115344e37df21996f323eefd16136d278360..d9f12223ba45e520dd018baf7be084809d531d81
Latest version of the PR should be OK.

Now it results in: ajp://localhost:8009/test;food=bar/index.jsp
We keep the path parameters since the alias (/test) does not end with '/'.

>
> ProxyMappingDecoded On
> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
> mapping=servlet 404 httpd.
>
> ProxyMappingDecoded On
> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ 404 httpd.

Hmm, I can't reproduce these ones, they do not take the
alias_match_servlet() path and should not be affected by my changes.
Can you still reproduce with the latest version? I made somes pushes
yesterday, perhaps a transient invalid state..


Regards;
Yann.

On 19/06/2020 12:02, Yann Ylavic wrote:
> On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere <jfclere@gmail.com> wrote:
>>
>> ProxyMappingDecoded Off
>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
> []
>> what is going wrong with
>> "http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp"
>> same for "curl -v --path-as-is
>> "http://localhost:8000/test;food=bar/index.jsp"
>
> Good catch, should be fixed with
> https://github.com/apache/httpd/compare/491a115344e37df21996f323eefd16136d278360..d9f12223ba45e520dd018baf7be084809d531d81
> Latest version of the PR should be OK.
>
> Now it results in: ajp://localhost:8009/test;food=bar/index.jsp
> We keep the path parameters since the alias (/test) does not end with '/'.

Cool fixed.

>
>>
>> ProxyMappingDecoded On
>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
>> mapping=servlet 404 httpd.
>>
>> ProxyMappingDecoded On
>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ 404 httpd.
>
> Hmm, I can't reproduce these ones, they do not take the
> alias_match_servlet() path and should not be affected by my changes.
> Can you still reproduce with the latest version? I made somes pushes
> yesterday, perhaps a transient invalid state...

In fact I was screwing it, sorryt:

But there is still something I want to prevent:
ProxyPass /docs ajp://localhost:8009/docs
and url like:
curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
How do we do that? Do we want a 400 for that? (my proposal do that :-)).

>
>
> Regards;
> Yann.
>


--
Cheers

Jean-Frederic

On Mon, Jun 22, 2020 at 11:20 AM jean-frederic clere <jfclere@gmail.com> wrote:
>
> On 19/06/2020 12:02, Yann Ylavic wrote:
> > On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere <jfclere@gmail.com> wrote:
> >>
> >> ProxyMappingDecoded Off
> >> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
> > []
> >> what is going wrong with
> >> "http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp"
> >> same for "curl -v --path-as-is
> >> "http://localhost:8000/test;food=bar/index.jsp"
> >
> > Good catch, should be fixed with
> > https://github.com/apache/httpd/compare/491a115344e37df21996f323eefd16136d278360..d9f12223ba45e520dd018baf7be084809d531d81
> > Latest version of the PR should be OK.
> >
> > Now it results in: ajp://localhost:8009/test;food=bar/index.jsp
> > We keep the path parameters since the alias (/test) does not end with '/'.
>
> Cool fixed.

Thanks for testing.

>
> >
> >>
> >> ProxyMappingDecoded On
> >> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
> >> mapping=servlet 404 httpd.
> >>
> >> ProxyMappingDecoded On
> >> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ 404 httpd.
> >
> > Hmm, I can't reproduce these ones, they do not take the
> > alias_match_servlet() path and should not be affected by my changes.
> > Can you still reproduce with the latest version? I made somes pushes
> > yesterday, perhaps a transient invalid state...
>
> In fact I was screwing it, sorryt:
>
> But there is still something I want to prevent:
> ProxyPass /docs ajp://localhost:8009/docs
> and url like:
> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
> How do we do that? Do we want a 400 for that? (my proposal do that :-)).

Why would we 400?
Either there is a mapping for /test[/] and we'll be OK, or there is
none we'll be DECLINED.

The 400 will come only if no module handles the URI, and if the
default_handler() finds no "docs/..;food=bar/test/index.jsp" in the
path (where "..;foo=bar" is not considered a directory traversal in
this case).

On my system, this runs smoothly:
$ mkdir -p 'docs/..;foo=bar/test'
$ touch 'docs/..;foo=bar/test/index.php'
$ ls 'docs/..;foo=bar/test/index.php'
'docs/..;foo=bar/test/index.php'


Regards;
Yann.

On 22/06/2020 11:50, Yann Ylavic wrote:
> On Mon, Jun 22, 2020 at 11:20 AM jean-frederic clere <jfclere@gmail.com> wrote:
>>
>> On 19/06/2020 12:02, Yann Ylavic wrote:
>>> On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere <jfclere@gmail.com> wrote:
>>>>
>>>> ProxyMappingDecoded Off
>>>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
>>> []
>>>> what is going wrong with
>>>> "http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp"
>>>> same for "curl -v --path-as-is
>>>> "http://localhost:8000/test;food=bar/index.jsp"
>>>
>>> Good catch, should be fixed with
>>> https://github.com/apache/httpd/compare/491a115344e37df21996f323eefd16136d278360..d9f12223ba45e520dd018baf7be084809d531d81
>>> Latest version of the PR should be OK.
>>>
>>> Now it results in: ajp://localhost:8009/test;food=bar/index.jsp
>>> We keep the path parameters since the alias (/test) does not end with '/'.
>>
>> Cool fixed.
>
> Thanks for testing.
>
>>
>>>
>>>>
>>>> ProxyMappingDecoded On
>>>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@
>>>> mapping=servlet 404 httpd.
>>>>
>>>> ProxyMappingDecoded On
>>>> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ 404 httpd.
>>>
>>> Hmm, I can't reproduce these ones, they do not take the
>>> alias_match_servlet() path and should not be affected by my changes.
>>> Can you still reproduce with the latest version? I made somes pushes
>>> yesterday, perhaps a transient invalid state...
>>
>> In fact I was screwing it, sorryt:
>>
>> But there is still something I want to prevent:
>> ProxyPass /docs ajp://localhost:8009/docs
>> and url like:
>> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
>> How do we do that? Do we want a 400 for that? (my proposal do that :-)).
>
> Why would we 400?
> Either there is a mapping for /test[/] and we'll be OK, or there is
> none we'll be DECLINED.

For the moment I am getting a 200 and the test/index.jsp from tomcat...

>
> The 400 will come only if no module handles the URI, and if the
> default_handler() finds no "docs/..;food=bar/test/index.jsp" in the
> path (where "..;foo=bar" is not considered a directory traversal in
> this case).

ProxyPass /docs ajp://localhost:8009/docs
being mapped as /test/index.jsp (by tomcat) when you
query"http://localhost:8000/docs/..;food=bar/test/index.jsp" looks wrong
and should avoidable.

>
> On my system, this runs smoothly:
> $ mkdir -p 'docs/..;foo=bar/test'
> $ touch 'docs/..;foo=bar/test/index.php'
> $ ls 'docs/..;foo=bar/test/index.php'
> 'docs/..;foo=bar/test/index.php'
>

Correct the hardening is to prevent "tomcat customers mistake" that gets
unexpected contexts exposed. I am not able to get it working with you
proposal.

>
> Regards;
> Yann.
>


--
Cheers

Jean-Frederic

On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere <jfclere@gmail.com> wrote:
>
> >>
> >> But there is still something I want to prevent:
> >> ProxyPass /docs ajp://localhost:8009/docs
> >> and url like:
> >> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
> >> How do we do that? Do we want a 400 for that? (my proposal do that :-)).
> >
> > Why would we 400?
> > Either there is a mapping for /test[/] and we'll be OK, or there is
> > none we'll be DECLINED.
>
> For the moment I am getting a 200 and the test/index.jsp from tomcat...

Hmm, do you mean that mod_proxy (alias_match_servlet) forwards
http://localhost:8000/test/index.php in this case, even if there is no
mapping for "/test" ??

In my testing it's not mapped, so it ends up being handled by the
default_handler() which returns 404.

>
> >
> > The 400 will come only if no module handles the URI, and if the
> > default_handler() finds no "docs/..;food=bar/test/index.jsp" in the
> > path (where "..;foo=bar" is not considered a directory traversal in
> > this case).
>
> ProxyPass /docs ajp://localhost:8009/docs
> being mapped as /test/index.jsp (by tomcat) when you
> query"http://localhost:8000/docs/..;food=bar/test/index.jsp" looks wrong
> and should avoidable.
>
> >
> > On my system, this runs smoothly:
> > $ mkdir -p 'docs/..;foo=bar/test'
> > $ touch 'docs/..;foo=bar/test/index.php'
> > $ ls 'docs/..;foo=bar/test/index.php'
> > 'docs/..;foo=bar/test/index.php'
> >
>
> Correct the hardening is to prevent "tomcat customers mistake" that gets
> unexpected contexts exposed. I am not able to get it working with you
> proposal.

I don't think we should refuse anything in mod_proxy, either forward
or let it be handled elsewhere.

>
> >
> > Regards;
> > Yann.

On 22/06/2020 12:23, Yann Ylavic wrote:
> On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere <jfclere@gmail.com> wrote:
>>
>>>>
>>>> But there is still something I want to prevent:
>>>> ProxyPass /docs ajp://localhost:8009/docs
>>>> and url like:
>>>> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
>>>> How do we do that? Do we want a 400 for that? (my proposal do that :-)).
>>>
>>> Why would we 400?
>>> Either there is a mapping for /test[/] and we'll be OK, or there is
>>> none we'll be DECLINED.
>>
>> For the moment I am getting a 200 and the test/index.jsp from tomcat...
>
> Hmm, do you mean that mod_proxy (alias_match_servlet) forwards
> http://localhost:8000/test/index.php in this case, even if there is no
> mapping for "/test" ??

Yes :D
Well tomcat maps "http://localhost:8080/docs/..;food=bar/test/index.jsp"
to http://localhost:8080/test/index.jsp which looks bad if you only map
ProxyPass /docs ajp://localhost:8009/docs

>
> In my testing it's not mapped, so it ends up being handled by the
> default_handler() which returns 404.
>
>>
>>>
>>> The 400 will come only if no module handles the URI, and if the
>>> default_handler() finds no "docs/..;food=bar/test/index.jsp" in the
>>> path (where "..;foo=bar" is not considered a directory traversal in
>>> this case).
>>
>> ProxyPass /docs ajp://localhost:8009/docs
>> being mapped as /test/index.jsp (by tomcat) when you
>> query"http://localhost:8000/docs/..;food=bar/test/index.jsp" looks wrong
>> and should avoidable.
>>
>>>
>>> On my system, this runs smoothly:
>>> $ mkdir -p 'docs/..;foo=bar/test'
>>> $ touch 'docs/..;foo=bar/test/index.php'
>>> $ ls 'docs/..;foo=bar/test/index.php'
>>> 'docs/..;foo=bar/test/index.php'
>>>
>>
>> Correct the hardening is to prevent "tomcat customers mistake" that gets
>> unexpected contexts exposed. I am not able to get it working with you
>> proposal.
>
> I don't think we should refuse anything in mod_proxy, either forward
> or let it be handled elsewhere.

I disagree and think that some should be rejected ;-) but of course I am
using httpd to protect "a" back-end. That behaviour should NOT be the
default behavior (and yes mod_rewrite or mod_security are also there to
help).

>
>>
>>>
>>> Regards;
>>> Yann.


--
Cheers

Jean-Frederic

On Mon, Jun 22, 2020 at 12:33 PM jean-frederic clere <jfclere@gmail.com> wrote:
>
> On 22/06/2020 12:23, Yann Ylavic wrote:
> > On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere <jfclere@gmail.com> wrote:
> >>
> >>>>
> >>>> But there is still something I want to prevent:
> >>>> ProxyPass /docs ajp://localhost:8009/docs
> >>>> and url like:
> >>>> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
> >>>> How do we do that? Do we want a 400 for that? (my proposal do that :-)).
> >>>
> >>> Why would we 400?
> >>> Either there is a mapping for /test[/] and we'll be OK, or there is
> >>> none we'll be DECLINED.
> >>
> >> For the moment I am getting a 200 and the test/index.jsp from tomcat...
> >
> > Hmm, do you mean that mod_proxy (alias_match_servlet) forwards
> > http://localhost:8000/test/index.php in this case, even if there is no
> > mapping for "/test" ??
>
> Yes :D

I can't reproduce, did you forget "ProxyMappingDecoded off" by any chance?

> Well tomcat maps "http://localhost:8080/docs/..;food=bar/test/index.jsp"
> to http://localhost:8080/test/index.jsp which looks bad if you only map
> ProxyPass /docs ajp://localhost:8009/docs

Sure, but mod_proxy shouldn't forward
"/docs/..;food=bar/test/index.jsp" if there is no mapping for "/test",
but only when servlet mapping is activated. Otherwise the "normal"
mapping applies, which with "/docs" as alias does indeed forward the
above..

Remember that currently with my patch, servlet mapping only applies in
pre_translate_name hook, before URI-path decoding, because we don't
want that "%3B" be decoded first and then interpreted as ';' by
servlet mapping (this is not a sub-delims when encoded), thus
"ProxyMappingDecoded off" is required.

Now that the patches are committed to svn (I just did), I was about to
open another thread about this or more generally how we should handle
decoding w.r.t. ProxyMappingDecoded, because as it stands
ProxyMappingDecoded will affect all location / directory / file /
etc.. walks and matchings, since r->uri will remain non-decoded for
the whole request handling.
That may be what we want, but a mod_proxy directive for this can look
a bit surprising since it affects the core handling too.


Regards;
Yann.

> You need to set:
> ProxyMappingDecoded off
> in your vhost (or directory) for servlet mapping to be active, with a

Does it work in directory context? pre_trans is before location_walk.

On Mon, Jun 22, 2020 at 2:44 PM Eric Covener <covener@gmail.com> wrote:
>
> > You need to set:
> > ProxyMappingDecoded off
> > in your vhost (or directory) for servlet mapping to be active, with a
>
> Does it work in directory context? pre_trans is before location_walk.

Argh no, didn't think of it :/

For this we have to add a third location walk in
ap_process_request_internal(), something like the attached.
To minimize impact, I save the original r->uri and don't re-walk if it
didn't change, that should address most common cases IMHO.

Would that work?

On 22/06/2020 13:02, Yann Ylavic wrote:
> On Mon, Jun 22, 2020 at 12:33 PM jean-frederic clere <jfclere@gmail.com> wrote:
>>
>> On 22/06/2020 12:23, Yann Ylavic wrote:
>>> On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere <jfclere@gmail.com> wrote:
>>>>
>>>>>>
>>>>>> But there is still something I want to prevent:
>>>>>> ProxyPass /docs ajp://localhost:8009/docs
>>>>>> and url like:
>>>>>> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
>>>>>> How do we do that? Do we want a 400 for that? (my proposal do that :-)).
>>>>>
>>>>> Why would we 400?
>>>>> Either there is a mapping for /test[/] and we'll be OK, or there is
>>>>> none we'll be DECLINED.
>>>>
>>>> For the moment I am getting a 200 and the test/index.jsp from tomcat...
>>>
>>> Hmm, do you mean that mod_proxy (alias_match_servlet) forwards
>>> http://localhost:8000/test/index.php in this case, even if there is no
>>> mapping for "/test" ??
>>
>> Yes :D
>
> I can't reproduce, did you forget "ProxyMappingDecoded off" by any chance?

Oops: retesting:
curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"


ProxyMappingDecoded On
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@
Mapped to tomcat

ProxyMappingDecoded On
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
Mapped to tomcat

ProxyMappingDecoded Off
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
404 httpd

ProxyMappingDecoded Off
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@
Mapped to tomcat

>
>> Well tomcat maps "http://localhost:8080/docs/..;food=bar/test/index.jsp"
>> to http://localhost:8080/test/index.jsp which looks bad if you only map
>> ProxyPass /docs ajp://localhost:8009/docs
>
> Sure, but mod_proxy shouldn't forward
> "/docs/..;food=bar/test/index.jsp" if there is no mapping for "/test",
> but only when servlet mapping is activated. Otherwise the "normal"
> mapping applies, which with "/docs" as alias does indeed forward the
> above..
>
> Remember that currently with my patch, servlet mapping only applies in
> pre_translate_name hook, before URI-path decoding, because we don't
> want that "%3B" be decoded first and then interpreted as ';' by
> servlet mapping (this is not a sub-delims when encoded), thus
> "ProxyMappingDecoded off" is required.

OK. Using:
curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
ProxyMappingDecoded Off
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
Mapped to tomcat and the log:
+++
[Mon Jun 22 14:07:29.840763 2020] [proxy:trace2] [pid 36180:tid
140211478607616] proxy_util.c(2220): [client ::1:54188] ajp: found
worker ajp://localhost/test for ajp://localhost/test/index.jsp
+++
Correct it uses ajp://localhost/test
Looks good.

>
> Now that the patches are committed to svn (I just did),

Cool back port to 2.4.x is that easy correct?

> I was about to
> open another thread about this or more generally how we should handle
> decoding w.r.t. ProxyMappingDecoded, because as it stands
> ProxyMappingDecoded will affect all location / directory / file /
> etc.. walks and matchings, since r->uri will remain non-decoded for
> the whole request handling.
> That may be what we want, but a mod_proxy directive for this can look
> a bit surprising since it affects the core handling too.

Yes the name ProxyMappingDecoded now looks weird ;-)

>
>
> Regards;
> Yann.
>


--
Cheers

Jean-Frederic

On 22/06/2020 16:12, Yann Ylavic wrote:
> On Mon, Jun 22, 2020 at 2:44 PM Eric Covener <covener@gmail.com> wrote:
>>
>>> You need to set:
>>> ProxyMappingDecoded off
>>> in your vhost (or directory) for servlet mapping to be active, with a
>>
>> Does it work in directory context? pre_trans is before location_walk.
>
> Argh no, didn't think of it :/
>
> For this we have to add a third location walk in
> ap_process_request_internal(), something like the attached.
> To minimize impact, I save the original r->uri and don't re-walk if it
> didn't change, that should address most common cases IMHO.
>
> Would that work?
>

Do we want:
curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
ProxyMappingDecoded Off
<Location "/docs">
ProxyPass ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
</Location>
<Location "/test">
ProxyPass ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
</Location>
To map to tomcat?

like:
ProxyMappingDecoded Off
ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet


--
Cheers

Jean-Frederic

On Mon, Jun 22, 2020 at 5:04 PM jean-frederic clere <jfclere@gmail.com> wrote:
>
> On 22/06/2020 16:12, Yann Ylavic wrote:
> > On Mon, Jun 22, 2020 at 2:44 PM Eric Covener <covener@gmail.com> wrote:
> >>
> >>> You need to set:
> >>> ProxyMappingDecoded off
> >>> in your vhost (or directory) for servlet mapping to be active, with a
> >>
> >> Does it work in directory context? pre_trans is before location_walk.
> >
> > Argh no, didn't think of it :/
> >
> > For this we have to add a third location walk in
> > ap_process_request_internal(), something like the attached.
> > To minimize impact, I save the original r->uri and don't re-walk if it
> > didn't change, that should address most common cases IMHO.
> >
> > Would that work?
> >
>
> Do we want:
> curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
> ProxyMappingDecoded Off
> <Location "/docs">
> ProxyPass ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
> </Location>
> <Location "/test">
> ProxyPass ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
> </Location>
> To map to tomcat?
>
> like:
> ProxyMappingDecoded Off
> ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
> ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet

Yes, that's the idea. I didn't test, but the patch is supposed to
allow for that, or:

<Proxy "ajp://localhost:8009/docs">
ProxyMappingDecoded Off
ProxySet secret=%A1b2!@ mapping=servlet
ProxyPass /docs
</Proxy>
<Proxy "ajp://localhost:8009/test">
ProxyMappingDecoded Off
ProxySet secret=%A1b2!@ mapping=servlet
ProxyPass /test
</Proxy>


Regards;
Yann.

On Mon, Jun 22, 2020 at 5:13 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Mon, Jun 22, 2020 at 5:04 PM jean-frederic clere <jfclere@gmail.com> wrote:
> >
> > Do we want:
> > curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp"
> > ProxyMappingDecoded Off
> > <Location "/docs">
> > ProxyPass ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
> > </Location>
> > <Location "/test">
> > ProxyPass ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
> > </Location>
> > To map to tomcat?

It should work now with my latest updates to trunk.
As Eric noticed, the original version did not work in location context.

> >
> > like:
> > ProxyMappingDecoded Off
> > ProxyPass /docs ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
> > ProxyPass /test ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet

ProxyMappingDecoded is not needed anymore (and was removed).
The mapping= tells mod_proxy at which stage ([pre_]translate) it
should map the request path.
For "servlet" mapping (and the other existing "encoded" mapping, which
"servlet" is a subset of), that's pre_trans when r->uri is still
encoded.
I think it simplifies quite some things to handle it this way.

>
> I didn't test, but the patch is supposed to
> allow for that, or:
>
> <Proxy "ajp://localhost:8009/docs">
> ProxyMappingDecoded Off
> ProxySet secret=%A1b2!@ mapping=servlet
> ProxyPass /docs
> </Proxy>
> <Proxy "ajp://localhost:8009/test">
> ProxyMappingDecoded Off
> ProxySet secret=%A1b2!@ mapping=servlet
> ProxyPass /test
> </Proxy>

Scratch that, <Proxy context is resolved at a later stage/hook, so I
don't think that an inner ProxyPass mapping= can work as expected.
Also, mapping= is a property of the ProxyPass, not the worker..


Regards;
Yann.