On 08.09.2018 11:17, Stefan Fuhrmann wrote:
> These are the guiding principles for the 1.10 authz design:
>
> (1) ACLs are only evaluated on a per-user bases; ACLs that
> don't mention this user (or any of their groups) are ignored.
> Rationale: We don't want to explicitly repeat inherited access
> specs that don't change for the respective path / section.
This is not entirely true, as seen in the fix for SVN-4793. If a user is
"not mentioned" in an inverted selector, those rights do propagate to
the global level. For example:
[groups]
readers = foo, bar
[/]
~@readers = rw
@readers = r
In this case 'user' has read-write access to '[/]' even though she's not
mentioned anywhere in the authz file or the specific ACL for '[/]'.
>> In 1.9 any repeat acl lines that were the exact same match, such as:
>>
>> [/some/path]
>> user = rw
>> user = r
>>
>> resulted in the last line overriding all the other lines, so user=r in
>> the example above. In 1.10 the lines combine, so user=rw in the example
>> above. Is this a bug in 1.10, or a bug in 1.9 that is fixed in 1.10, or
>> an acceptable behaviour change?
> Ouch. That is a bad one and an oversight in the design - I think.
>
> According to (3), 1.9 behaves correctly. I guess we consider it
> an unordered collection in 1.10 and then a union is the only thing
> that approximates intent when a user is a member of different
> groups with different access rights. Strict ordering becomes
> very useful when assigning rights to groups:
>
> [/some/path]
> @Users = rw
> @BadUsers = r
We already have strict ordering within an ACL (authz_acl_t in
libsvn_repos/authz.h):
/* All other user- or group-specific access rights.
Aliases are replaced with their definitions, rules for the same
user or group are merged. */
apr_array_header_t *user_access;
The "merge" semantics was intentional; if we decide it's a bug (and I
think it is), it's fairly easy to change. I would lean in the direction
of making repeating the same access entry selection a hard error at
parsing time. This requires changes to the merge semantics implemented
in add_access_entry() and merge_alias_ace() in
libsvn_repos/authz_parse.c. The nice part is that it would catch errors
like this:
[aliases]
afoo = foo
abar = bar
[/]
&afoo = rw
foo = r
~&abar = rw
~bar = r
With the current implementation we translate the ACL to:
[/]
foo = rw
foo = r
~bar = rw
~bar = r
and even with strict ordering I'd say this is a bug and not intentional.
> I guess the fix in 1.10 is simple but will change 1.10 behavior.
> My proposal here:
>
> * apply replacement semantics here as in 1.9
> * error out / warn on repeated lines with the same user or group
> (this is hardly ever intentional)
^^^ this
> * provide a script / tool that takes 1.10 authz and checks for
> changed behavior ("r" after "rw" rules etc.)
>
> The last one is a bit of work but would be really handy.
The remaining ambiguity that I would prefer _not_ to resolve at parsing
time is this:
[groups]
@readers = foo, bar, user
@writers = baz, quz, user
[/]
@writers = rw
@readers = r
With strict ordering and replacement semantics 'user' would get
read-only rights. But that doesn't seem right; surely if someone is a
member of both 'readers' and 'writers' groups, they should get merged
access rights of both?
Note that the current behaviour is to merge.
-- Brane
Received on 2018-12-02 08:25:14 CET