Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-24 Thread Eric Covener
>
> ProxyMappingDecoded is not needed anymore (and was removed).
> The mapping= tells mod_proxy at which stage ([pre_]translate) it
> should map the request path.
+1


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-24 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 5:13 PM Yann Ylavic  wrote:
>
> On Mon, Jun 22, 2020 at 5:04 PM jean-frederic clere  wrote:
> >
> > Do we want:
> > curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp;
> > ProxyMappingDecoded Off
> > 
> >ProxyPass  ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
> > 
> > 
> >ProxyPass  ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
> > 
> > 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:
>
> 
>ProxyMappingDecoded Off
>ProxySet secret=%A1b2!@ mapping=servlet
>ProxyPass /docs
> 
> 
>ProxyMappingDecoded Off
>ProxySet secret=%A1b2!@ mapping=servlet
>ProxyPass /test
> 

Scratch that, 

Re: svn commit: r1879080 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.c mod_proxy.h

2020-06-24 Thread Yann Ylavic
On Tue, Jun 23, 2020 at 9:59 AM Ruediger Pluem  wrote:
>
> On 6/22/20 12:37 PM, yla...@apache.org 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.
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..
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.


Regards,
Yann.