httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William A Rowe Jr <wr...@rowe-clan.net>
Subject Re: svn commit: r1799731 - in /httpd/httpd/trunk: CHANGES server/request.c
Date Mon, 26 Jun 2017 21:44:44 GMT
Hi Gregg,

sending publicly while you have a negotiation with your email client :)

On Mon, Jun 26, 2017 at 3:40 PM, Gregg Smith <gls@gknw.net> wrote:
>
> On 6/24/2017 10:02 AM, William A Rowe Jr wrote:
>>
>> On Sat, Jun 24, 2017 at 12:49 AM,  <gsmith@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

Mime
View raw message