Hi devs,
At WANdisco, people have been playing with the new
wildcard support for authz (see authz-performance branch)
and ran into an interesting problem.
Today, recursive operations (COPY, DELETE and MOVE)
require recursive access rights on the respective trees.
For instance, COPY requires recursive read rights on the
source - which is fine b/c we don't want the copy to expose
previously protected data.
The problem with authz, however, is that we don't perform
a full tree crawl (and don't intend to) but check the authz
file for rules that *may* affect the respective sub-tree. A
rule like [:glob:/**/yeti] will *always* match whether there
are "yeti"s in your repo or not. Without wildcard support
the problem exists as well but is less prevalent because
tend to write explicit rules for paths that don't exist. For
wildcard rules, OTOH, it is almost the whole point to cover
existing and not-yet existing, future paths was well.
My suggestion is to relax the requirements as follows:
COPY & MOVE still require recursive read access on
the source but only non-recursive write access on the
destination. Thus it becomes possible to protect data
in branches and tags right from their inception without
relaxing read access requirements to existing data. The
attached patch achieves just that.
I have 3 questions:
(1) Is there something fundamentally wrong with this
approach, e.g. braking major use-cases?
(2) Should we require write access to target and target's
parent folder (as done by the patch) or just to the target's
parent folder?
(3) Should we (optionally?) reduce the access rights reqs
for DELETE similarly such that no recursive write access
is needed? That seems risky but would be symmetrical
to creating copies with r/o or n/a sub-paths. MOVE would
be updated accordingly (always the same reqs as a COPY
+ DELETE).
-- Stefan^2.
Index: subversion/mod_authz_svn/mod_authz_svn.c
===================================================================
--- subversion/mod_authz_svn/mod_authz_svn.c (revision 1679144)
+++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
@@ -714,8 +714,7 @@
dest_repos_name,
dest_repos_path,
username_to_authorize,
- svn_authz_write
- |svn_authz_recursive,
+ svn_authz_write,
&authz_access_granted,
r->pool);
if (svn_err)
Index: subversion/libsvn_repos/commit.c
===================================================================
--- subversion/libsvn_repos/commit.c (revision 1679144)
+++ subversion/libsvn_repos/commit.c (working copy)
@@ -327,9 +327,9 @@
size_t repos_url_len;
svn_repos_authz_access_t required;
- /* Copy requires recursive write access to the destination path
+ /* Copy requires write access to the destination path
and write access to the parent path. */
- required = svn_authz_write | (is_dir ? svn_authz_recursive : 0);
+ required = svn_authz_write;
SVN_ERR(check_authz(eb, full_path, eb->txn_root,
required, subpool));
SVN_ERR(check_authz(eb, pb->path, eb->txn_root,
Don't require recursive write access rights for copy targets.
* subversion/libsvn_repos/commit.c
(add_file_or_directory): Require only simple, non-recursive read access
to the copy target.
* subversion/mod_authz_svn/mod_authz_svn.c
(req_check_access): Same for COPY and MOVE targets.
