On Sat, Jun 24, 2017 at 12:49 AM, <gsm...@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?