[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: authz changes between 1.9 and 1.10

From: Branko Čibej <brane_at_apache.org>
Date: Sun, 2 Dec 2018 08:25:03 +0100

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

This is an archived mail posted to the Subversion Dev mailing list.