On 6/24/20 1:09 PM, Yann Ylavic wrote:
> On Tue, Jun 23, 2020 at 9:59 AM Ruediger Pluem <[email protected]> wrote:
>>
>> On 6/22/20 12:37 PM, [email protected] wrote:
>>>
>>> +/*
>>> + * Inspired by mod_jk's jk_servlet_normalize().
>>> + */
>>> +static int alias_match_servlet(apr_pool_t *p,
>>> + const char *uri,
>>> + const char *alias)
>>> +{
>>
>> The code for alias_match_servlet looks awful complex.
>
> Yeah I know..
>
>> I think it is this way not because it is done wrong in some way, but because
>> the problem to solve is complex. This brought me to the point if it is worth
>> having this complexity.
>
> I suppose that it helps to close a mod_jk vs mod_proxy_ajp gap, but I
> don't use either actually.
> It seems that Jean-Frédéric is interested in the feature, probably
> some Tomcat users too who want the proxy to isolate/restrict
> applications (paths) on the proxy.
>
>> My understanding is that the original issue we faced was that traversal
>> problem as HTTP and Servlet spec handle path parameters
>> differently in the '..' case. So we need to do something about
>>
>> /app/..;pp=somevalue/admin
>>
>> URI patterns. The question for me is: Are URI's that contain these patterns
>> sensible for a servlet based application or can we
>> always assume bad intentions by the originator?
>
> Dunno, we do not assume bad intentions for "../" patterns in
> non-servlet paths though, and normalize them.
In non-servlet paths we just normalize them in case of ../, but we just keep
them in case of ..;pp=somevalue/.
I think now in the servlet case we silently ignore the path parameter and just
normalize as if it was just ../
> I know about the OpenAPI specification, and you probably don't want to
> see how applications put segment parameters all over the place ;)
> All I can tell is that in the API/REST world, it's quite used (never
> saw the "..;" thing, though).
>
>> If we always assume bad intentions we could just reject such URI's (probably
>> only
>> if they match a ProxyPass with a flag set, for the Rewrite case we could
>> just document a Rewrite rule that kills them).
>
> Could be, but the nominal use case is probably the below (IMHO).
>
>> The other case I see covered with alias_match_servlet is the case where we
>> have path parameters on the prefix part that ProxyPass
>> should match. Without this commit
>>
>> /app;pp=something/somethingelse
>>
>> wouldn't match
>>
>> ProxyPass /app schema://backend/app
>
> This, I suppose applications that handle path/segment parameters want
> to see the ones for /app.
> That's a real use case IMO.
>
>>
>> But given the complexity of the code I am not sure if these cases are worth
>> fixing or if people should just use ProyPassMatch / a
>> RewriteRule that covers such cases if the application needs that. Of course
>> that depends on how often such cases appear. If they
>> are frequent than a direct ProxyPass support seems sensible. If they are
>> rare probably not.
>
> Yeah, it depends, Tomcat users are probably more able to answer this than me.
> I _think_ I got alias_match_servlet() right, sure it's complex, but we
> have it now..
Yes and this is fine. Thanks for doing. I just worry a little bit about if such
complex code isn't prone to vulnerabilities.
> It can possibly be simplified a bit (store more than a single int per
> segment in the stack, so that we don't need to maintain a normalized
> path separately for rewind), but it's complex anyway I concur.
>
> Possibly I'll need a mapping=openapi some day, at least I have a base
> implementation for that now (not sure it's interesting people either
> and that I would propose it upstream..).
>
> Anyway, if there is a need for app isolation on the proxy with
> accurate path parameters forwarding, it's not such a bad way to do it.
> If there is no need, and we simply want to block "..;", I agree that a
> RewriteRule is more than enough.
The other thing that still worries me and that I think is not covered is the
following scenario:
<Location /admin>
..... some authn and authz ....
</Location>
ProxyPass / http://
Here the Apache is used to do authn and authz for the backend.
/app/..;pp=somevalue/admin/nastything
IMHO could circumvent this no matter what the mapping option is set to in
ProxyPass
Regards
Rüdiger
