iOS 14 / macOS 11 and HTTP/3 support

2020-06-22 Thread Alex Hautequest
From Apple’s developer beta release notes, the newest Apple code is now 
shipping with HTTP/3 support. Disabled by default, but can be enabled by users. 
As of today, HTTP/3 Draft 29 isn’t yet supported.

Alex

Re: pre_translate_name hook vs location/directory walk (was: svn commit: r1879079)

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 2:57 PM Eric Covener  wrote:
>
> On Mon, Jun 22, 2020 at 8:28 AM Yann Ylavic  wrote:
> >
> > > Allow for URI-path pre_translate_name before (and/or instead of) decoding.
> > >
> > > Only if no hook takes "ownership" of the URI (returning OK), apply
> > > percent decoding for the rest of request handling. Otherwise r->uri 
> > > remains
> > > encoded meaning that further location/directory/file/if/.. sections 
> > > (walks)
> > > should that into account.
> >
> > This change allows a module to avoid URI decoding in
> > pre_translate_name, such that r->uri remains encoded for the whole
> > request handling.
> >
> > For instance (same series), mod_proxy will use that hook when
> > "ProxyMappingDecoded off" is configured, and return OK if a ProxyPass
> > matches. The decoding was already disabled (before this commit) for
> > the forward-proxy case (ProxyRequests on) since r->proxyreq was set at
> > an early stage, but now it "works" for the reverse-proxy case too.
> >
> > This means that when "ProxyMappingDecoded off" is configured (the
> > default is still "on", though), for instance a  > a fixup RewriteRule or a non-early RequestHeader rule will have to be
> > expressed with the encoded form since reserved characters will not be
> > decoded as before (note that unreserved characters will always be
> > decoded anyway).
> >
> > I don't know how much it can break existing configurations, and wonder
> > if we should have a core directive that still controls whether r->uri
> > should be decoded, regardless of ProxyMappingDecoded or any further
> > module pre_translate_name hook with its own directives.
> >
> > For mod_proxy this is not really an issue to have r->uri modified
> > after the (pre_)translate phase, because the handler will later work
> > on r->filename regardless (the "proxy:..." set by proxy_detect or
> > proxy_trans), so maybe we should simply always decode r->uri after the
> > pre_translate hooks have run. Though I find it easier in a proxy
> > context to match a URI-path (LocationMatch, RewriteRule..) in its
> > encoded form, and not worry about original vs decoded separators. That
> > may be just my preference, but a new core directive looks sane to me..
> >
> > Thoughts?
>
> I have not followed too closely so take with a grain of salt / for the
> sake of argument:

Thanks, always welcome!

>
> From an externals perspective (maybe being detached here is a benefit)
> it might be better to have something like "ProxyUseOriginalURL" that

Yeah, much better name. Will use *URI which is somewhat closer to the
RFC's URI-path nomenclature.

> has the following caveats:
> 1. it's partially decoded
> 2. it is required for whatever the servlet mapping option is (iiuc?)

Correct, I'm a bit reluctant to issue servlet mapping on decoded URIs..

> 3. it will affect the string used in comparisons/substitutions for
> , Rewrite, , (*CAUTION*)
>- Some details is needed here about the partial decoding, for
> example %2f or multiple leading slashes so "simple" context root
> configs can be written without too much paranoia
> 4. If there are risks of re-injecting URL's with [PT] or per-dir
> rewrites try to explain that it's a complex combination to worry
> about.

Possibly we are better off decoding r->uri in any case then, and not
change its current semantics.
As I said it won't affect the mod_proxy pre_trans case since its
translation already occurred and was saved in r->filename.
If we later want to let the user work with either the decoded or
encoded URI then we will add r->encoded_uri or alike, with some
opacity and helpers to keep it in sync with r->uri. But that would
affect module writers too, that's why I was thinking more of a "core"
directive than ProxyUseOriginalURI, or no directive at all with
trunk/2.next/3 material only.

>
> I would not worry too much about "existing configs" as this is all
> opt-in and it's probably a lost cause anyway.
>
> I would instead give people new built-in vars in the important places
> (rewrite, expr) to be able to match the partially decoded URI that the
> proxy will be working on so a savvy user can safely interoperate
> without monstrous regexes.

The hard part here (IMHO) is to keep encoded_uri and decoded_uri in
sync, depending on whichever is updated/accessed by a module or e.g. a
RewriteRule. It may be too much of an API/ABI change for the simple
proxy encoded URI case.

For now we can probably live with r->uri being always
normalized/decoded as before (and 

Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 5:04 PM jean-frederic clere  wrote:
>
> On 22/06/2020 16:12, Yann Ylavic wrote:
> > On Mon, Jun 22, 2020 at 2:44 PM Eric Covener  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
> 
>ProxyPass  ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet
> 
> 
>ProxyPass  ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet
> 
> 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:


   ProxyMappingDecoded Off
   ProxySet secret=%A1b2!@ mapping=servlet
   ProxyPass /docs


   ProxyMappingDecoded Off
   ProxySet secret=%A1b2!@ mapping=servlet
   ProxyPass /test



Regards;
Yann.


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread jean-frederic clere

On 22/06/2020 16:12, Yann Ylavic wrote:

On Mon, Jun 22, 2020 at 2:44 PM Eric Covener  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

  ProxyPass  ajp://localhost:8009/docs secret=%A1b2!@ mapping=servlet


  ProxyPass  ajp://localhost:8009/test secret=%A1b2!@ mapping=servlet

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


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread jean-frederic clere

On 22/06/2020 13:02, Yann Ylavic wrote:

On Mon, Jun 22, 2020 at 12:33 PM jean-frederic clere  wrote:


On 22/06/2020 12:23, Yann Ylavic wrote:

On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere  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


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 2:44 PM Eric Covener  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?
Index: server/request.c
===
--- server/request.c	(revision 1879079)
+++ server/request.c	(working copy)
@@ -160,6 +160,27 @@ AP_DECLARE(int) ap_some_authn_required(request_rec
 return rv;
 }
 
+static int walk_location_and_if(request_rec *r)
+{
+int access_status;
+
+if ((access_status = ap_location_walk(r))) {
+return access_status;
+}
+if ((access_status = ap_if_walk(r))) {
+return access_status;
+}
+
+/* Don't set per-dir loglevel if LogLevelOverride is set */
+if (!r->connection->log) {
+core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
+if (d->log)
+r->log = d->log;
+}
+
+return OK;
+}
+
 /* This is the master logic for processing requests.  Do NOT duplicate
  * this logic elsewhere, or the security model will be broken by future
  * API changes.  Each phase must be individually optimized to pick up
@@ -168,13 +189,13 @@ AP_DECLARE(int) ap_some_authn_required(request_rec
 AP_DECLARE(int) ap_process_request_internal(request_rec *r)
 {
 int file_req = (r->main && r->filename);
-int access_status;
-core_dir_config *d;
+int access_status = DECLINED;
 core_server_config *sconf =
 ap_get_core_module_config(r->server->module_config);
+const char *saved_uri = apr_pstrdup(r->pool, r->uri);
 unsigned int normalize_flags = 0;
 
-if (r->main) {
+if (file_req) {
 /* Lookup subrequests can have a relative path. */
 normalize_flags = AP_NORMALIZE_ALLOW_RELATIVE;
 }
@@ -188,22 +209,40 @@ AP_DECLARE(int) ap_process_request_internal(reques
normalize_flags |
AP_NORMALIZE_DECODE_UNRESERVED)) {
 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
-  "invalid URI path (%s)", r->uri);
+  "invalid URI path (%s)", saved_uri);
 return HTTP_BAD_REQUEST;
 }
 }
 
-/* Let pre_translate_name hooks work with non-decoded URIs,
- * and eventually apply their own transformations (return OK).
+/* All file subrequests are a huge pain... they cannot bubble through the
+ * next several steps.  Only file subrequests are allowed an empty uri,
+ * otherwise let translate_name kill the request.
  */
-access_status = ap_run_pre_translate_name(r);
-if (access_status != OK && access_status != DECLINED) {
-return access_status;
+if (!file_req) {
+r->per_dir_config = r->server->lookup_defaults;
+
+if ((access_status = walk_location_and_if(r))) {
+return access_status;
+}
+
+/* Let pre_translate_name hooks work with non-decoded URIs,
+ * and eventually apply their own transformations (return OK).
+ */
+access_status = ap_run_pre_translate_name(r);
+if (access_status != OK && access_status != DECLINED) {
+return access_status;
+}
 }
 
 /* Ignore URL unescaping for translated URIs already */
 if (access_status == DECLINED && r->parsed_uri.path) {
-d = ap_get_core_module_config(r->per_dir_config);
+core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
+
+/* May have changed since original */
+if (strcmp(r->uri, saved_uri) != 0) {
+saved_uri = apr_pstrdup(r->pool, r->uri);
+}
+
 if (d->allow_encoded_slashes) {
 access_status = ap_unescape_url_keep2f(r->parsed_uri.path,
d->decode_encoded_slashes);
@@ -217,7 +256,7 @@ AP_DECLARE(int) ap_process_request_internal(reques
 ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00026)
   "found %%2f (encoded '/') in URI "
   "(decoded='%s'), returning 404",
-  r->uri);
+  saved_uri);
 }
 }
 return access_status;
@@ -237,29 +276,17 @@ AP_DECLARE(int) ap_process_request_internal(reques
 /* We still didn't merged slashes yet, do it now. */
 ap_no2slash(r->parsed_uri.path);
 }
-}
 
-/* All file subrequests are a huge pain... they cannot bubble through the
- * next several steps.  Only file 

Re: mod_proxy_fcgi bug using CONTENT_LENGTH and Transfer-Encoding chunked

2020-06-22 Thread Oliver Dunk
Just wanted to resurface this one. I posted it last week, but there hasn’t been 
any activity yet. I’d still love someone to take a look if they have a minute.

Kind regards,
Oliver

> On 16 Jun 2020, at 22:57, Oliver Dunk  wrote:
> 
> Hi,
> 
> I wanted to politely ask if anyone could take a look at 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=57087 
> . This is an issue 
> where CONTENT_LENGTH isn’t forwarded correctly when using Transfer-Encoding 
> chunked. I feel uncomfortable bumping an issue in this mailing list, but it’s 
> been open since 2014, and I’m not sure where else to ask.
> 
> In the real world, this is causing issues for me when uploading to a 
> Nextcloud instance through macOS finder. Files are uploaded with a size of 0b 
> and macOS shows an error. There’s an issue here which shows some instances of 
> people running in to it: 
> https://github.com/nextcloud/nextcloud-snap/issues/365 
> .
> 
> For reference, the same issue was fixed in mod_fcgid a while back: 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=53332 
>  and 
> https://bz.apache.org/bugzilla/show_bug.cgi?id=50274 
> 
> 
> It would be great if anyone could take a look at this, and see if it is 
> feasible to fix in the near future. The only alternative I can think of is 
> moving my installation away from mod_fcgid.
> 
> As a final note, I did consider the user mailing list, but I get the 
> impression few developers look there. Hoping I made the right call.
> 
> Kind regards,
> Oliver



Re: svn commit: r1879076 - in /httpd/httpd/trunk: include/ap_mmn.h include/http_request.h server/request.c

2020-06-22 Thread Eric Covener
On Mon, Jun 22, 2020 at 6:32 AM  wrote:
>
> Author: ylavic
> Date: Mon Jun 22 10:32:15 2020
> New Revision: 1879076
>
> URL: http://svn.apache.org/viewvc?rev=1879076=rev
> Log:
> Add pre_translate_name hook running before URI-path decoding.
>
> This allows any module to work with un-decoded URI-path (besides
> unreserved characters) in r->uri, and eventually to avoid decoding by
> returning OK.
>
> The first candidate is mod_proxy (following commit) when
> ProxyMappingDecoded is disabled, such that the forwarded URI is
> equivalent to the original one.
>
>
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_request.h
> httpd/httpd/trunk/server/request.c
>
> Modified: httpd/httpd/trunk/include/ap_mmn.h
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=1879076=1879075=1879076=diff
> ==
> --- httpd/httpd/trunk/include/ap_mmn.h (original)
> +++ httpd/httpd/trunk/include/ap_mmn.h Mon Jun 22 10:32:15 2020
> @@ -633,6 +633,7 @@
>   * 20200420.2 (2.5.1-dev)  Add ap_proxy_worker_can_upgrade()
>   * 20200420.3 (2.5.1-dev)  Add ap_parse_strict_length()
>   * 20200420.4 (2.5.1-dev)  Add ap_normalize_path()
> + * 20200420.5 (2.5.1-dev)  Add pre_translate_name hook
>   */
>
>  #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
> @@ -640,7 +641,7 @@
>  #ifndef MODULE_MAGIC_NUMBER_MAJOR
>  #define MODULE_MAGIC_NUMBER_MAJOR 20200420
>  #endif
> -#define MODULE_MAGIC_NUMBER_MINOR 4/* 0...n */
> +#define MODULE_MAGIC_NUMBER_MINOR 5/* 0...n */
>
>  /**
>   * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
>
> Modified: httpd/httpd/trunk/include/http_request.h
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_request.h?rev=1879076=1879075=1879076=diff
> ==
> --- httpd/httpd/trunk/include/http_request.h (original)
> +++ httpd/httpd/trunk/include/http_request.h Mon Jun 22 10:32:15 2020
> @@ -364,6 +364,16 @@ AP_DECLARE_HOOK(int,create_request,(requ
>
>  /**
>   * This hook allow modules an opportunity to translate the URI into an
> + * actual filename, before URL decoding happens.
> + * rules will be followed.

^ stray 2nd-half of sentence copied from below.  Probably axe since I
don't think there is any "default" rules for pre_trans.
But maybe we mention the side effect here (IIUC?) that is currently
still being discussed?  i.e. it's not just a modules chance to
set r->filename before decoding but it will short-circuit decoding in
other non uri2file ways?


> + * @param r The current request
> + * @return OK, DECLINED, or HTTP_...
> + * @ingroup hooks
> + */
> +AP_DECLARE_HOOK(int,pre_translate_name,(request_rec *r))
> +
> +/**
> + * This hook allow modules an opportunity to translate the URI into an
>   * actual filename.  If no modules do anything special, the server's default
>   * rules will be followed.
>   * @param r The current request


Re: pre_translate_name hook vs location/directory walk (was: svn commit: r1879079)

2020-06-22 Thread Eric Covener
On Mon, Jun 22, 2020 at 8:28 AM Yann Ylavic  wrote:
>
> > Allow for URI-path pre_translate_name before (and/or instead of) decoding.
> >
> > Only if no hook takes "ownership" of the URI (returning OK), apply
> > percent decoding for the rest of request handling. Otherwise r->uri remains
> > encoded meaning that further location/directory/file/if/.. sections (walks)
> > should that into account.
>
> This change allows a module to avoid URI decoding in
> pre_translate_name, such that r->uri remains encoded for the whole
> request handling.
>
> For instance (same series), mod_proxy will use that hook when
> "ProxyMappingDecoded off" is configured, and return OK if a ProxyPass
> matches. The decoding was already disabled (before this commit) for
> the forward-proxy case (ProxyRequests on) since r->proxyreq was set at
> an early stage, but now it "works" for the reverse-proxy case too.
>
> This means that when "ProxyMappingDecoded off" is configured (the
> default is still "on", though), for instance a  a fixup RewriteRule or a non-early RequestHeader rule will have to be
> expressed with the encoded form since reserved characters will not be
> decoded as before (note that unreserved characters will always be
> decoded anyway).
>
> I don't know how much it can break existing configurations, and wonder
> if we should have a core directive that still controls whether r->uri
> should be decoded, regardless of ProxyMappingDecoded or any further
> module pre_translate_name hook with its own directives.
>
> For mod_proxy this is not really an issue to have r->uri modified
> after the (pre_)translate phase, because the handler will later work
> on r->filename regardless (the "proxy:..." set by proxy_detect or
> proxy_trans), so maybe we should simply always decode r->uri after the
> pre_translate hooks have run. Though I find it easier in a proxy
> context to match a URI-path (LocationMatch, RewriteRule..) in its
> encoded form, and not worry about original vs decoded separators. That
> may be just my preference, but a new core directive looks sane to me..
>
> Thoughts?

I have not followed too closely so take with a grain of salt / for the
sake of argument:

>From an externals perspective (maybe being detached here is a benefit)
it might be better to have something like "ProxyUseOriginalURL" that
has the following caveats:
1. it's partially decoded
2. it is required for whatever the servlet mapping option is (iiuc?)
3. it will affect the string used in comparisons/substitutions for
, Rewrite, , (*CAUTION*)
   - Some details is needed here about the partial decoding, for
example %2f or multiple leading slashes so "simple" context root
configs can be written without too much paranoia
4. If there are risks of re-injecting URL's with [PT] or per-dir
rewrites try to explain that it's a complex combination to worry
about.

I would not worry too much about "existing configs" as this is all
opt-in and it's probably a lost cause anyway.

I would instead give people new built-in vars in the important places
(rewrite, expr) to be able to match the partially decoded URI that the
proxy will be working on so a savvy user can safely interoperate
without monstrous regexes.


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread Eric Covener
> 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.


Re: pre_translate_name hook vs location/directory walk (was: svn commit: r1879079)

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 2:28 PM Yann Ylavic  wrote:
>
> Also, since we are at it, I'm on the fence about running the
> pre_translate hooks before quick handlers (thus before
> ap_process_request_internal() too), good or bad idea?

Sorry, I meant running the *normalization* (including decoding
unreserved characters) before the quick handlers.


pre_translate_name hook vs location/directory walk (was: svn commit: r1879079)

2020-06-22 Thread Yann Ylavic
> Allow for URI-path pre_translate_name before (and/or instead of) decoding.
>
> Only if no hook takes "ownership" of the URI (returning OK), apply
> percent decoding for the rest of request handling. Otherwise r->uri remains
> encoded meaning that further location/directory/file/if/.. sections (walks)
> should that into account.

This change allows a module to avoid URI decoding in
pre_translate_name, such that r->uri remains encoded for the whole
request handling.

For instance (same series), mod_proxy will use that hook when
"ProxyMappingDecoded off" is configured, and return OK if a ProxyPass
matches. The decoding was already disabled (before this commit) for
the forward-proxy case (ProxyRequests on) since r->proxyreq was set at
an early stage, but now it "works" for the reverse-proxy case too.

This means that when "ProxyMappingDecoded off" is configured (the
default is still "on", though), for instance a uri
should be decoded, regardless of ProxyMappingDecoded or any further
module pre_translate_name hook with its own directives.

For mod_proxy this is not really an issue to have r->uri modified
after the (pre_)translate phase, because the handler will later work
on r->filename regardless (the "proxy:..." set by proxy_detect or
proxy_trans), so maybe we should simply always decode r->uri after the
pre_translate hooks have run. Though I find it easier in a proxy
context to match a URI-path (LocationMatch, RewriteRule..) in its
encoded form, and not worry about original vs decoded separators. That
may be just my preference, but a new core directive looks sane to me..

Thoughts?


Also, since we are at it, I'm on the fence about running the
pre_translate hooks before quick handlers (thus before
ap_process_request_internal() too), good or bad idea?


Regards;
Yann.


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 12:33 PM jean-frederic clere  wrote:
>
> On 22/06/2020 12:23, Yann Ylavic wrote:
> > On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere  
> > 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.


Passed: apache/httpd#855 (trunk - d884808)

2020-06-22 Thread Travis CI
Build Update for apache/httpd
-

Build: #855
Status: Passed

Duration: 12 mins and 40 secs
Commit: d884808 (trunk)
Author: Yann Ylavic
Message: Allow for proxy servlet mapping at pre_translate_name stage.

Provide alias_match_servlet(), the servlet counterpart of alias_match(),
which maps the request URI-path to the ProxyPass alias ignoring path
parameters, while still forwarding them (above the alias).

This is needed to proxy servlet URIs for application handled by Tomcat,
which can then make use of the path/segments parameters.

Github: closes #128



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1879080 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/6b3b91a616b4...d8848084e32d

View the full build log and details: 
https://travis-ci.org/github/apache/httpd/builds/700831193?utm_medium=notification_source=email

--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://travis-ci.org/account/preferences/unsubscribe?repository=69847_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread jean-frederic clere

On 22/06/2020 12:23, Yann Ylavic wrote:

On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere  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


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 12:13 PM jean-frederic clere  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.


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread jean-frederic clere

On 22/06/2020 11:50, Yann Ylavic wrote:

On Mon, Jun 22, 2020 at 11:20 AM jean-frederic clere  wrote:


On 19/06/2020 12:02, Yann Ylavic wrote:

On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere  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


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread Yann Ylavic
On Mon, Jun 22, 2020 at 11:20 AM jean-frederic clere  wrote:
>
> On 19/06/2020 12:02, Yann Ylavic wrote:
> > On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere  
> > 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.


Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

2020-06-22 Thread jean-frederic clere

On 19/06/2020 12:02, Yann Ylavic wrote:

On Thu, Jun 18, 2020 at 6:37 PM jean-frederic clere  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