Index: subversion/mod_authz_svn/mod_authz_svn.c =================================================================== --- subversion/mod_authz_svn/mod_authz_svn.c (revision 24559) +++ subversion/mod_authz_svn/mod_authz_svn.c (working copy) @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #if AP_MODULE_MAGIC_AT_LEAST(20060110,0) @@ -34,6 +35,7 @@ #endif #include "mod_dav_svn.h" +#include "mod_authz_svn.h" #include "svn_path.h" #include "svn_config.h" #include "svn_string.h" @@ -95,6 +97,47 @@ { NULL } }; +/* + * Get the, possibly cached, svn_authz_t for this request. + */ +static svn_authz_t* get_access_conf(request_rec *r, + authz_svn_config_rec *conf) +{ + const char* cache_key=NULL; + void* user_data=NULL; + svn_authz_t* access_conf=NULL; + svn_error_t *svn_err; + char errbuf[256]; + + cache_key = apr_pstrcat(r->pool, "mod_authz_svn:", conf->access_file, NULL); + apr_pool_userdata_get(&user_data, cache_key, r->connection->pool); + access_conf = user_data; + if (access_conf == NULL) { + svn_err = svn_repos_authz_read(&access_conf, conf->access_file, + TRUE, r->connection->pool); + if (svn_err) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, + /* If it is an error code that APR can make sense + of, then show it, otherwise, pass zero to avoid + putting "APR does not understand this error code" + in the error log. */ + ((svn_err->apr_err >= APR_OS_START_USERERR && + svn_err->apr_err < APR_OS_START_CANONERR) ? + 0 : svn_err->apr_err), + r, "Failed to load the AuthzSVNAccessFile: %s", + svn_err_best_message(svn_err, + errbuf, sizeof(errbuf))); + svn_error_clear(svn_err); + access_conf=NULL; + } else { + /* Cache the open repos for the next request on this connection */ + apr_pool_userdata_set(access_conf, cache_key, + NULL, r->connection->pool); + } + } + return access_conf; +} + /* Check if the current request R is allowed. Upon exit *REPOS_PATH_REF * will contain the path and repository name that an operation was requested * on in the form 'name:path'. *DEST_REPOS_PATH_REF will contain the @@ -121,8 +164,6 @@ svn_boolean_t authz_access_granted = FALSE; svn_authz_t *access_conf = NULL; svn_error_t *svn_err; - const char *cache_key; - void *user_data; char errbuf[256]; switch (r->method_number) { @@ -239,34 +280,10 @@ } /* Retrieve/cache authorization file */ - cache_key = apr_pstrcat(r->pool, "mod_authz_svn:", conf->access_file, NULL); - apr_pool_userdata_get(&user_data, cache_key, r->connection->pool); - access_conf = user_data; - if (access_conf == NULL) { - svn_err = svn_repos_authz_read(&access_conf, conf->access_file, - TRUE, r->connection->pool); - if (svn_err) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, - /* If it is an error code that APR can make sense - of, then show it, otherwise, pass zero to avoid - putting "APR does not understand this error code" - in the error log. */ - ((svn_err->apr_err >= APR_OS_START_USERERR && - svn_err->apr_err < APR_OS_START_CANONERR) ? - 0 : svn_err->apr_err), - r, "Failed to load the AuthzSVNAccessFile: %s", - svn_err_best_message(svn_err, - errbuf, sizeof(errbuf))); - svn_error_clear(svn_err); + access_conf = get_access_conf(r,conf); + if(NULL==access_conf) + return DECLINED; - return DECLINED; - } - - /* Cache the open repos for the next request on this connection */ - apr_pool_userdata_set(access_conf, cache_key, - NULL, r->connection->pool); - } - /* Perform authz access control. * * First test the special case where repos_path == NULL, and skip @@ -286,6 +303,9 @@ * XXX: For now, requesting access to the entire repository always * XXX: succeeds, until we come up with a good way of figuring * XXX: this out. + * + * Changes to this section of the code should also be propagated to + * subreq_bypass if needed. */ if (repos_path || (!repos_path && (authz_svn_type & svn_authz_write))) @@ -408,6 +428,70 @@ } /* + * This function is used as a provider to allow mod_dav_svn to bypass the + * generation of an apache request when checking GET access from + * "mod_dav_svn/authz.c" . + */ +static int subreq_bypass(request_rec *r, + const char *repos_path, + const char *repos_name) +{ + svn_error_t *svn_err=NULL; + svn_authz_t *access_conf = NULL; + authz_svn_config_rec *conf = NULL; + svn_boolean_t authz_access_granted = FALSE; + char errbuf[256]; + + conf = ap_get_module_config(r->per_dir_config, + &authz_svn_module); + + /* If configured properly, this should never be true, but just in case. */ + if (!conf->anonymous || !conf->access_file) { + log_access_verdict(APLOG_MARK, r, 0, repos_path, NULL); + return HTTP_FORBIDDEN; + } + + /* Retrieve authorization file */ + access_conf = get_access_conf(r,conf); + if(NULL==access_conf) + return HTTP_FORBIDDEN; + + /* Perform authz access control. + * See similarly labeled comment in req_check_access. + */ + if (repos_path) { + svn_err = svn_repos_authz_check_access(access_conf, repos_name, + repos_path, r->user, + svn_authz_none|svn_authz_read, + &authz_access_granted, + r->pool); + if (svn_err) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, + /* If it is an error code that APR can make + sense of, then show it, otherwise, pass + zero to avoid putting "APR does not + understand this error code" in the error + log. */ + ((svn_err->apr_err >= APR_OS_START_USERERR && + svn_err->apr_err < APR_OS_START_CANONERR) ? + 0 : svn_err->apr_err), + r, "Failed to perform access control: %s", + svn_err_best_message(svn_err, errbuf, sizeof(errbuf))); + svn_error_clear(svn_err); + return HTTP_FORBIDDEN; + } + if (!authz_access_granted) { + log_access_verdict(APLOG_MARK, r, 0, repos_path, NULL); + return HTTP_FORBIDDEN; + } + } + + log_access_verdict(APLOG_MARK, r, 1, repos_path, NULL); + + return OK; +} + +/* * Hooks */ @@ -552,6 +636,11 @@ #if AP_MODULE_MAGIC_AT_LEAST(20060110,0) ap_hook_optional_fn_retrieve(import_ap_satisfies, NULL, NULL, APR_HOOK_MIDDLE); #endif + ap_register_provider(p, + AUTHZ_SVN__SUBREQ_BYPASS_PROV_GRP, + AUTHZ_SVN__SUBREQ_BYPASS_PROV_NAME, + AUTHZ_SVN__SUBREQ_BYPASS_PROV_VER, + (void*)subreq_bypass); } module AP_MODULE_DECLARE_DATA authz_svn_module = Index: subversion/include/mod_authz_svn.h =================================================================== --- subversion/include/mod_authz_svn.h (revision 0) +++ subversion/include/mod_authz_svn.h (revision 0) @@ -0,0 +1,26 @@ +// Author: jwg@google.com (Josh Gilkerson) + +#ifndef MOD_AUTHZ_SVN_H +#define MOD_AUTHZ_SVN_H + +#include + +#ifdef __cplusplus +extern "C" { +#endif /* __cplusplus */ + +/* + * mod_dav_svn to mod_authz_svn bypass mechanism + */ +#define AUTHZ_SVN__SUBREQ_BYPASS_PROV_GRP "dav2authz_subreq_bypass" +#define AUTHZ_SVN__SUBREQ_BYPASS_PROV_NAME "mod_authz_svn_subreq_bypass" +#define AUTHZ_SVN__SUBREQ_BYPASS_PROV_VER "00.00a" +typedef int (*authz_svn__subreq_bypass_func_t)(request_rec *r, + const char *repos_path, + const char *repos_name); + +#ifdef __cplusplus +} +#endif /* __cplusplus */ + +#endif Index: subversion/mod_dav_svn/mod_dav_svn.c =================================================================== --- subversion/mod_dav_svn/mod_dav_svn.c (revision 24559) +++ subversion/mod_dav_svn/mod_dav_svn.c (working copy) @@ -39,6 +39,9 @@ (e.g. working resources, activities) */ #define SVN_DEFAULT_SPECIAL_URI "!svn" +/* This is the value to be given to SVNPathAuthz to bypass the apache + * subreq mechanism and make a call directly to mod_authz_svn. */ +#define PATHAUTHZ_BYPASS_ARG "dangerous_direct" /* per-server configuration */ typedef struct { @@ -55,6 +58,13 @@ CONF_FLAG_OFF }; +/* An enum used for the per directory configuration path_authz_method. */ +enum path_authz_conf { + CONF_PATHAUTHZ_DEFAULT, + CONF_PATHAUTHZ_ON, + CONF_PATHAUTHZ_OFF, + CONF_PATHAUTHZ_BYPASS +}; /* per-dir configuration */ typedef struct { @@ -63,7 +73,7 @@ const char *xslt_uri; /* XSL transform URI */ const char *fs_parent_path; /* path to parent of SVN FS'es */ enum conf_flag autoversioning; /* whether autoversioning is active */ - enum conf_flag do_path_authz; /* whether GET subrequests are active */ + enum path_authz_conf path_authz_method; /* how GET subrequests are handled */ enum conf_flag list_parentpath; /* whether to allow GET of parentpath */ const char *root_dir; /* our top-level directory */ const char *master_uri; /* URI to the master SVN repos */ @@ -160,7 +170,7 @@ newconf->xslt_uri = INHERIT_VALUE(parent, child, xslt_uri); newconf->fs_parent_path = INHERIT_VALUE(parent, child, fs_parent_path); newconf->autoversioning = INHERIT_VALUE(parent, child, autoversioning); - newconf->do_path_authz = INHERIT_VALUE(parent, child, do_path_authz); + newconf->path_authz_method = INHERIT_VALUE(parent, child, path_authz_method); newconf->list_parentpath = INHERIT_VALUE(parent, child, list_parentpath); /* Prefer our parent's value over our new one - hence the swap. */ newconf->root_dir = INHERIT_VALUE(child, parent, root_dir); @@ -217,14 +227,16 @@ static const char * -SVNPathAuthz_cmd(cmd_parms *cmd, void *config, int arg) +SVNPathAuthz_cmd(cmd_parms *cmd, void *config, const char* arg1) { dir_conf_t *conf = config; - if (arg) - conf->do_path_authz = CONF_FLAG_ON; + if (!apr_strnatcasecmp("off",arg1)) + conf->path_authz_method = CONF_PATHAUTHZ_OFF; + else if (!apr_strnatcasecmp(PATHAUTHZ_BYPASS_ARG,arg1)) + conf->path_authz_method = CONF_PATHAUTHZ_BYPASS; else - conf->do_path_authz = CONF_FLAG_OFF; + conf->path_authz_method = CONF_PATHAUTHZ_ON; return NULL; } @@ -433,17 +445,30 @@ return conf->autoversioning == CONF_FLAG_ON; } - +/* FALSE if path authorization should be skipped. + * TRUE if either the bypass or the apache subrequest methods should be used. + */ svn_boolean_t dav_svn__get_pathauthz_flag(request_rec *r) { dir_conf_t *conf; conf = ap_get_module_config(r->per_dir_config, &dav_svn_module); - return conf->do_path_authz != CONF_FLAG_OFF; + return conf->path_authz_method != CONF_PATHAUTHZ_OFF; } +/* TRUE if we should use the bypass directly to mod_authz_svn. + * FALSE otherwise. */ +svn_boolean_t +dav_svn__get_pathauthz_bypass(request_rec *r) +{ + dir_conf_t *conf; + conf = ap_get_module_config(r->per_dir_config, &dav_svn_module); + return conf->path_authz_method == CONF_PATHAUTHZ_BYPASS; +} + + svn_boolean_t dav_svn__get_list_parentpath_flag(request_rec *r) { @@ -609,7 +634,9 @@ /* per directory/location */ AP_INIT_FLAG("SVNPathAuthz", SVNPathAuthz_cmd, NULL, ACCESS_CONF|RSRC_CONF, - "control path-based authz by enabling/disabling subrequests"), + "control path-based authz by enabling subrequests(On,default), " + "disabling subrequests(Off), or" + "querying mod_authz_svn directly(" PATHAUTHZ_BYPASS_ARG ")"), /* per directory/location */ AP_INIT_FLAG("SVNListParentPath", SVNListParentPath_cmd, NULL, Index: subversion/mod_dav_svn/dav_svn.h =================================================================== --- subversion/mod_dav_svn/dav_svn.h (revision 24559) +++ subversion/mod_dav_svn/dav_svn.h (working copy) @@ -252,6 +252,9 @@ /* for the repository referred to by this request, are subrequests active? */ svn_boolean_t dav_svn__get_pathauthz_flag(request_rec *r); +/* for the repository referred to by this request, are subrequests bypassed? */ +svn_boolean_t dav_svn__get_pathauthz_bypass(request_rec *r); + /* for the repository referred to by this request, is a GET of SVNParentPath allowed? */ svn_boolean_t dav_svn__get_list_parentpath_flag(request_rec *r); Index: subversion/mod_dav_svn/authz.c =================================================================== --- subversion/mod_dav_svn/authz.c (revision 24559) +++ subversion/mod_dav_svn/authz.c (working copy) @@ -18,10 +18,12 @@ #include #include +#include #include "svn_pools.h" #include "svn_path.h" +#include "mod_authz_svn.h" #include "dav_svn.h" @@ -41,6 +43,7 @@ request_rec *subreq; enum dav_svn__build_what uri_type; svn_boolean_t allowed = FALSE; + authz_svn__subreq_bypass_func_t allow_read_bypass=NULL; /* Easy out: if the admin has explicitly set 'SVNPathAuthz Off', then this whole callback does nothing. */ @@ -49,26 +52,43 @@ return TRUE; } - /* If no revnum is specified, assume HEAD. */ - if (SVN_IS_VALID_REVNUM(rev)) - uri_type = DAV_SVN__BUILD_URI_VERSION; - else - uri_type = DAV_SVN__BUILD_URI_PUBLIC; + /* If bypass is specified and authz has exported the provider. + * Otherwise, we fall through to the full version. This should be + * safer than allowing or disallowing all accesses if there + * is a configuration error. + * XXX: Is this the proper thing to do in this case? + * XXX: Can/should we cache the provider lookup? + */ + if (dav_svn__get_pathauthz_bypass(r) + && NULL != (allow_read_bypass=ap_lookup_provider( + AUTHZ_SVN__SUBREQ_BYPASS_PROV_GRP, + AUTHZ_SVN__SUBREQ_BYPASS_PROV_NAME, + AUTHZ_SVN__SUBREQ_BYPASS_PROV_VER)) + ) { + if (OK==allow_read_bypass(r,uri,repos->repo_name)) + allowed = TRUE; + } else { + /* If no revnum is specified, assume HEAD. */ + if (SVN_IS_VALID_REVNUM(rev)) + uri_type = DAV_SVN__BUILD_URI_VERSION; + else + uri_type = DAV_SVN__BUILD_URI_PUBLIC; + + /* Build a Version Resource uri representing (rev, path). */ + uri = dav_svn__build_uri(repos, uri_type, rev, path, FALSE, pool); + + /* Check if GET would work against this uri. */ + subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters); + + if (subreq) + { + if (subreq->status == HTTP_OK) + allowed = TRUE; + + ap_destroy_sub_req(subreq); + } + } - /* Build a Version Resource uri representing (rev, path). */ - uri = dav_svn__build_uri(repos, uri_type, rev, path, FALSE, pool); - - /* Check if GET would work against this uri. */ - subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters); - - if (subreq) - { - if (subreq->status == HTTP_OK) - allowed = TRUE; - - ap_destroy_sub_req(subreq); - } - return allowed; }