[repost] [PATCH] mod_negotiation and authorization

[Francis Daly <de...@daoine.org>]

Hi there,

This is a repost of an earlier mail.  The patch has been edited to
rename the directive (from "expose" to "reveal") and to rename the
#define'd macros to avoid namespace invasion.

This patch introduces a config directive which changes the behaviour of
MultiViews.  It potentially exposes names of authentication-requiring
URLs to unauthenticated users.  I've called the directive
"MultiviewsRevealSecretURL" to make sure that it isn't unintentionally
enabled.

My specific intention was to be able to require authentication for
the file "thing.cgi" while advertising the url "thing", without
requiring authentication for the directory in which "thing.cgi"
resided.  MultiViews doesn't work the way I want it to in directories
in which only some files require authentication.  I know that there are
sound security reasons for that behaviour.  I was looking for a way to
selectively override the behaviour.

Incidentally, the non-code-changing workaround I have been using is
renaming "thing.cgi" to "thing" and using "SetHandler cgi-script"
in a "<Files thing>" block.  I know I'm abusing MultiViews for
"extension-hiding" rather than "content negotiation", but I suspect I'm
not the only person doing that.

===========

Docs for the MultiviewsRevealSecretURL directive:

set On or Off on a per-directory basis.  Default Off overall.  If unset,
inherit the value from the parent directory.  AllowOverride AuthConfig to
use in .htaccess.

A MultiViews request which would otherwise fail 404 not found can
instead fail 401 authorization required, if there is a filename which
might suit the request when the correct credentials are provided.  The
subsequent authenticated request can succeed 200 or 300, or can fail
406 or 404, or could persistently fail 401.  The last two failures might
appear surprising.

It's only useful in directories where only some files require
authentication; it will reveal to unauthenticated clients that the url
they requested may match one requiring authentication.  However, it also
allows MultiViews to work more like I expect it to in such directories.

============

One thing I'm not certain about is that I've got the best
option-handling code.  If the patch is considered worthwhile, I'd suggest
that someone with more experience there take a look to see if there
isn't a better way to achieve the aims in the first paragraph of the
docs.

The directive name itself wouldn't say no to a change, either.

Built against the version of mod_negotiation released with httpd-2.0.36.

	f
-- 
Francis Daly        deva@daoine.org

--- modules/mappers/mod_negotiation.c	Fri Mar 29 08:17:23 2002
+++ modules/mappers/mod_negotiation.c.new	Thu May  9 09:05:11 2002
@@ -88,10 +88,17 @@
  */
 
 typedef struct {
+    int reveal_secret_url;
     int forcelangpriority;
     apr_array_header_t *language_priority;
 } neg_dir_config;
 
+/* reveal_secret_url flags
+ */
+#define RSU_UNDEF    2    /* this means "no explicit config" */
+#define RSU_ON       1    /* "config on" */
+#define RSU_OFF      0    /* "config off" */
+
 /* forcelangpriority flags 
  */
 #define FLP_UNDEF    0    /* Same as FLP_DEFAULT, but base overrides */
@@ -107,6 +114,7 @@
 {
     neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
 
+    new->reveal_secret_url = RSU_UNDEF;
     new->forcelangpriority = FLP_UNDEF;
     new->language_priority = NULL;
     return new;
@@ -119,6 +127,9 @@
     neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
 
     /* give priority to the config in the subdirectory */
+    new->reveal_secret_url = (add->reveal_secret_url != RSU_UNDEF)
+				? add->reveal_secret_url 
+                                : base->reveal_secret_url;
     new->forcelangpriority = (add->forcelangpriority != FLP_UNDEF)
 				? add->forcelangpriority 
 				: base->forcelangpriority;
@@ -128,6 +139,22 @@
     return new;
 }
 
+static const char *reveal_secret_url(cmd_parms *cmd, void *n_, int arg)
+{
+    neg_dir_config *n = n_;
+    const char *err = ap_check_cmd_context(cmd, NOT_IN_FILES);
+
+    if (err != NULL) {
+        return err;
+    }
+    n->reveal_secret_url = arg == RSU_OFF ? RSU_OFF : RSU_ON;
+/* that is functionally equivalent to
+    n->reveal_secret_url = arg != 0;
+   for the RSU_* values #defined'd above. Clarity vs efficiency?
+*/
+    return NULL;
+}
+
 static const char *set_language_priority(cmd_parms *cmd, void *n_,
 					 const char *lang)
 {
@@ -188,6 +215,8 @@
 {
     AP_INIT_FLAG("CacheNegotiatedDocs", cache_negotiated_docs, NULL, RSRC_CONF, 
                  "Either 'on' or 'off' (default)"),
+    AP_INIT_FLAG("MultiviewsRevealSecretURL", reveal_secret_url, NULL, RSRC_CONF|OR_AUTHCFG, 
+                 "Either 'on' or 'off' (default)"),
     AP_INIT_ITERATE("LanguagePriority", set_language_priority, NULL, OR_FILEINFO, 
                     "space-delimited list of MIME language abbreviations"),
     AP_INIT_ITERATE("ForceLanguagePriority", set_force_priority, NULL, OR_FILEINFO,
@@ -1045,6 +1074,7 @@
     struct accept_rec accept_info;
     void *new_var;
     int anymatch = 0;
+    int secretmatch = 0;
 
     clean_var_rec(&mime_info);
 
@@ -1110,6 +1140,13 @@
         if (sub_req->finfo.filetype != APR_REG)
             continue;
 
+        /* Note if it failed UNAUTHORIZED. We may want to return this
+         * status, eventually
+         */
+        if (sub_req->status == HTTP_UNAUTHORIZED) {
+            secretmatch = 1;
+        }
+
         /* If it has a handler, we'll pretend it's a CGI script,
          * since that's a good indication of the sort of thing it
          * might be doing.
@@ -1232,6 +1269,9 @@
      * request must die.
      */
     if (anymatch && !neg->avail_vars->nelts) {
+        if (secretmatch && neg->conf->reveal_secret_url == RSU_ON) {
+            return HTTP_UNAUTHORIZED;
+        }
 	ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
 		      "Negotiation: discovered file(s) matching request: %s"
                       " (None could be negotiated).",