Index: subversion/mod_authz_svn/mod_authz_svn.c =================================================================== --- subversion/mod_authz_svn/mod_authz_svn.c (revision 10345) +++ subversion/mod_authz_svn/mod_authz_svn.c (working copy) @@ -33,6 +33,7 @@ #include "svn_error.h" #include "svn_path.h" #include "svn_config.h" +#include "../libsvn_subr/config_impl.h" #include "svn_string.h" @@ -40,8 +41,9 @@ enum { AUTHZ_SVN_NONE = 0, - AUTHZ_SVN_READ, - AUTHZ_SVN_WRITE + AUTHZ_SVN_READ = 1, + AUTHZ_SVN_WRITE = 2, + AUTHZ_SVN_RECURSIVE = 4 }; typedef struct { @@ -51,15 +53,20 @@ const char *access_file; } authz_svn_config_rec; -struct parse_authz_line_baton { +struct parse_authz_baton { apr_pool_t *pool; svn_config_t *config; const char *user; int allow; int deny; + + int required_access; + const char *repos_path; + const char *qualified_repos_path; + + int access; }; - /* * Configuration */ @@ -122,7 +129,7 @@ static svn_boolean_t parse_authz_line(const char *name, const char *value, void *baton) { - struct parse_authz_line_baton *b = baton; + struct parse_authz_baton *b = baton; if (strcmp(name, "*")) { if (!b->user) { @@ -159,6 +166,9 @@ return TRUE; } +/* + * Return TRUE when ACCESS has been determined. + */ static int parse_authz_lines(svn_config_t *cfg, const char *repos_name, const char *repos_path, const char *user, @@ -166,7 +176,7 @@ apr_pool_t *pool) { const char *qualified_repos_path; - struct parse_authz_line_baton baton = { 0 }; + struct parse_authz_baton baton = { 0 }; baton.pool = pool; baton.config = cfg; @@ -178,7 +188,7 @@ svn_config_enumerate(cfg, qualified_repos_path, parse_authz_line, &baton); *access = !(baton.deny & required_access) - || (baton.allow & required_access) != 0; + || (baton.allow & required_access); if ((baton.deny & required_access) || (baton.allow & required_access)) @@ -187,18 +197,69 @@ svn_config_enumerate(cfg, repos_path, parse_authz_line, &baton); *access = !(baton.deny & required_access) - || (baton.allow & required_access) != 0; + || (baton.allow & required_access); return (baton.deny & required_access) || (baton.allow & required_access); } -static int check_access(svn_config_t *cfg, - const char *repos_name, const char *repos_path, - const char *user, int required_access, - apr_pool_t *pool) +static svn_boolean_t parse_authz_section(const char *section_name, + void *baton) { + struct parse_authz_baton *b = baton; + int conclusive; + + if (strncmp(section_name, b->qualified_repos_path, + strlen(b->qualified_repos_path)) + && strncmp(section_name, b->repos_path, + strlen(b->repos_path))) { + /* No match, move on to the next section. */ + return TRUE; + } + + b->allow = b->deny = 0; + svn_config_enumerate(b->config, section_name, + parse_authz_line, b); + + conclusive = (b->deny & b->required_access) + || (b->allow & b->required_access); + + b->access = !(b->deny & b->required_access) + || (b->allow & b->required_access) + || !conclusive; + + /* If access isn't denied, move on to check the next section. */ + return b->access; +} + +static int parse_authz_sections(svn_config_t *cfg, + const char *repos_name, const char *repos_path, + const char *user, + int required_access, + apr_pool_t *pool) +{ + struct parse_authz_baton baton = { 0 }; + + baton.pool = pool; + baton.config = cfg; + baton.user = user; + baton.required_access = required_access; + baton.repos_path = repos_path; + baton.qualified_repos_path = apr_pstrcat(pool, repos_name, ":", + repos_path, NULL); + + baton.access = 1; /* Allow by default */ + svn_config__enumerate_sections(cfg, parse_authz_section, &baton); + + return baton.access; +} + +static int check_access(svn_config_t *cfg, const char *repos_name, + const char *repos_path, const char *user, + int required_access, apr_pool_t *pool) +{ const char *base_name; + const char *original_repos_path = repos_path; int access; if (!repos_path) { @@ -209,13 +270,10 @@ return 1; } - if (parse_authz_lines(cfg, repos_name, repos_path, - user, required_access, &access, - pool)) - return access; - base_name = repos_path; - do { + while (!parse_authz_lines(cfg, repos_name, repos_path, + user, required_access, &access, + pool)) { if (base_name[0] == '/' && base_name[1] == '\0') { /* By default, deny access */ return 0; @@ -223,17 +281,22 @@ svn_path_split(repos_path, &repos_path, &base_name, pool); } - while (!parse_authz_lines(cfg, repos_name, repos_path, - user, required_access, &access, - pool)); + if (access && (required_access & AUTHZ_SVN_RECURSIVE) != 0) { + /* Check access on entries below the current repos path */ + access = parse_authz_sections(cfg, + repos_name, original_repos_path, + user, required_access, + pool); + } + return access; } /* 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 - * destination path if the the requested operation was a MOVE or a COPY. + * destination path if the requested operation was a MOVE or a COPY. * Returns OK when access is allowed, DECLINED when it isn't, or an HTTP_ * error code when an error occurred. */ @@ -252,35 +315,43 @@ const char *repos_path; const char *dest_repos_path = NULL; dav_error *dav_err; - int authz_svn_type; + int authz_svn_type = 0; svn_config_t *access_conf = NULL; svn_error_t *svn_err; + const char *cache_key; + void *user_data; switch (r->method_number) { + /* All methods requiring read access to all subtrees of r->uri */ + case M_COPY: + authz_svn_type |= AUTHZ_SVN_RECURSIVE; + /* All methods requiring read access to r->uri */ case M_OPTIONS: case M_GET: - case M_COPY: case M_PROPFIND: case M_REPORT: - authz_svn_type = AUTHZ_SVN_READ; + authz_svn_type |= AUTHZ_SVN_READ; break; + /* All methods requiring write access to all subtrees of r->uri */ + case M_MOVE: + case M_DELETE: + authz_svn_type |= AUTHZ_SVN_RECURSIVE; + /* All methods requiring write access to r->uri */ - case M_MOVE: case M_MKCOL: - case M_DELETE: case M_PUT: case M_PROPPATCH: case M_CHECKOUT: case M_MERGE: case M_MKACTIVITY: - authz_svn_type = AUTHZ_SVN_WRITE; + authz_svn_type |= AUTHZ_SVN_WRITE; break; default: /* Require most strict access for unknown methods */ - authz_svn_type = AUTHZ_SVN_WRITE; + authz_svn_type |= AUTHZ_SVN_WRITE|AUTHZ_SVN_RECURSIVE; break; } @@ -325,7 +396,7 @@ apr_uri_parse(r->pool, dest_uri, &parsed_dest_uri); dest_uri = parsed_dest_uri.path; - ap_unescape_url(dest_uri); + ap_unescape_url((char *)dest_uri); if (strncmp(dest_uri, conf->base_path, strlen(conf->base_path))) { /* If it is not the same location, then we don't allow it. * XXX: Instead we could compare repository uuids, but that @@ -357,12 +428,23 @@ dest_repos_path, NULL); } - svn_err = svn_config_read(&access_conf, conf->access_file, FALSE, r->pool); - if (svn_err) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, svn_err->apr_err, r, - "%s", svn_err->message); + /* 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_config_read(&access_conf, conf->access_file, FALSE, + r->connection->pool); + if (svn_err) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, svn_err->apr_err, r, + "%s", svn_err->message); - 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); } if (!check_access(access_conf, @@ -372,10 +454,15 @@ return DECLINED; } - /* XXX: DELETE, MOVE, MKCOL and PUT, if the path doesn't exist yet, also - * XXX: require write access to the parent dir of repos_path. + /* XXX: MKCOL, MOVE, DELETE + * XXX: Require write access to the parent dir of repos_path. */ + /* XXX: PUT + * XXX: If the path doesn't exist, require write access to the + * XXX: parent dir of repos_path. + */ + /* Only MOVE and COPY have a second uri we have to check access to. */ if (r->method_number != M_MOVE && r->method_number != M_COPY) { @@ -385,7 +472,7 @@ /* Check access on the first repos_path */ if (!check_access(access_conf, dest_repos_name, dest_repos_path, - r->user, AUTHZ_SVN_WRITE, + r->user, AUTHZ_SVN_WRITE|AUTHZ_SVN_RECURSIVE, r->pool)) { return DECLINED; } @@ -413,15 +500,26 @@ if (!conf->anonymous || !conf->access_file) return DECLINED; - /* It makes no sense to check if a location is both accessible - * anonymous and by an authenticated user. - */ - if (ap_some_auth_required(r) && ap_satisfies(r) != SATISFY_ANY) - return DECLINED; + if (ap_some_auth_required(r)) { + /* It makes no sense to check if a location is both accessible + * anonymous and by an authenticated user (in the same request!). + */ + if (ap_satisfies(r) != SATISFY_ANY) + return DECLINED; - /* XXX: Don't run when ap_some_auth_required(r) and an Authorization - * XXX: or Proxy-Authorization are present? - */ + /* 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 (apr_table_get(r->headers_in, + (PROXYREQ_PROXY == r->proxyreq) + ? "Proxy-Authorization" : "Authorization")) { + /* Given Satisfy Any is in effect, we have to forbid access + * to let the auth_checker hook have a go at it. + */ + return HTTP_FORBIDDEN; + } + } /* If anon access is allowed, return OK */ status = req_check_access(r, conf, &repos_path, &dest_repos_path); Index: subversion/include/svn_config.h =================================================================== --- subversion/include/svn_config.h (revision 10345) +++ subversion/include/svn_config.h (working copy) @@ -207,7 +207,7 @@ * of an enumeration early, with no error, than an invocation of * @a callback is likely to need to return an error? ### * - * @a callback's @a name and @a name parameters are only valid for the + * @a callback's @a name and @a value parameters are only valid for the * duration of the call. */ int svn_config_enumerate (svn_config_t *cfg, const char *section, Index: subversion/libsvn_subr/config.c =================================================================== --- subversion/libsvn_subr/config.c (revision 10345) +++ subversion/libsvn_subr/config.c (working copy) @@ -645,7 +645,33 @@ } + +int +svn_config__enumerate_sections (svn_config_t *cfg, + svn_config__section_enumerator_t callback, + void *baton) +{ + apr_hash_index_t *sec_ndx; + int count = 0; + for (sec_ndx = apr_hash_first (cfg->x_pool, cfg->sections); + sec_ndx != NULL; + sec_ndx = apr_hash_next (sec_ndx)) + { + void *sec_ptr; + cfg_section_t *sec; + + apr_hash_this (sec_ndx, NULL, NULL, &sec_ptr); + sec = sec_ptr; + ++count; + if (!callback (sec->name, baton)) + break; + } + + return count; +} + + int svn_config_enumerate (svn_config_t *cfg, const char *section, Index: subversion/libsvn_subr/config_impl.h =================================================================== --- subversion/libsvn_subr/config_impl.h (revision 10345) +++ subversion/libsvn_subr/config_impl.h (working copy) @@ -157,6 +157,28 @@ const char *mode, apr_pool_t *pool); +/** A callback function used in enumerating config sections. + * + * See @c svn_config_enumerate_sections for the details of this type. + */ +typedef svn_boolean_t (*svn_config__section_enumerator_t) + (const char *name, void *baton); + +/** Enumerate the sections, passing @a baton and the current section's name to + * @a callback. Continue the enumeration if @a callback returns @c TRUE. + * Return the number of times @a callback was called. + * + * ### See kff's comment to @c svn_config_enumerate. It applies to this + * function, too. ### + * + * @a callback's @a name and @a name parameters are only valid for the + * duration of the call. + */ +int svn_config__enumerate_sections (svn_config_t *cfg, + svn_config__section_enumerator_t callback, + void *baton); + + #ifdef __cplusplus } #endif /* __cplusplus */