You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <Ke...@Golux.Com> on 2003/01/13 12:46:19 UTC

Re: workaround for encoded slashes (%2f)

okey, here's anpther take on the %2f thing.  as a reminder, we
currently reject any reques that includes an encoded slash in the
pat with a 404.  this breaks environments which need to use %2f in
the path info.  the issue has to do, in part, with decoding the
%2fs into slashes, and possibly having to re-encode them again.

i've been fighting with saving %2f state and restoring it for a long
time, but to no definitely acceptable conclusion.  but last week a
possible alternative came to mind:  if the AllowEncodedSlashes directive
is set to 'on', unencode everything *except* %2f.  then let it go
through the normal processing.  if %2f is part of the path, the
request will fail with a 404 anyway when the document isn't found;
if the %2f is strictly in the path-info, it won't intefere with
document location and will be available to the resource as expected.

i have a patch for this against 2.1 that i'd like to send, but i
won't bother if people are going to veto all over it without even
looking at it.


Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
William A. Rowe, Jr. wrote:
>  
> I should be able to blow some holes in the patch, but I can't do that
> right now while spending so many hours vetting our coming 2.0.44
> release, and I consider it more than a little gratuitous that you presume
> lazy consensus on a patch that I'd vetoed in theory.

completely different approach, completely different patch, therefore
no standing pat.  and, by the way, no 'vetoes in theory,' either.
you examine this on its own merits; don't try to grandfather it.
if you can prove a problem with it, it'll get reverted; but i'm not
going to sit on a working solution that *i* can't find any problems
with just because *you* don't have time to test it.  there are other
people here.

greg, will you try/test this patch?


Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
William A. Rowe, Jr. wrote:
> 
>  I consider it more than a little gratuitous that you presume
> lazy consensus on a patch that I'd vetoed in theory.  I then agreed

oh, and by the way?  i said i'd presume lazy consensus in a couple of days,
not right now.  so i'm allowing plenty of time for discussion, and not
jsut blasting it in there.

and yes, i'm going in a straight line because this is a *real* problem
*right now*, not a theoretical one that *might* happen later.  it
should have been fixed years ago -- there are prs about it going 'way
back -- but 'we've always done it that way' doesn't hold water as a
reason for delaying a 'good enough' fix 'til perfection is achieved.


Re: workaround for encoded slashes (%2f)

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 10:10 AM 1/16/2003, Rodent of Unusual Size wrote:
>Rodent of Unusual Size wrote:
>>okey, here is the patch.  i have been unable to detect any
>>security flaws in my testing.  please apply and test this
>>as fixing the existing issue in a 'good enough' way until
>>a rework/redesign of the filesystem intertwingling is addressed.
>
>no negative remarks, so i'm going to assume lazy consensus in
>a couple of days and commit it into all three branches.

You've already received a number of comments, and you already know
I'm strongly opposed in principle to just tossing this security and trusting
lazy 3rd party module authors to perform this testing themselves.

For example, we've seen a number of vulnerabilities in Tomcat HTTP
connector that weren't susceptible due to the added protections of
http.

I agree that this is a conundrum to be solved.  We only disagree on 
the solution.  You are going for the straight line, while I'm arguing that
we need a stronger framework before we proceed.  Such a framework
has been suggested and that discussion is certainly not finished.

I should be able to blow some holes in the patch, but I can't do that
right now while spending so many hours vetting our coming 2.0.44
release, and I consider it more than a little gratuitous that you presume
lazy consensus on a patch that I'd vetoed in theory.  I then agreed
to give your patch the benefit of the doubt and prove up my objections
or shut up.  I need time to do so, and the veto against potentially
introducing security holes into 3rd party modules stands till I can
perform that review.  With good fortune, within a week of the release
of 2.0.44.

Bill



Re: workaround for encoded slashes (%2f)

Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Stein wrote:

> Product features and changes *are* subject to majority vote, however, which
> is why Ken is operating under a (lazy) consensus model.


for 2.1 only I hope...  2.0 is supposed to be R-T-C...



Re: workaround for encoded slashes (%2f)

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Jan 16, 2003 at 10:57:06PM +0100, Sander Striker wrote:
>...
> Ken, could you please bring the interpretation differences that
> you and OtherBill have to the list, so that we can reach a consensus
> on that?

I'd say make it OtherBill's job. Ken has a solution, albeit hacky, to a
problem. OtherBill doesn't like it, so let him describe an alternative
solution which can be done.

[ OtherBill doesn't have to totally solve it, but at least explain enough of
  an alternative such that somebody can implement the thing; I don't see a
  way to do it "Right" without breaking half the planet ]


I don't like the hack either, but it does solve a problem. And we cannot and
should not use vetoes to knock down product design issues. Technical
problems, certainly, but not "I don't like that feature, so I'm gonna veto
the thing."

Product features and changes *are* subject to majority vote, however, which
is why Ken is operating under a (lazy) consensus model.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

RE: workaround for encoded slashes (%2f)

Posted by Sander Striker <st...@apache.org>.
> From: Rodent of Unusual Size [mailto:Ken.Coar@Golux.Com]
> Sent: Thursday, January 16, 2003 9:05 PM

> Rodent of Unusual Size wrote:
> > 
> > no negative remarks, so i'm going to assume lazy consensus in
> > a couple of days and commit it into all three branches.
> 
> not for 2.0.44, though.

To get it into 2.0, commit it to 2.1 first and put an item in the
2.0 STATUS file about the disire to backport it.  I'd rather see
a patch like this, where at least two parties aren't in agreement,
reviewed and positively received by three people than to asume
lazy consensus.

Ken, could you please bring the interpretation differences that
you and OtherBill have to the list, so that we can reach a consensus
on that?

Thanks,

Sander

Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Rodent of Unusual Size wrote:
> 
> no negative remarks, so i'm going to assume lazy consensus in
> a couple of days and commit it into all three branches.

not for 2.0.44, though.
-- 
#ken	P-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist      http://Apache-Server.Com/

"Millennium hand and shrimp!"


Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Rodent of Unusual Size wrote:
> okey, here is the patch.  i have been unable to detect any
> security flaws in my testing.  please apply and test this
> as fixing the existing issue in a 'good enough' way until
> a rework/redesign of the filesystem intertwingling is addressed.

no negative remarks, so i'm going to assume lazy consensus in
a couple of days and commit it into all three branches.
-- 
#ken	P-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist      http://Apache-Server.Com/

"Millennium hand and shrimp!"


Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
okey, here is the patch.  i have been unable to detect any
security flaws in my testing.  please apply and test this
as fixing the existing issue in a 'good enough' way until
a rework/redesign of the filesystem intertwingling is addressed.

in addition to 2.1 (where it might become obviated by a
better solution), i would like to put this into 2.0 and
possibly 1.3.  and, if it doesn't open any security exposures,
i don't see why that shouldn't happen.

Index: include/ap_mmn.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/ap_mmn.h,v
retrieving revision 1.52
diff -u -r1.52 ap_mmn.h
--- include/ap_mmn.h	3 Sep 2002 23:39:43 -0000	1.52
+++ include/ap_mmn.h	14 Jan 2003 16:49:53 -0000
@@ -111,6 +111,7 @@
   * 20020625 (2.0.40-dev) Changed conn_rec->keepalive to an enumeration
   * 20020628 (2.0.40-dev) Added filter_init to filter registration functions
   * 20020903 (2.0.41-dev) APR's error constants changed
+ * 20020903.1 (2.0.44-dev) allow_encoded_slashes added to core_dir_config
   */

  #define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */
@@ -118,7 +119,7 @@
  #ifndef MODULE_MAGIC_NUMBER_MAJOR
  #define MODULE_MAGIC_NUMBER_MAJOR 20020903
  #endif
-#define MODULE_MAGIC_NUMBER_MINOR 0                     /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1                     /* 0...n */

  /**
   * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Index: include/http_core.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_core.h,v
retrieving revision 1.71
diff -u -r1.71 http_core.h
--- include/http_core.h	15 Dec 2002 20:05:23 -0000	1.71
+++ include/http_core.h	14 Jan 2003 16:49:53 -0000
@@ -539,7 +539,8 @@
  #define ENABLE_SENDFILE_ON     (1)
  #define ENABLE_SENDFILE_UNSET  (2)
      unsigned int enable_sendfile : 2;  /* files in this dir can be mmap'ed */
-
+    unsigned int allow_encoded_slashes : 1; /* URLs may contain %2f w/o being
+                                             * pitched indiscriminately */
  } core_dir_config;

  /* Per-server core configuration */
Index: include/httpd.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/httpd.h,v
retrieving revision 1.191
diff -u -r1.191 httpd.h
--- include/httpd.h	8 Nov 2002 17:19:10 -0000	1.191
+++ include/httpd.h	14 Jan 2003 16:49:53 -0000
@@ -1314,10 +1314,16 @@

  /**
   * Unescape a URL
- * @param url The url to unescapte
+ * @param url The url to unescape
   * @return 0 on success, non-zero otherwise
   */
  AP_DECLARE(int) ap_unescape_url(char *url);
+/**
+ * Unescape a URL, but leaving %2f (slashes) escaped
+ * @param url The url to unescape
+ * @return 0 on success, non-zero otherwise
+ */
+AP_DECLARE(int) ap_unescape_url_keep2f(char *url);
  /**
   * Convert all double slashes to single slashes
   * @param name The string to convert
Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.227
diff -u -r1.227 core.c
--- server/core.c	14 Jan 2003 03:01:51 -0000	1.227
+++ server/core.c	14 Jan 2003 16:49:54 -0000
@@ -182,6 +182,7 @@

      conf->enable_mmap = ENABLE_MMAP_UNSET;
      conf->enable_sendfile = ENABLE_SENDFILE_UNSET;
+    conf->allow_encoded_slashes = 0;

      return (void *)conf;
  }
@@ -452,6 +453,8 @@
          conf->enable_sendfile = new->enable_sendfile;
      }

+    conf->allow_encoded_slashes = new->allow_encoded_slashes;
+
      return (void*)conf;
  }

@@ -2087,6 +2090,19 @@
      return NULL;
  }

+static const char *set_allow2f(cmd_parms *cmd, void *d_, int arg)
+{
+    core_dir_config *d = d_;
+    const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
+
+    if (err != NULL) {
+        return err;
+    }
+
+    d->allow_encoded_slashes = arg != 0;
+    return NULL;
+}
+
  static const char *set_hostname_lookups(cmd_parms *cmd, void *d_,
                                          const char *arg)
  {
@@ -3080,6 +3096,8 @@
  AP_INIT_ITERATE2("AddOutputFilterByType", add_ct_output_filters,
         (void *)APR_OFFSETOF(core_dir_config, ct_output_filters), OR_FILEINFO,
       "output filter name followed by one or more content-types"),
+AP_INIT_FLAG("AllowEncodedSlashes", set_allow2f, NULL, RSRC_CONF,
+             "Allow URLs containing '/' encoded as '%2F'"),

  /*
   * These are default configuration directives that mpms can/should
Index: server/request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/request.c,v
retrieving revision 1.122
diff -u -r1.122 request.c
--- server/request.c	12 Dec 2002 07:05:54 -0000	1.122
+++ server/request.c	14 Jan 2003 16:49:54 -0000
@@ -147,13 +147,22 @@

      /* Ignore embedded %2F's in path for proxy requests */
      if (!r->proxyreq && r->parsed_uri.path) {
-        access_status = ap_unescape_url(r->parsed_uri.path);
+        core_dir_config *d;
+        d = ap_get_module_config(r->per_dir_config, &core_module);
+        if (d->allow_encoded_slashes) {
+            access_status = ap_unescape_url_keep2f(r->parsed_uri.path);
+        }
+        else {
+            access_status = ap_unescape_url(r->parsed_uri.path);
+        }
          if (access_status) {
              if (access_status == HTTP_NOT_FOUND) {
-                ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
-                              "found %%2f (encoded '/') in URI "
-                              "(decoded='%s'), returning 404",
-                              r->parsed_uri.path);
+                if (! d->allow_encoded_slashes) {
+                    ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+                                  "found %%2f (encoded '/') in URI "
+                                  "(decoded='%s'), returning 404",
+                                  r->parsed_uri.path);
+                }
              }
              return access_status;
          }
Index: server/util.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/util.c,v
retrieving revision 1.134
diff -u -r1.134 util.c
--- server/util.c	8 Dec 2002 21:05:57 -0000	1.134
+++ server/util.c	14 Jan 2003 16:49:54 -0000
@@ -1595,6 +1595,57 @@
          return OK;
  }

+AP_DECLARE(int) ap_unescape_url_keep2f(char *url)
+{
+    register int badesc, badpath;
+    char *x, *y;
+
+    badesc = 0;
+    badpath = 0;
+    /* Initial scan for first '%'. Don't bother writing values before
+     * seeing a '%' */
+    y = strchr(url, '%');
+    if (y == NULL) {
+        return OK;
+    }
+    for (x = y; *y; ++x, ++y) {
+        if (*y != '%') {
+            *x = *y;
+        }
+        else {
+            if (!apr_isxdigit(*(y + 1)) || !apr_isxdigit(*(y + 2))) {
+                badesc = 1;
+                *x = '%';
+            }
+            else {
+                char decoded;
+                decoded = x2c(y + 1);
+                if (IS_SLASH(decoded)) {
+                    *x++ = *y++;
+                    *x = *y;
+                }
+                else {
+                    *x = decoded;
+                    y += 2;
+                    if (decoded == '\0') {
+                        badpath = 1;
+                    }
+                }
+            }
+        }
+    }
+    *x = '\0';
+    if (badesc) {
+        return HTTP_BAD_REQUEST;
+    }
+    else if (badpath) {
+        return HTTP_NOT_FOUND;
+    }
+    else {
+        return OK;
+    }
+}
+
  AP_DECLARE(char *) ap_construct_server(apr_pool_t *p, const char *hostname,
                                         apr_port_t port, const request_rec *r)
  {
-- 
#ken	P-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist      http://Apache-Server.Com/

"Millennium hand and shrimp!"



Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
William A. Rowe, Jr. wrote:
> 
>>If Apache is going to decode the URI, then it should first split
 >>the URI into path segments (an array) and then perform the decoding.
 >>Thus, you can make http://host/path/foo%2fbar/baz into ["path",
 >>"foo/bar", "baz"]. You can then encode each piece, and join them
 >>together with "/".
> 
> This is, in fact, the only way to handle this scenario.

um, no, i disagree, for reasons i just mentioned in reply
to greg.  you cannot reliably re-assemble the path-info
because you cannot tell what transformations have been
performed upon it.  i have spent a lot of time on trying
to figure out exactly what is suggested above, and have
come up against that brick wall each time, in various
forms.

 > The decoded %2f should then be treated as invalid within
 > any apache file resource (dir_walk/file_walk)

of which my proposal for the immediate term takes advantage.
of course, if someone creates a file 'a%2fb' it should be
accessible, and that would happen as well.

 > Since some folks are itching to cut apart the filesystem
 > from the request_rec, now seems like a golden opportunity
 > to play out how this new logic would work.

cool.  in the meantime, there is a need to allow %2f in
path-info, which my patch addresses.  i will post the patch
directly; please examine it and actually *test* it before
dismissing it out of hand as not a perfect solution.  'good
enough' should be sufficient until perfection arrives.
-- 
#ken	P-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist      http://Apache-Server.Com/

"Millennium hand and shrimp!"


Re: workaround for encoded slashes (%2f)

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 04:37 PM 1/13/2003, Greg Stein wrote:
>On Mon, Jan 13, 2003 at 06:46:19AM -0500, Rodent of Unusual Size wrote:
>> okey, here's anpther take on the %2f thing.  as a reminder, we
>> currently reject any reques that includes an encoded slash in the
>> pat with a 404.  this breaks environments which need to use %2f in
>> the path info.  the issue has to do, in part, with decoding the
>> %2fs into slashes, and possibly having to re-encode them again.
>> 
>> i've been fighting with saving %2f state and restoring it for a long
>> time, but to no definitely acceptable conclusion.  but last week a
>> possible alternative came to mind:  if the AllowEncodedSlashes directive
>> is set to 'on', unencode everything *except* %2f.  then let it go
>> through the normal processing.  if %2f is part of the path, the
>> request will fail with a 404 anyway when the document isn't found;
>> if the %2f is strictly in the path-info, it won't intefere with
>> document location and will be available to the resource as expected.
>> 
>> i have a patch for this against 2.1 that i'd like to send, but i
>> won't bother if people are going to veto all over it without even
>> looking at it.
>
>That sounds rather hacky. IMO, Apache should not do any URI decoding.
>Instead, Apache should figure out who is going to handle the request (i.e.
>map the identified resource to a handler), then let the handler do whatever
>processing is appropriate on the URI.

Agreed; %2F is not '/' and cannot be handled as '/'.

>Today, Apache unconditionally decodes the URI (see the problems that Mike
>Pilato just posted about today), which really horks systems that need to
>pass that URI further along (e.g. proxy-like code).

Well, there's nothing wrong with Apache today, we have two different
values, unparsed_uri and uri.

Granted, the model could be much cleaner ...

>If Apache is going to decode the URI, then it should first split the URI
>into path segments (an array) and then perform the decoding. Thus, you can
>make http://host/path/foo%2fbar/baz into ["path", "foo/bar", "baz"]. You can
>then encode each piece, and join them together with "/".

This is, in fact, the only way to handle this scenario.  The introduction of
%2F means that we have two URI entities (%2F and '/') each of which has
a distinct meaning.  And in general I like the proposal.

%2F as a segment separator cannot not be permitted.

The decoded %2f should then be treated as invalid within any apache file
resource (dir_walk/file_walk) but could then be permitted by some other
module.  The risk that those 'other modules' aren't prepared for it had better
be dealt with by introducing this into the 2.1-dev branch soon, allowing our
3rd party module authors time to prepare for the consequences before 2.2
is released to the world.

Since some folks are itching to cut apart the filesystem from the request_rec,
now seems like a golden opportunity to play out how this new logic would work.

Bill



Re: workaround for encoded slashes (%2f)

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Greg Stein wrote:
> 
> That sounds rather hacky. IMO, Apache should not do any URI decoding.

yes, well, maybe.  and then again, maybe not.  i'm working within the
current framework, not trying to friggin' redesign something that we've
been doing for more than five years.

 > Instead, Apache should figure out who is going to handle
 > the request (i.e. map the identified resource to a handler),
 > then let the handler do whatever processing is appropriate on
 > the URI.

in a more perfect world, i agree.

 > If Apache is going to decode the URI, then it should first
 > split the URI into path segments (an array) and then perform
 > the decoding.

um, trust me: we *do not* want to go there without a massive
rework.  i've spent a lot of time on that.  it gets very, very
messy because different phases may do different things which
make it impossible to deterministically re-assemble the path.
consider mod_rewrite for a moment.

 > But again: URI munging should be left to handlers rather than
 > the core. (altho, of course, the core can provide utilities
 > to those handlers)

that would be wonderful.  unfortunately, we do the decoding
because of the legacy involvement with the filesystem
(directory and file walk).  i agree that a more perfect solution
is to *not* do any decoding and let the various/appropriate
handlers figure out wth they need to do with the raw uri.
but that's not possible until the filesystem access is a true
peer and not a preferential case.
-- 
#ken	P-)}

Ken Coar, Sanagendamgagwedweinini  http://Golux.Com/coar/
Author, developer, opinionist      http://Apache-Server.Com/

"Millennium hand and shrimp!"


Re: workaround for encoded slashes (%2f)

Posted by Glenn <gs...@gluelogic.com>.
On Mon, Jan 13, 2003 at 02:37:03PM -0800, Greg Stein wrote:
> IMO, Apache should not do any URI decoding.
> Instead, Apache should figure out who is going to handle the request (i.e.
> map the identified resource to a handler), then let the handler do whatever
> processing is appropriate on the URI.
[...]
> But again: URI munging should be left to handlers rather than the core.
> (altho, of course, the core can provide utilities to those handlers)

+1
At the very least, the original URI should be preserved and exposed to the
handlers, in addition to the decoded parts.

> Today, Apache unconditionally decodes the URI (see the problems that Mike
> Pilato just posted about today), which really horks systems that need to
> pass that URI further along (e.g. proxy-like code).

Exactly.
URI decoding discards information that could be valuable to later handlers.

Another example besides proxy-related code is in the case of using the
Action directive, which can also add a layer of encoding on top of any
existing encoding.

-Glenn

Re: workaround for encoded slashes (%2f)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jan 13, 2003 at 06:46:19AM -0500, Rodent of Unusual Size wrote:
> okey, here's anpther take on the %2f thing.  as a reminder, we
> currently reject any reques that includes an encoded slash in the
> pat with a 404.  this breaks environments which need to use %2f in
> the path info.  the issue has to do, in part, with decoding the
> %2fs into slashes, and possibly having to re-encode them again.
> 
> i've been fighting with saving %2f state and restoring it for a long
> time, but to no definitely acceptable conclusion.  but last week a
> possible alternative came to mind:  if the AllowEncodedSlashes directive
> is set to 'on', unencode everything *except* %2f.  then let it go
> through the normal processing.  if %2f is part of the path, the
> request will fail with a 404 anyway when the document isn't found;
> if the %2f is strictly in the path-info, it won't intefere with
> document location and will be available to the resource as expected.
> 
> i have a patch for this against 2.1 that i'd like to send, but i
> won't bother if people are going to veto all over it without even
> looking at it.

That sounds rather hacky. IMO, Apache should not do any URI decoding.
Instead, Apache should figure out who is going to handle the request (i.e.
map the identified resource to a handler), then let the handler do whatever
processing is appropriate on the URI.

Today, Apache unconditionally decodes the URI (see the problems that Mike
Pilato just posted about today), which really horks systems that need to
pass that URI further along (e.g. proxy-like code).

If Apache is going to decode the URI, then it should first split the URI
into path segments (an array) and then perform the decoding. Thus, you can
make http://host/path/foo%2fbar/baz into ["path", "foo/bar", "baz"]. You can
then encode each piece, and join them together with "/".

But again: URI munging should be left to handlers rather than the core.
(altho, of course, the core can provide utilities to those handlers)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/