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

[Yann Ylavic]

On Thu, Jun 25, 2020 at 9:24 AM Ruediger Pluem <rpl...@apache.org> wrote:
>
> 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

Yes, very true.

But if we want mapping=servlet to take "ownership" of r->uri, we can
do something like the attached patch.
The result is that if mod_proxy matches a servlet URI-path, r->uri
gets servlet normalized (without the path parameters) for the rest of
the request handling.
So your above example gets rewritten to "/admin/nastything" and
"<Location /admin>" now applies.

Would that be better?

Regards;
Yann

Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c	(revision 1879145)
+++ modules/proxy/mod_proxy.c	(working copy)
@@ -574,10 +574,11 @@ static int alias_match(const char *uri, const char
  * Inspired by mod_jk's jk_servlet_normalize().
  */
 static int alias_match_servlet(apr_pool_t *p,
-                               const char *uri,
+                               const char **urip,
                                const char *alias)
 {
     char *map;
+    const char *uri = *urip;
     apr_array_header_t *stack;
     int map_pos, uri_pos, alias_pos, first_pos;
     int alias_depth = 0, depth;
@@ -759,6 +760,8 @@ static int alias_match_servlet(apr_pool_t *p,
     if (alias[alias_pos - 1] != '/' && uri[uri_pos - 1] == '/') {
         uri_pos--;
     }
+
+    *urip = map;
     return uri_pos;
 }
 
@@ -869,7 +872,7 @@ PROXY_DECLARE(int) ap_proxy_trans_match(request_re
     ap_regmatch_t regm[AP_MAX_REG_MATCH];
     ap_regmatch_t reg1[AP_MAX_REG_MATCH];
     char *found = NULL;
-    int mismatch = 0;
+    int mismatch = 0, servlet = 0;
     unsigned int nocanon = ent->flags & PROXYPASS_NOCANON;
     const char *use_uri = nocanon ? r->unparsed_uri : r->uri;
 
@@ -933,8 +936,9 @@ PROXY_DECLARE(int) ap_proxy_trans_match(request_re
     }
     else {
         if ((ent->flags & PROXYPASS_MAP_SERVLET) == PROXYPASS_MAP_SERVLET) {
+            len = alias_match_servlet(r->pool, (const char **)&r->uri, fake);
             nocanon = 0; /* ignored since servlet's normalization applies */
-            len = alias_match_servlet(r->pool, r->uri, fake);
+            servlet = 1;
         }
         else {
             len = alias_match(r->uri, fake);
@@ -969,7 +973,7 @@ PROXY_DECLARE(int) ap_proxy_trans_match(request_re
          */
         int rc = proxy_run_check_trans(r, found + 6);
         if (rc != OK && rc != DECLINED) {
-            return DONE;
+            return HTTP_CONTINUE;
         }
 
         r->filename = found;
@@ -987,10 +991,10 @@ PROXY_DECLARE(int) ap_proxy_trans_match(request_re
                       "URI path '%s' matches proxy handler '%s'", r->uri,
                       found);
 
-        return OK;
+        return servlet ? DONE : OK;
     }
 
-    return DONE;
+    return HTTP_CONTINUE;
 }
 
 static int proxy_trans(request_rec *r, int pre_trans)
@@ -1042,7 +1046,7 @@ static int proxy_trans(request_rec *r, int pre_tra
         enc = (dconf->alias->flags & PROXYPASS_MAP_ENCODED) != 0;
         if (!(pre_trans ^ enc)) {
             int rv = ap_proxy_trans_match(r, dconf->alias, dconf);
-            if (DONE != rv) {
+            if (rv != HTTP_CONTINUE) {
                 return rv;
             }
         }
@@ -1054,7 +1058,7 @@ static int proxy_trans(request_rec *r, int pre_tra
         enc = (ent->flags & PROXYPASS_MAP_ENCODED) != 0;
         if (!(pre_trans ^ enc)) {
             int rv = ap_proxy_trans_match(r, ent, dconf);
-            if (DONE != rv) {
+            if (rv != HTTP_CONTINUE) {
                 return rv;
             }
         }