You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by gs...@apache.org on 2017/06/24 05:49:46 UTC
svn commit: r1799731 - in /httpd/httpd/trunk: CHANGES server/request.c
Author: gsmith
Date: Sat Jun 24 05:49:45 2017
New Revision: 1799731
URL: http://svn.apache.org/viewvc?rev=1799731&view=rev
Log:
Send a 404 response like other OSs do instead of 403 on Windows when
a path segment or file requested uses a reserved word so Windows
cannot be fingerprinted. PR55887
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/server/request.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1799731&r1=1799730&r2=1799731&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Jun 24 05:49:45 2017
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) core: Send a 404 response like other OSs do instead of 403 on Windows
+ when a path segment or file requested uses a reserved word so Windows
+ cannot be fingerprinted. PR55887 [Gregg Smith]
+
*) mod_rewrite: Add 'RewriteOptions LongURLOptimization' to free memory
from each set of unmatched rewrite conditions.
[Eric Covener]
Modified: httpd/httpd/trunk/server/request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1799731&r1=1799730&r2=1799731&view=diff
==============================================================================
--- httpd/httpd/trunk/server/request.c (original)
+++ httpd/httpd/trunk/server/request.c Sat Jun 24 05:49:45 2017
@@ -1211,10 +1211,25 @@ AP_DECLARE(int) ap_directory_walk(reques
break;
}
else if (thisinfo.filetype != APR_DIR) {
+#ifdef _WIN32
+ ap_regex_t *preg;
+#endif
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
"Forbidden: %s doesn't point to "
"a file or directory",
r->filename);
+#ifdef _WIN32
+ /* Windows has a number of reserved words that cannot be used
+ * as a file or directory name so thisinfo.filetype will
+ * always be != APR_DIR. Don't allow us be fingerprinted with
+ * a 403 and instead send a 404 like other OSs would. PR55887
+ */
+ preg = ap_pregcomp(r->pool,
+ "/(aux|con|com[1-9]|lpt[1-9]|nul|prn)"
+ "($|/|.)", AP_REG_EXTENDED | AP_REG_ICASE);
+ if (ap_regexec(preg, r->uri, 0, NULL, 0) == 0)
+ return r->status = HTTP_NOT_FOUND;
+#endif
return r->status = HTTP_FORBIDDEN;
}
Re: svn commit: r1799731 - in /httpd/httpd/trunk: CHANGES
server/request.c
Posted by Gregg Smith <gl...@gknw.net>.
Sorry Bill for the off-list mail now twice but either the list or
thunderbird's behavior with this list has changed. I'll assume the
former since I do not see the reply-to dev@ header anymore.
On 6/24/2017 10:02 AM, William A Rowe Jr wrote:
> On Sat, Jun 24, 2017 at 12:49 AM, <gs...@apache.org> wrote:
>> Author: gsmith
>> Date: Sat Jun 24 05:49:45 2017
>> New Revision: 1799731
>>
>> URL: http://svn.apache.org/viewvc?rev=1799731&view=rev
>> Log:
>> Send a 404 response like other OSs do instead of 403 on Windows when
>> a path segment or file requested uses a reserved word so Windows
>> cannot be fingerprinted. PR55887
>
> How does this solve fingerprinting?
Simple Bill, does this 403?
http://httpd.apache.org/lpt1/
No.
Does it on Windows?
https://is.kaput.gq/lpt1
Yes
Does this?
http://httpd.apache.org/foo/bar/lpt1/test/
No.
This?
https://is.kaput.gq/foo/bar/lpt1/test/
Yes
Granted /foo/bar has to exist or it will never make it that far and 404's.
While my server header may only say Apache, not hard to figure out what
it's running on due to the different behavior.
It's always going to 403 on Windows
https://is.kaput.gq/lpt1.php
Which is what lead me to 00038 in the first place.
>
> This patch was ill-conceived... -1 as explained below;
>
>> +#ifdef _WIN32
>> + /* Windows has a number of reserved words that cannot be used
>> + * as a file or directory name so thisinfo.filetype will
>> + * always be != APR_DIR. Don't allow us be fingerprinted with
>> + * a 403 and instead send a 404 like other OSs would. PR55887
>> + */
>> + preg = ap_pregcomp(r->pool,
>> + "/(aux|con|com[1-9]|lpt[1-9]|nul|prn)"
>> + "($|/|.)", AP_REG_EXTENDED | AP_REG_ICASE);
>> + if (ap_regexec(preg, r->uri, 0, NULL, 0) == 0)
>> + return r->status = HTTP_NOT_FOUND;
>> +#endif
>
> You should be aware that this code path can be hit a number of times
> per request, often hundreds for an autoindex listing, etc. You should
> see a notable rise in cpu.
I am after sitting on the beach thinking about what Yann meant in his reply.
>
> As suggested this could be a compile-once and recycle pattern.
>
> But why a regex against the URI?!? That's horrible, you are reparsing
> all those leading bytes we already parsed. Part of the URI may have
> been alias-mapped away from one resource name to another (or in
> the htaccess case, mapped into a badly named path.) r->filename
> is the cumulative name of the *FILE* we are inspecting, not *URI*.
Mainly because I had a problem with r->filename and with r->uri I
didn't. However, I know the problem I had was not with r->filename but I
forgot to run back to it.
> Worse still, because seg_name is the actual path component we are
> now looking at, e.g. from /path/to/lpt1/foo - if we are at the third elt
> that string becomes the normalized form of lpt1. There was no reason
> to be reinspecting the earlier path elts throughout the segment walk!
>
> But this could all be identified by the APR_CHR ->filetype, no? Sure
> beats an unnecessarily long string pattern match.
Mainly because I did not know about it and I can't say whether or not
I'd have known what "a character device" meant though I hope I would have.
>
> While we are at it, why even forking WIN32? If you want to prevent
> APR_CHR files on Windows (or netware or os2 or...) from being
> identified, why wouldn't you simply change the behavior across all
> architectures with a new test case ahead of the != APR_DIR check...
>
> else if (thisinfo.filetype == APR_CHR) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
> "Forbidden: %s points to a char stream path",
> r->filename);
> return r->status = HTTP_NOT_FOUND;
> }
Uh yes, but as shown up front the 404 is what we'd get on Unix and
forcing it 404 is what I was doing while improperly.
> else if (thisinfo.filetype != APR_DIR) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
> "Forbidden: %s doesn't point to "
> "a file or directory",
> r->filename);
> return r->status = HTTP_FORBIDDEN;
> }
>
> As I asked upfront, why is it believed that this solves the win32
> issue?
Because I tested and got a 404 instead of a 403 with the numerous
possibilities I tried.
It's trivial to check for case folding and that folding will
> occur in a manner unique to the Windows implementation of the
> NTFS filesystem. You would need to modify the following code
> to force case mismatches to a 404 result to try to convince anyone
> that the server is running on unix, non-samba;
>
> if ((thisinfo.valid & APR_FINFO_NAME)
> && strcmp(seg_name, thisinfo.name)) {
> /* TODO: provide users an option that an internal/external
> * redirect is required here? We need to walk the URI and
> * filename in tandem to properly correlate these.
> */
> strcpy(seg_name, thisinfo.name);
> filename_len = strlen(r->filename);
> }
>
> More to the point, the actual TCP traffic itself is subject to all sorts
> of inferences if it can be observed (and that this isn't a proxied box
> behind the firewall.)
>
> Let's please back out that patch entirely and start again by asking
> the question, do we want to treat CHR file paths as not found rather
> than forbidden, and does anyone see an opportunity for httpd's
> core or third party modules to proceed to try to work around that
> file/path leading to potential security blunders? E.g. mod_speling?
> If we agree that APR_CHR can be 'anonymized' then a generic
> and fast fix is in order.
>
I think I've proved it's going to 403 where it won't on Unix. Not I too
much but many Windows users are extremely paranoid and will not like the
fact that this can be so easily figured out with a simple request while
some evildoer is doing reconnaissance. That's probably why the PR is
there in the first place.
I'll revert by the end of the day UTC -8.
Thanks.
Re: svn commit: r1799731 - in /httpd/httpd/trunk: CHANGES server/request.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Hi Gregg,
sending publicly while you have a negotiation with your email client :)
On Mon, Jun 26, 2017 at 3:40 PM, Gregg Smith <gl...@gknw.net> wrote:
>
> On 6/24/2017 10:02 AM, William A Rowe Jr wrote:
>>
>> On Sat, Jun 24, 2017 at 12:49 AM, <gs...@apache.org> wrote:
>>>
>>> Author: gsmith
>>> Date: Sat Jun 24 05:49:45 2017
>>> New Revision: 1799731
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1799731&view=rev
>>> Log:
>>> Send a 404 response like other OSs do instead of 403 on Windows when
>>> a path segment or file requested uses a reserved word so Windows
>>> cannot be fingerprinted. PR55887
>>
>> How does this solve fingerprinting?
>
> Simple Bill, does this 403?
> http://httpd.apache.org/foo/bar/lpt1/test/
> No.
>
> This?
> https://is.kaput.gq/foo/bar/lpt1/test/
> Yes
I didn't ask whether there is a difference. I asked if this patch has
comprehensively avoided fingerprinting. The answer is no for a long
host of reasons, from 8.3 pathnames to mixed case to... and on and
on. It is not worth breaking code I fixed almost 20 years ago to maybe
pretend to accomplish.
>> You should be aware that this code path can be hit a number of times
>> per request, often hundreds for an autoindex listing, etc. You should
>> see a notable rise in cpu.
So this is not altogether correct, I overreacted on this one point. As it
turns out, the file path element has to already fail a REG or DIR check
before we start exercising your code path.
Still, it's inappropriate and incorrect.
>> But why a regex against the URI?!? That's horrible, you are reparsing
>> all those leading bytes we already parsed. Part of the URI may have
>> been alias-mapped away from one resource name to another (or in
>> the htaccess case, mapped into a badly named path.) r->filename
>> is the cumulative name of the *FILE* we are inspecting, not *URI*.
>
> Mainly because I had a problem with r->filename and with r->uri I didn't.
> However, I know the problem I had was not with r->filename but I forgot to
> run back to it.
No worries, but not ready for trunk.
>> Worse still, because seg_name is the actual path component we are
>> now looking at, e.g. from /path/to/lpt1/foo - if we are at the third elt
>> that string becomes the normalized form of lpt1. There was no reason
>> to be reinspecting the earlier path elts throughout the segment walk!
>>
>> But this could all be identified by the APR_CHR ->filetype, no? Sure
>> beats an unnecessarily long string pattern match.
>
> Mainly because I did not know about it and I can't say whether or not I'd
> have known what "a character device" meant though I hope I would have.
Again, no worries, not ready for trunk.
>> While we are at it, why even forking WIN32? If you want to prevent
>> APR_CHR files on Windows (or netware or os2 or...) from being
>> identified, why wouldn't you simply change the behavior across all
>> architectures with a new test case ahead of the != APR_DIR check...
>>
>> else if (thisinfo.filetype == APR_CHR) {
>> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
>> "Forbidden: %s points to a char stream
>> path",
>> r->filename);
>> return r->status = HTTP_NOT_FOUND;
>> }
>
> Uh yes, but as shown up front the 404 is what we'd get on Unix and forcing
> it 404 is what I was doing while improperly.
No no no. Unix has CHR devices!!! You can mount them whereever, by
convention they all live under /dev/. But that doesn't mean they cannot
be found elsewhere. Unix httpd will 403 on these.
We should consider if it's worthwhile "hiding" CHR references under some
response 404, but ***ONLY*** if we can assert this is a terminal 404, not
a recoverable 404. We don't have such a construct yet in httpd, AFAIK.
> Because I tested and got a 404 instead of a 403 with the numerous
> possibilities I tried.
Sure. And this is a bug how?
>> Let's please back out that patch entirely and start again by asking
>> the question, do we want to treat CHR file paths as not found rather
>> than forbidden, and does anyone see an opportunity for httpd's
>> core or third party modules to proceed to try to work around that
>> file/path leading to potential security blunders? E.g. mod_speling?
>> If we agree that APR_CHR can be 'anonymized' then a generic
>> and fast fix is in order.
>
> I think I've proved it's going to 403 where it won't on Unix. Not I too much
> but many Windows users are extremely paranoid and will not like the fact
> that this can be so easily figured out with a simple request while some
> evildoer is doing reconnaissance. That's probably why the PR is there in the
> first place.
Sure, Windows will behave differently. Users will complain. They should
change to a strict filesystem (much like the OS/X choice between HFS+
and UFS) if they want to avoid this. Whether Windows offers a case
insensitive option is beyond me, I stopped caring, but know that there
is a toggle for denying 8.3 notation in an NTFS mount.
> I'll revert by the end of the day UTC -8.
No worries, I took care of reverting once I worked out a scenario where
mod_speling would potentially reveal not-so-well 'hidden' files if that
combination was deployed.
The report itself is invalid, I updated the mod_sec github ticket (which
already seems to have a workaround for this) and our own bugzilla
report as invalid.
Cheers,
Bill
Re: svn commit: r1799731 - in /httpd/httpd/trunk: CHANGES server/request.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Sat, Jun 24, 2017 at 12:49 AM, <gs...@apache.org> wrote:
> Author: gsmith
> Date: Sat Jun 24 05:49:45 2017
> New Revision: 1799731
>
> URL: http://svn.apache.org/viewvc?rev=1799731&view=rev
> Log:
> Send a 404 response like other OSs do instead of 403 on Windows when
> a path segment or file requested uses a reserved word so Windows
> cannot be fingerprinted. PR55887
How does this solve fingerprinting?
This patch was ill-conceived... -1 as explained below;
> +#ifdef _WIN32
> + /* Windows has a number of reserved words that cannot be used
> + * as a file or directory name so thisinfo.filetype will
> + * always be != APR_DIR. Don't allow us be fingerprinted with
> + * a 403 and instead send a 404 like other OSs would. PR55887
> + */
> + preg = ap_pregcomp(r->pool,
> + "/(aux|con|com[1-9]|lpt[1-9]|nul|prn)"
> + "($|/|.)", AP_REG_EXTENDED | AP_REG_ICASE);
> + if (ap_regexec(preg, r->uri, 0, NULL, 0) == 0)
> + return r->status = HTTP_NOT_FOUND;
> +#endif
You should be aware that this code path can be hit a number of times
per request, often hundreds for an autoindex listing, etc. You should
see a notable rise in cpu.
As suggested this could be a compile-once and recycle pattern.
But why a regex against the URI?!? That's horrible, you are reparsing
all those leading bytes we already parsed. Part of the URI may have
been alias-mapped away from one resource name to another (or in
the htaccess case, mapped into a badly named path.) r->filename
is the cumulative name of the *FILE* we are inspecting, not *URI*.
Worse still, because seg_name is the actual path component we are
now looking at, e.g. from /path/to/lpt1/foo - if we are at the third elt
that string becomes the normalized form of lpt1. There was no reason
to be reinspecting the earlier path elts throughout the segment walk!
But this could all be identified by the APR_CHR ->filetype, no? Sure
beats an unnecessarily long string pattern match.
While we are at it, why even forking WIN32? If you want to prevent
APR_CHR files on Windows (or netware or os2 or...) from being
identified, why wouldn't you simply change the behavior across all
architectures with a new test case ahead of the != APR_DIR check...
else if (thisinfo.filetype == APR_CHR) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
"Forbidden: %s points to a char stream path",
r->filename);
return r->status = HTTP_NOT_FOUND;
}
else if (thisinfo.filetype != APR_DIR) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
"Forbidden: %s doesn't point to "
"a file or directory",
r->filename);
return r->status = HTTP_FORBIDDEN;
}
As I asked upfront, why is it believed that this solves the win32
issue? It's trivial to check for case folding and that folding will
occur in a manner unique to the Windows implementation of the
NTFS filesystem. You would need to modify the following code
to force case mismatches to a 404 result to try to convince anyone
that the server is running on unix, non-samba;
if ((thisinfo.valid & APR_FINFO_NAME)
&& strcmp(seg_name, thisinfo.name)) {
/* TODO: provide users an option that an internal/external
* redirect is required here? We need to walk the URI and
* filename in tandem to properly correlate these.
*/
strcpy(seg_name, thisinfo.name);
filename_len = strlen(r->filename);
}
More to the point, the actual TCP traffic itself is subject to all sorts
of inferences if it can be observed (and that this isn't a proxied box
behind the firewall.)
Let's please back out that patch entirely and start again by asking
the question, do we want to treat CHR file paths as not found rather
than forbidden, and does anyone see an opportunity for httpd's
core or third party modules to proceed to try to work around that
file/path leading to potential security blunders? E.g. mod_speling?
If we agree that APR_CHR can be 'anonymized' then a generic
and fast fix is in order.
p.s. Next time we try to deoptimize the code this badly, let's add a
config option so the admin chooses to spend such extra cycles?
Re: svn commit: r1799731 - in /httpd/httpd/trunk: CHANGES server/request.c
Posted by Yann Ylavic <yl...@gmail.com>.
Hi Gregg,
On Sat, Jun 24, 2017 at 7:49 AM, <gs...@apache.org> wrote:
> Author: gsmith
> Date: Sat Jun 24 05:49:45 2017
> New Revision: 1799731
>
> URL: http://svn.apache.org/viewvc?rev=1799731&view=rev
> Log:
> Send a 404 response like other OSs do instead of 403 on Windows when
> a path segment or file requested uses a reserved word so Windows
> cannot be fingerprinted. PR55887
>
> Modified:
> httpd/httpd/trunk/server/request.c
>
> Modified: httpd/httpd/trunk/server/request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1799731&r1=1799730&r2=1799731&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/request.c (original)
> +++ httpd/httpd/trunk/server/request.c Sat Jun 24 05:49:45 2017
> @@ -1211,10 +1211,25 @@ AP_DECLARE(int) ap_directory_walk(reques
> break;
> }
> else if (thisinfo.filetype != APR_DIR) {
> +#ifdef _WIN32
> + ap_regex_t *preg;
> +#endif
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00038)
> "Forbidden: %s doesn't point to "
> "a file or directory",
> r->filename);
> +#ifdef _WIN32
> + /* Windows has a number of reserved words that cannot be used
> + * as a file or directory name so thisinfo.filetype will
> + * always be != APR_DIR. Don't allow us be fingerprinted with
> + * a 403 and instead send a 404 like other OSs would. PR55887
> + */
> + preg = ap_pregcomp(r->pool,
> + "/(aux|con|com[1-9]|lpt[1-9]|nul|prn)"
> + "($|/|.)", AP_REG_EXTENDED | AP_REG_ICASE);
Couldn't we compile this regexp once at load time (e.g. a static preg
at pre/post_config)?
> + if (ap_regexec(preg, r->uri, 0, NULL, 0) == 0)
> + return r->status = HTTP_NOT_FOUND;
> +#endif
> return r->status = HTTP_FORBIDDEN;
> }
Regards,
Yann.