Disable automatic creation of MellonPostDirectory.

Now that the POST replay functionality has been disabled by default,
we can force the administrator to create this directory manually. This
saves us from worrying about temp file/directory vulnerabilities.

git-svn-id: https://modmellon.googlecode.com/svn/trunk@178 a716ebb1-153a-0410-b759-cfb97c6a1b53

4 files changed, 27 insertions, 80 deletions

diff --git a/README b/README
index 12b2825..40bccf3 100644
--- a/README
+++ b/README
@@ -105,11 +105,10 @@ MellonCacheSize 100
 MellonLockFile "/var/run/mod_auth_mellon.lock"
 
 # MellonPostDirectory is the full path of a directory where POST requests
-# are saved during authentication. This directory must be owned by the
-# Apache user and be mode 700. We will attempt to create it if it does not
-# exist.
-# Default: MellonPostDirectory "/var/tmp/mellonpost"
-MellonPostDirectory "/var/tmp/mellonpost"
+# are saved during authentication. This directory must writeable by the
+# Apache user. It should not be writeable (or readable) by other users.
+# Default: None
+# Example: MellonPostDirectory "/var/cache/mod_auth_mellon_postdata"
 
 # MellonPostTTL is the delay in seconds before a saved POST request can
 # be flushed.
@@ -470,6 +469,9 @@ MellonPostCount 100
         # authentication is completed, and then replayed. If this option isn't enabled,
         # the requests will be turned into normal GET requests after authentication.
         #
+        # Note that if this option is enabled, you must also
+        # set the MellonPostDirectory option in the server configuration.
+        #
         # The default is that it is "Off".
         # MellonPostReplay Off
 
diff --git a/auth_mellon_config.c b/auth_mellon_config.c
index 6a1eb2d..4a938b5 100644
--- a/auth_mellon_config.c
+++ b/auth_mellon_config.c
@@ -57,11 +57,6 @@ static const int default_dump_saml_response = 0;
  */
 static const char *default_login_path = "/";
 
-/* This is the directory for storing saved POST sessions
- * the MellonPostDirectory configuration directive if you change this.
- */
-static const char *post_dir = "/var/tmp/mellonpost";
-
 /* saved POST session time to live
  * the MellonPostTTL configuration directive if you change this.
  */
@@ -1489,7 +1484,7 @@ void *auth_mellon_server_config(apr_pool_t *p, server_rec *s)
 
     mod->cache_size = 100;  /* ought to be enough for everybody */
     mod->lock_file  = "/var/run/mod_auth_mellon.lock";
-    mod->post_dir   = post_dir;
+    mod->post_dir   = NULL;
     mod->post_ttl   = post_ttl;
     mod->post_count = post_count;
     mod->post_size  = post_size;
diff --git a/auth_mellon_handler.c b/auth_mellon_handler.c
index cdc4c28..f21f9bb 100644
--- a/auth_mellon_handler.c
+++ b/auth_mellon_handler.c
@@ -2307,8 +2307,15 @@ static int am_handle_repost(request_rec *r)
     }
 
     mod_cfg = am_get_mod_cfg(r->server);
+
+    if (!mod_cfg->post_dir) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "Repost query without MellonPostDirectory.");
+        return HTTP_NOT_FOUND;
+    }
+
     query = r->parsed_uri.query;
-    
+
     enctype = am_extract_query_parameter(r->pool, query, "enctype");
     if (enctype == NULL) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
diff --git a/auth_mellon_util.c b/auth_mellon_util.c
index a3407a6..da6a43e 100644
--- a/auth_mellon_util.c
+++ b/auth_mellon_util.c
@@ -911,72 +911,8 @@ char *am_getfile(apr_pool_t *conf, server_rec *s, const char *file)
     return data;
 }
 
-/* 
- * Create a directory for saved POST sessions, check for proper permissions
- *
- * Parameters:
- *   request_rec *r     The current request
- *
- * Returns:
- *  OK on success, or HTTP_INTERNAL_SERVER on failure.
- */
-static int am_postdir_mkdir(request_rec *r)
-{
-    apr_int32_t wanted;
-    apr_finfo_t afi;
-    apr_status_t rv;
-    char buffer[512];
-    am_mod_cfg_rec *mod_cfg;
-    apr_fileperms_t mode;
-    apr_uid_t user;
-    apr_uid_t group;
-    apr_fileperms_t prot;
-
-    mod_cfg = am_get_mod_cfg(r->server);
-
-    mode = APR_FPROT_UREAD|APR_FPROT_UWRITE|APR_FPROT_UEXECUTE;
-    if ((rv = apr_dir_make_recursive(mod_cfg->post_dir, mode, r->pool)) != OK) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
-                      "cannot create POST directory \"%s\": %s",
-                      mod_cfg->post_dir,
-                      apr_strerror(rv, buffer, sizeof(buffer)));
-        return HTTP_INTERNAL_SERVER_ERROR;
-    }
-
-    /* 
-     * The directory may have already existed. Check we really own it
-     */
-    wanted = APR_FINFO_USER|APR_FINFO_UPROT|APR_FINFO_GPROT|APR_FINFO_WPROT;
-    if (apr_stat(&afi, mod_cfg->post_dir, wanted, r->pool) == OK) {
-        if (apr_uid_current(&user, &group, r->pool) != OK) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
-                          "apr_uid_current failed");
-            return HTTP_INTERNAL_SERVER_ERROR;
-        }
-
-        if (afi.user != user) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
-                          "POST directory \"%s\" must be owned by the same "
-                          "user as the web server is running as.",
-                          mod_cfg->post_dir);
-            return HTTP_INTERNAL_SERVER_ERROR;
-        }
-
-        prot = APR_FPROT_UREAD|APR_FPROT_UWRITE|APR_FPROT_UEXECUTE;
-        if (afi.protection != prot) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
-                          "Premissions on POST directory \"%s\" must be 0700.",
-                          mod_cfg->post_dir);
-            return HTTP_INTERNAL_SERVER_ERROR;
-        }
-    }
-
-    return OK;
-}
-
-/* 
- * Purge outdated saved POST requests. If the MellonPostDirectory
- * directory does not exist, create it first.
+/*
+ * Purge outdated saved POST requests.
  *
  * Parameters:
  *   request_rec *r     The current request
@@ -989,6 +925,7 @@ int am_postdir_cleanup(request_rec *r)
     am_mod_cfg_rec *mod_cfg;
     apr_dir_t *postdir;
     apr_status_t rv;
+    char error_buffer[64];
     apr_finfo_t afi;
     char *fname;
     int count;
@@ -998,8 +935,14 @@ int am_postdir_cleanup(request_rec *r)
     /*
      * Open our POST directory or create it. 
      */
-    if (apr_dir_open(&postdir, mod_cfg->post_dir, r->pool) != OK)
-        return am_postdir_mkdir(r);
+    rv = apr_dir_open(&postdir, mod_cfg->post_dir, r->pool);
+    if (rv != 0) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "Unable to open MellonPostDirectory \"%s\": %s",
+                      mod_cfg->post_dir,
+                      apr_strerror(rv, error_buffer, sizeof(error_buffer)));
+        return HTTP_INTERNAL_SERVER_ERROR;
+    }
 
     /*
      * Purge outdated items