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

Re: Recursive operations and authz

From: Branko ─îibej <brane_at_wandisco.com>
Date: Wed, 13 May 2015 16:35:26 +0200

On 13 May 2015 at 15:24, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> On 05/13/2015 02:21 AM, Stefan Fuhrmann wrote:
>> 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.
>
> [Details snipped. Welcome to 2004, where CollabNet ran into the same
> issues with the proprietary regexp-based authz implementation we shipped
> in our enterprise products.]
>
>> 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).
>
> Well, the use-case being broken here is kinda the obvious one: I
> shouldn't have permission to create/delete some path /foo/bar (it's a
> system-critical file that shouldn't go away, or a password-bearing file
> path blocked by the VC system because devs keep stupidly committing
> them, or...), but you've provided and end-around for me to do so. :-)
> In the copy and move scenarios, you're allowing me to create a path I
> can't later delete or further modify. Now, while this is a very handy
> semantic for tag creation[1], it's also something an administrator
> should be able to make a conscious decision about, not just an
> unfortunate side-effect of an effort to make the code faster.

This proposal really has nothing to do with performance; it's about
intended/consistent authz semantics, which we never really bothered to
define, let alone prove. IMO, the fact that you can have authz rules
that allow you to create a directory but not its children is
unintended. I bet no-one thought about it during the initial
development of mod_authz_svn.

> Subversion's path-based authz subsystem is about paths, not nodes as we
> devs think of them. Our users have generally not been asked to consider
> a copy, addition, or deletion of an item to be changes made to the
> parent of that item. So the changes you're suggesting are not merely
> ones of functionality, but ones of philosophy.

Interesting point, but I disagree: we're still talking about authz in
terms of paths, the proposed change just removes the restriction that
you cannot copy something to a location where you can create
something. Currently create requires non-recursive write access to the
target, but copy (and move) require recursive write access. IMO,
that's inconsistent and unintuitive, something I'd classify as a
design bug rather than a change in philosophy.

> I lean towards thinking
> that falls outside the scope of acceptable changes in behavior in the
> 1.x line *unless* you can find a way to, via configuration, allow
> administrators to explicitly toggle this new paradigm.

Assuming we agree that it's not a new paradigm, would proper
documentation, as I proposed, be sufficient?

> [1] Along with regexp access rules, CollabNet has for many years
> supported this tag creation use-case by splitting the "write" permission
> into multiple ones: "add", "copy", "delete", and "modify". Something
> Subversion should consider?

That's what the authzperf branch is for, indeed. Hopefully many of
these things will land in 1.10.

-- Brane
Received on 2015-05-13 16:35:43 CEST

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