subversion-1.9.7HEADsubversion-1.9.7master

1 files changed, 169 insertions, 25 deletions

diff --git a/subversion/mod_authz_svn/mod_authz_svn.c b/subversion/mod_authz_svn/mod_authz_svn.c
index e9e43eb..0adeaf9 100644
--- a/subversion/mod_authz_svn/mod_authz_svn.c
+++ b/subversion/mod_authz_svn/mod_authz_svn.c
@@ -48,6 +48,23 @@
 #include "svn_dirent_uri.h"
 #include "private/svn_fspath.h"
 
+/* The apache headers define these and they conflict with our definitions. */
+#ifdef PACKAGE_BUGREPORT
+#undef PACKAGE_BUGREPORT
+#endif
+#ifdef PACKAGE_NAME
+#undef PACKAGE_NAME
+#endif
+#ifdef PACKAGE_STRING
+#undef PACKAGE_STRING
+#endif
+#ifdef PACKAGE_TARNAME
+#undef PACKAGE_TARNAME
+#endif
+#ifdef PACKAGE_VERSION
+#undef PACKAGE_VERSION
+#endif
+#include "svn_private_config.h"
 
 #ifdef APLOG_USE_MODULE
 APLOG_USE_MODULE(authz_svn);
@@ -67,6 +84,28 @@ typedef struct authz_svn_config_rec {
   const char *force_username_case;
 } authz_svn_config_rec;
 
+/* version where ap_some_auth_required breaks */
+#if AP_MODULE_MAGIC_AT_LEAST(20060110,0)
+/* first version with force_authn hook and ap_some_authn_required()
+   which allows us to work without ap_some_auth_required() */
+#  if AP_MODULE_MAGIC_AT_LEAST(20120211,47) || defined(SVN_USE_FORCE_AUTHN)
+#    define USE_FORCE_AUTHN 1
+#    define IN_SOME_AUTHN_NOTE "authz_svn-in-some-authn"
+#    define FORCE_AUTHN_NOTE "authz_svn-force-authn"
+#  else 
+     /* ap_some_auth_required() is busted and no viable alternative exists */
+#    ifndef SVN_ALLOW_BROKEN_HTTPD_AUTH
+#      error This Apache httpd has broken auth (CVE-2015-3184)
+#    else
+       /* user wants to build anyway */
+#      define USE_FORCE_AUTHN 0
+#    endif
+#  endif
+#else
+   /* old enough that ap_some_auth_required() still works */
+#  define USE_FORCE_AUTHN 0
+#endif
+
 /*
  * Configuration
  */
@@ -132,7 +171,7 @@ AuthzSVNAccessFile_cmd(cmd_parms *cmd, void *config, const char *arg1)
 
   conf->access_file = canonicalize_access_file(arg1, TRUE, cmd->pool);
   if (!conf->access_file)
-    return apr_pstrcat(cmd->pool, "Invalid file path ", arg1, NULL);
+    return apr_pstrcat(cmd->pool, "Invalid file path ", arg1, SVN_VA_NULL);
 
   return NULL;
 }
@@ -153,7 +192,7 @@ AuthzSVNReposRelativeAccessFile_cmd(cmd_parms *cmd,
                                                              cmd->pool);
 
   if (!conf->repo_relative_access_file)
-    return apr_pstrcat(cmd->pool, "Invalid file path ", arg1, NULL);
+    return apr_pstrcat(cmd->pool, "Invalid file path ", arg1, SVN_VA_NULL);
 
   return NULL;
 }
@@ -166,7 +205,7 @@ AuthzSVNGroupsFile_cmd(cmd_parms *cmd, void *config, const char *arg1)
   conf->groups_file = canonicalize_access_file(arg1, TRUE, cmd->pool);
 
   if (!conf->groups_file)
-    return apr_pstrcat(cmd->pool, "Invalid file path ", arg1, NULL);
+    return apr_pstrcat(cmd->pool, "Invalid file path ", arg1, SVN_VA_NULL);
 
   return NULL;
 }
@@ -244,15 +283,26 @@ static const command_rec authz_svn_cmds[] =
  * per-module loglevel configuration.  It expands to FILE and LINE
  * in older server versions.  ALLOWED is boolean.
  * REPOS_PATH and DEST_REPOS_PATH are information
- * about the request.  DEST_REPOS_PATH may be NULL. */
+ * about the request.  DEST_REPOS_PATH may be NULL.
+ * Non-zero IS_SUBREQ_BYPASS means that this authorization check was
+ * implicitly requested using 'subrequest bypass' callback from
+ * mod_dav_svn.
+ */
 static void
 log_access_verdict(LOG_ARGS_SIGNATURE,
-                   const request_rec *r, int allowed,
+                   const request_rec *r, int allowed, int is_subreq_bypass,
                    const char *repos_path, const char *dest_repos_path)
 {
   int level = allowed ? APLOG_INFO : APLOG_ERR;
   const char *verdict = allowed ? "granted" : "denied";
 
+  /* Use less important log level for implicit sub-request authorization
+     checks. */
+  if (is_subreq_bypass)
+    level = APLOG_INFO;
+  else if (r->main && r->method_number == M_GET)
+    level = APLOG_INFO;
+
   if (r->user)
     {
       if (dest_repos_path)
@@ -361,7 +411,7 @@ get_access_conf(request_rec *r, authz_svn_config_rec *conf,
   svn_error_t *svn_err = SVN_NO_ERROR;
   dav_error *dav_err;
 
-  dav_err = dav_svn_get_repos_path(r, conf->base_path, &repos_path);
+  dav_err = dav_svn_get_repos_path2(r, conf->base_path, &repos_path, scratch_pool);
   if (dav_err)
     {
       ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "%s", dav_err->desc);
@@ -376,7 +426,7 @@ get_access_conf(request_rec *r, authz_svn_config_rec *conf,
         {
           access_file = svn_dirent_join_many(scratch_pool, repos_path, "conf",
                                              conf->repo_relative_access_file,
-                                             NULL);
+                                             SVN_VA_NULL);
         }
     }
   else
@@ -417,7 +467,7 @@ get_access_conf(request_rec *r, authz_svn_config_rec *conf,
     }
 
   cache_key = apr_pstrcat(scratch_pool, "mod_authz_svn:",
-                          access_file, groups_file, (char *)NULL);
+                          access_file, groups_file, SVN_VA_NULL);
   apr_pool_userdata_get(&user_data, cache_key, r->connection->pool);
   access_conf = user_data;
   if (access_conf == NULL)
@@ -585,10 +635,12 @@ req_check_access(request_rec *r,
     repos_path = svn_fspath__canonicalize(repos_path, r->pool);
 
   *repos_path_ref = apr_pstrcat(r->pool, repos_name, ":", repos_path,
-                                (char *)NULL);
+                                SVN_VA_NULL);
 
   if (r->method_number == M_MOVE || r->method_number == M_COPY)
     {
+      apr_status_t status;
+
       dest_uri = apr_table_get(r->headers_in, "Destination");
 
       /* Decline MOVE or COPY when there is no Destination uri, this will
@@ -597,7 +649,19 @@ req_check_access(request_rec *r,
       if (!dest_uri)
         return DECLINED;
 
-      apr_uri_parse(r->pool, dest_uri, &parsed_dest_uri);
+      status = apr_uri_parse(r->pool, dest_uri, &parsed_dest_uri);
+      if (status)
+        {
+          ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
+                        "Invalid URI in Destination header");
+          return HTTP_BAD_REQUEST;
+        }
+      if (!parsed_dest_uri.path)
+        {
+          ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                        "Invalid URI in Destination header");
+          return HTTP_BAD_REQUEST;
+        }
 
       ap_unescape_url(parsed_dest_uri.path);
       dest_uri = parsed_dest_uri.path;
@@ -633,7 +697,7 @@ req_check_access(request_rec *r,
         dest_repos_path = svn_fspath__canonicalize(dest_repos_path, r->pool);
 
       *dest_repos_path_ref = apr_pstrcat(r->pool, dest_repos_name, ":",
-                                         dest_repos_path, (char *)NULL);
+                                         dest_repos_path, SVN_VA_NULL);
     }
 
   /* Retrieve/cache authorization file */
@@ -749,7 +813,7 @@ subreq_bypass2(request_rec *r,
   if (!conf->anonymous
       || (! (conf->access_file || conf->repo_relative_access_file)))
     {
-      log_access_verdict(APLOG_MARK, r, 0, repos_path, NULL);
+      log_access_verdict(APLOG_MARK, r, 0, TRUE, repos_path, NULL);
       return HTTP_FORBIDDEN;
     }
 
@@ -778,12 +842,12 @@ subreq_bypass2(request_rec *r,
         }
       if (!authz_access_granted)
         {
-          log_access_verdict(APLOG_MARK, r, 0, repos_path, NULL);
+          log_access_verdict(APLOG_MARK, r, 0, TRUE, repos_path, NULL);
           return HTTP_FORBIDDEN;
         }
     }
 
-  log_access_verdict(APLOG_MARK, r, 1, repos_path, NULL);
+  log_access_verdict(APLOG_MARK, r, 1, TRUE, repos_path, NULL);
 
   return OK;
 }
@@ -819,14 +883,57 @@ access_checker(request_rec *r)
                                                     &authz_svn_module);
   const char *repos_path = NULL;
   const char *dest_repos_path = NULL;
-  int status;
+  int status, authn_required;
+
+#if USE_FORCE_AUTHN
+  /* Use the force_authn() hook available in 2.4.x to work securely
+   * given that ap_some_auth_required() is no longer functional for our
+   * purposes in 2.4.x.
+   */
+  int authn_configured;
+
+  /* We are not configured to run */
+  if (!conf->anonymous || apr_table_get(r->notes, IN_SOME_AUTHN_NOTE)
+      || (! (conf->access_file || conf->repo_relative_access_file)))
+    return DECLINED;
+
+  /* Authentication is configured */
+  authn_configured = ap_auth_type(r) != NULL;
+  if (authn_configured)
+    {
+      /* If the user is trying to authenticate, let him.  It doesn't
+       * make much sense to grant anonymous access but deny authenticated
+       * users access, even though you can do that with '$anon' in the
+       * access file.
+       */
+      if (apr_table_get(r->headers_in,
+                        (PROXYREQ_PROXY == r->proxyreq)
+                        ? "Proxy-Authorization" : "Authorization"))
+        {
+          /* Set the note to force authn regardless of what access_checker_ex
+             hook requires */
+          apr_table_setn(r->notes, FORCE_AUTHN_NOTE, (const char*)1);
+
+          /* provide the proper return so the access_checker hook doesn't
+           * prevent the code from continuing on to the other auth hooks */
+          if (ap_satisfies(r) != SATISFY_ANY)
+            return OK;
+          else
+            return HTTP_FORBIDDEN;
+        }
+    }    
+
+#else
+  /* Support for older versions of httpd that have a working
+   * ap_some_auth_required() */
 
   /* We are not configured to run */
   if (!conf->anonymous
       || (! (conf->access_file || conf->repo_relative_access_file)))
     return DECLINED;
 
-  if (ap_some_auth_required(r))
+  authn_required = ap_some_auth_required(r);
+  if (authn_required)
     {
       /* It makes no sense to check if a location is both accessible
        * anonymous and by an authenticated user (in the same request!).
@@ -834,9 +941,10 @@ access_checker(request_rec *r)
       if (ap_satisfies(r) != SATISFY_ANY)
         return DECLINED;
 
-      /* If the user is trying to authenticate, let him.  If anonymous
-       * access is allowed, so is authenticated access, by definition
-       * of the meaning of '*' in the access file.
+      /* If the user is trying to authenticate, let him.  It doesn't
+       * make much sense to grant anonymous access but deny authenticated
+       * users access, even though you can do that with '$anon' in the
+       * access file.
        */
       if (apr_table_get(r->headers_in,
                         (PROXYREQ_PROXY == r->proxyreq)
@@ -848,6 +956,7 @@ access_checker(request_rec *r)
           return HTTP_FORBIDDEN;
         }
     }
+#endif
 
   /* If anon access is allowed, return OK */
   status = req_check_access(r, conf, &repos_path, &dest_repos_path);
@@ -856,8 +965,29 @@ access_checker(request_rec *r)
       if (!conf->authoritative)
         return DECLINED;
 
-      if (!ap_some_auth_required(r))
-        log_access_verdict(APLOG_MARK, r, 0, repos_path, dest_repos_path);
+#if USE_FORCE_AUTHN
+      if (authn_configured) {
+          /* We have to check to see if authn is required because if so we must
+           * return DECLINED rather than FORBIDDEN (403) since returning
+           * the 403 leaks information about what paths may exist to
+           * unauthenticated users.  Returning DECLINED means apache's request
+           * handling will continue until the authn module itself generates
+           * UNAUTHORIZED (401).
+
+           * We must set a note here in order to use
+           * ap_some_authn_rquired() without triggering an infinite
+           * loop since the call will trigger this function to be
+           * called again. */
+          apr_table_setn(r->notes, IN_SOME_AUTHN_NOTE, (const char*)1);
+          authn_required = ap_some_authn_required(r);
+          apr_table_unset(r->notes, IN_SOME_AUTHN_NOTE);
+          if (authn_required)
+            return DECLINED;
+      }
+#else
+      if (!authn_required)
+#endif
+        log_access_verdict(APLOG_MARK, r, 0, FALSE, repos_path, dest_repos_path);
 
       return HTTP_FORBIDDEN;
     }
@@ -865,7 +995,7 @@ access_checker(request_rec *r)
   if (status != OK)
     return status;
 
-  log_access_verdict(APLOG_MARK, r, 1, repos_path, dest_repos_path);
+  log_access_verdict(APLOG_MARK, r, 1, FALSE, repos_path, dest_repos_path);
 
   return OK;
 }
@@ -892,7 +1022,7 @@ check_user_id(request_rec *r)
   if (status == OK)
     {
       apr_table_setn(r->notes, "authz_svn-anon-ok", (const char*)1);
-      log_access_verdict(APLOG_MARK, r, 1, repos_path, dest_repos_path);
+      log_access_verdict(APLOG_MARK, r, 1, FALSE, repos_path, dest_repos_path);
       return OK;
     }
 
@@ -922,7 +1052,7 @@ auth_checker(request_rec *r)
     {
       if (conf->authoritative)
         {
-          log_access_verdict(APLOG_MARK, r, 0, repos_path, dest_repos_path);
+          log_access_verdict(APLOG_MARK, r, 0, FALSE, repos_path, dest_repos_path);
           ap_note_auth_failure(r);
           return HTTP_FORBIDDEN;
         }
@@ -932,11 +1062,22 @@ auth_checker(request_rec *r)
   if (status != OK)
     return status;
 
-  log_access_verdict(APLOG_MARK, r, 1, repos_path, dest_repos_path);
+  log_access_verdict(APLOG_MARK, r, 1, FALSE, repos_path, dest_repos_path);
 
   return OK;
 }
 
+#if USE_FORCE_AUTHN
+static int
+force_authn(request_rec *r)
+{
+  if (apr_table_get(r->notes, FORCE_AUTHN_NOTE))
+    return OK;
+
+  return DECLINED;
+}
+#endif
+
 /*
  * Module flesh
  */
@@ -953,6 +1094,9 @@ register_hooks(apr_pool_t *p)
    * give SSLOptions +FakeBasicAuth a chance to work. */
   ap_hook_check_user_id(check_user_id, mod_ssl, NULL, APR_HOOK_FIRST);
   ap_hook_auth_checker(auth_checker, NULL, NULL, APR_HOOK_FIRST);
+#if USE_FORCE_AUTHN
+  ap_hook_force_authn(force_authn, NULL, NULL, APR_HOOK_FIRST);
+#endif
   ap_register_provider(p,
                        AUTHZ_SVN__SUBREQ_BYPASS_PROV_GRP,
                        AUTHZ_SVN__SUBREQ_BYPASS_PROV_NAME,