]> git.ipfire.org Git - thirdparty/apache/httpd.git/commitdiff
mod_proxy canon-tion improvements/fixes.
authorJim Jagielski <jim@apache.org>
Tue, 20 Nov 2007 15:11:39 +0000 (15:11 +0000)
committerJim Jagielski <jim@apache.org>
Tue, 20 Nov 2007 15:11:39 +0000 (15:11 +0000)
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@596712 13f79535-47bb-0310-9956-ffa450edef68

CHANGES
STATUS
docs/manual/mod/mod_proxy.xml
include/ap_mmn.h
modules/proxy/mod_proxy.c
modules/proxy/mod_proxy.h
modules/proxy/mod_proxy_http.c

diff --git a/CHANGES b/CHANGES
index 69dc5db6c4077c3045c8b371eaccba3bb5fa788a..6860af1b1a019abb473f76c746c95902aef3806e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,8 +1,13 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.2.7
 
+  *) mod_proxy: Canonicalisation improvements. Add "nocanon" keyword to
+     ProxyPass, to suppress URI-canonicalisation in a reverse proxy. Also,
+     don't escape/unescape forward-proxied URLs.
+     PR 41798, 42592 [Nick Kew, Ruediger Pluem, Roy Fielding, Jim Jagielski]
+
   *) mod_ssl: Prevent memory corruption of version string.
-     PR 43865 43334 [William Rowe, Joe Orton]
+     PR 43865, 43334 [William Rowe, Joe Orton]
 
   *) core: Avoid some unexpected connection closes by telling the client
      that the connection is not persistent if the MPM process handling
diff --git a/STATUS b/STATUS
index 8ff0805396931635f3f227f32d8540c274e08430..8475adab66b4bd50e6d53d54c09a0dd772982730 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -79,36 +79,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_proxy: add "nocanon" option to ProxyPass, to suppress
-     canonicalisation of reverse-proxied URLs.
-     PR 41798 PR 42592
-     trunk:
-        http://svn.apache.org/viewvc?view=rev&revision=588791
-     2.2.x:
-        mod_proxy.c patch at http://people.apache.org/~niq/41798.patch
-        mod_proxy_http.c patch: trunk works after applying PR#42592 patch
-        Other files: trunk patch works.
-     2.2.x composite patch: this together with PR42592 patch
-        http://people.apache.org/~niq/41798+42592.patch
-     +1: niq
-     -1: jim: patch should be self-contained. If a mix and match of
-              files, then please create a distinct 2.2 patch.
-     niq: we NEVER do that.  When did you incorporate the CHANGES
-          update in a separate 2.2 patch?  The complexity here is that
-          mod_proxy_http.c relies on 42592, and a separate fix to
-          that would invalidate the (very simple) patch to 42592.
-    jim: The issue is that the above "use trunk but use these external
-         patch" makes the backport hard to apply and test. Instead
-         of being able to just merge in r588791, you must just use
-         those that apply "cleanly" and handle mod_proxy.c as a
-         special case. This seems ripe for a specific 2.2 patch. Yes,
-         CHANGES always conflicts, but it is a documentation question,
-         not a code one. 
-     rpluem says: Please add a minor bump for the changes to mod_proxy.h.
-                  Then I am +1.
-     niq says: done
-     jim: +1 (needs doccos as well, 'natch)
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
index 215912b24d53f2275b651813a7c9ef619364724a..cacc994ac799513053a0bd91e2f7ab37960b6854 100644 (file)
@@ -548,7 +548,7 @@ expressions</description>
 <directivesynopsis>
 <name>ProxyPass</name>
 <description>Maps remote servers into the local server URL-space</description>
-<syntax>ProxyPass [<var>path</var>] !|<var>url</var> [<var>key=value</var> <var>key=value</var> ...]]</syntax>
+<syntax>ProxyPass [<var>path</var>] !|<var>url</var> [<var>key=value</var> <var>key=value</var> ...]] [nocanon]</syntax>
 <contextlist><context>server config</context><context>virtual host</context>
 <context>directory</context>
 </contextlist>
@@ -814,7 +814,14 @@ expressions</description>
       &lt;/Proxy&gt;
     </example>
 
-    
+    <p>Normally, mod_proxy will canonicalise ProxyPassed URLs.
+    But this may be incompatible with some backends, particularly those
+    that make use of <var>PATH_INFO</var>.  The optional <var>nocanon</var>
+    keyword suppresses this, and passes the URL path "raw" to the
+    backend.  Note that may affect the security of your backend, as it
+    removes the normal limited protection against URL-based attacks
+    provided by the proxy.</p>
+
     <p>When used inside a <directive type="section" module="core"
     >Location</directive> section, the first argument is omitted and the local
     directory is obtained from the <directive type="section" module="core"
index 80187a2e4aeed3bd5b3ca0843b5572cb7343e0af..a37c44a2d6fe90dcc5f1fbcabcdbee52cf7fdd03 100644 (file)
  * 20051115.5 (2.2.5)  Added ap_mpm_safe_kill() (minor)
  * 20051115.6 (2.2.7)  Added retry_set to proxy_worker (minor)
  * 20051115.7 (2.2.7)  Added conn_rec::clogging_input_filters (minor)
+ * 20051115.8 (2.2.7)  Added flags to proxy_alias (minor)
  *
  */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20051115
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 7                     /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 8                     /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index 7a1aa14e92236dd975e6e00f7c024a02ad6ffd77..870bb63a7a03da0dbbacf3f80c776aced357208f 100644 (file)
@@ -220,12 +220,12 @@ static const char *set_worker_param(apr_pool_t *p,
                 else
                     worker->status &= ~PROXY_WORKER_HOT_STANDBY;
             }
-           else if (*v == 'I' || *v == 'i') {
-               if (mode)
-                   worker->status |= PROXY_WORKER_IGNORE_ERRORS;
-               else
-                   worker->status &= ~PROXY_WORKER_IGNORE_ERRORS;
-           }
+            else if (*v == 'I' || *v == 'i') {
+                if (mode)
+                    worker->status |= PROXY_WORKER_IGNORE_ERRORS;
+                else
+                    worker->status &= ~PROXY_WORKER_IGNORE_ERRORS;
+            }
             else {
                 return "Unknown status parameter option";
             }
@@ -440,7 +440,9 @@ static int proxy_trans(request_rec *r)
     int i, len;
     struct proxy_alias *ent = (struct proxy_alias *) conf->aliases->elts;
     ap_regmatch_t regm[AP_MAX_REG_MATCH];
+    ap_regmatch_t reg1[AP_MAX_REG_MATCH];
     char *found = NULL;
+    int mismatch = 0;
 
     if (r->proxyreq) {
         /* someone has already set up the proxy, it was possibly ourselves
@@ -455,13 +457,22 @@ static int proxy_trans(request_rec *r)
      */
 
     for (i = 0; i < conf->aliases->nelts; i++) {
+        unsigned int nocanon = ent[i].flags & PROXYPASS_NOCANON;
+        const char *use_uri = nocanon ? r->unparsed_uri : r->uri;
         if (ent[i].regex) {
             if (!ap_regexec(ent[i].regex, r->uri, AP_MAX_REG_MATCH, regm, 0)) {
                 if ((ent[i].real[0] == '!') && (ent[i].real[1] == '\0')) {
                     return DECLINED;
                 }
-                found = ap_pregsub(r->pool, ent[i].real, r->uri, AP_MAX_REG_MATCH,
-                                   regm);
+                /* test that we haven't reduced the URI */
+                if (nocanon && ap_regexec(ent[i].regex, r->unparsed_uri,
+                                          AP_MAX_REG_MATCH, reg1, 0)) {
+                    mismatch = 1;
+                    use_uri = r->uri;
+                }
+                found = ap_pregsub(r->pool, ent[i].real, use_uri,
+                                   AP_MAX_REG_MATCH,
+                                   (use_uri == r->uri) ? regm : reg1);
                 /* Note: The strcmp() below catches cases where there
                  * was no regex substitution. This is so cases like:
                  *
@@ -479,8 +490,8 @@ static int proxy_trans(request_rec *r)
                     found = apr_pstrcat(r->pool, "proxy:", found, NULL);
                 }
                 else {
-                    found = apr_pstrcat(r->pool, "proxy:", ent[i].real, r->uri,
-                                        NULL);
+                    found = apr_pstrcat(r->pool, "proxy:", ent[i].real,
+                                        use_uri, NULL);
                 }
             }
         }
@@ -491,15 +502,31 @@ static int proxy_trans(request_rec *r)
                 if ((ent[i].real[0] == '!') && (ent[i].real[1] == '\0')) {
                     return DECLINED;
                 }
-
+                if (nocanon
+                    && len != alias_match(r->unparsed_uri, ent[i].fake)) {
+                    mismatch = 1;
+                    use_uri = r->uri;
+                }
                 found = apr_pstrcat(r->pool, "proxy:", ent[i].real,
-                                    r->uri + len, NULL);
+                                    use_uri + len, NULL);
             }
         }
+        if (mismatch) {
+            /* We made a reducing transformation, so we can't safely use
+             * unparsed_uri.  Safe fallback is to ignore nocanon.
+             */
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+                          "Unescaped URL path matched ProxyPass; ignoring unsafe nocanon");
+        }
+
         if (found) {
             r->filename = found;
             r->handler = "proxy-server";
             r->proxyreq = PROXYREQ_REVERSE;
+            if (nocanon && !mismatch) {
+                /* mod_proxy_http needs to be told.  Different module. */
+                apr_table_setn(r->notes, "proxy-nocanon", "1");
+            }
             return OK;
         }
     }
@@ -1090,6 +1117,7 @@ static const char *
     const apr_table_entry_t *elts;
     int i;
     int use_regex = is_regex;
+    unsigned int flags = 0;
 
     while (*arg) {
         word = ap_getword_conf(cmd->pool, &arg);
@@ -1103,8 +1131,12 @@ static const char *
             }
             f = word;
         }
-        else if (!r)
+        else if (!r) {
             r = word;
+        }
+        else if (!strcasecmp(word,"nocanon")) {
+            flags |= PROXYPASS_NOCANON;
+        }
         else {
             char *val = strchr(word, '=');
             if (!val) {
@@ -1135,6 +1167,7 @@ static const char *
     new = apr_array_push(conf->aliases);
     new->fake = apr_pstrdup(cmd->pool, f);
     new->real = apr_pstrdup(cmd->pool, r);
+    new->flags = flags;
     if (use_regex) {
         new->regex = ap_pregcomp(cmd->pool, f, AP_REG_EXTENDED);
         if (new->regex == NULL)
index 0f25caaff981295a9fef2710da385838b3ec5700..c0a29c9fcfb0376b16fe0b31ff90ae388e807d93 100644 (file)
@@ -109,10 +109,12 @@ struct proxy_remote {
     int use_regex;          /* simple boolean. True if we have a regex pattern */
 };
 
+#define PROXYPASS_NOCANON 0x01
 struct proxy_alias {
     const char  *real;
     const char  *fake;
     ap_regex_t  *regex;
+    unsigned int flags;
 };
 
 struct dirconn_entry {
index 7e6493b41cb525beff23b2aa635cf18c23a20b95..774852eed404236da0fca8c12fff7d377262b103 100644 (file)
@@ -81,7 +81,26 @@ static int proxy_http_canon(request_rec *r, char *url)
         search = r->args;
 
     /* process path */
-    path = ap_proxy_canonenc(r->pool, url, strlen(url), enc_path, 0, r->proxyreq);
+    /* In a reverse proxy, our URL has been processed, so canonicalise
+     * unless proxy-nocanon is set to say it's raw
+     * In a forward proxy, we have and MUST NOT MANGLE the original.
+     */
+    switch (r->proxyreq) {
+    default: /* wtf are we doing here? */
+    case PROXYREQ_REVERSE:
+        if (apr_table_get(r->notes, "proxy-nocanon")) {
+            path = url;   /* this is the raw path */
+        }
+        else {
+            path = ap_proxy_canonenc(r->pool, url, strlen(url),
+                                     enc_path, 0, r->proxyreq);
+        }
+        break;
+    case PROXYREQ_PROXY:
+        path = url;
+        break;
+    }
+
     if (path == NULL)
         return HTTP_BAD_REQUEST;