Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decision: Access Rule with acl:default does not imply acl:accessTo #193

Closed
Vinnl opened this issue Aug 17, 2020 · 27 comments
Closed

Decision: Access Rule with acl:default does not imply acl:accessTo #193

Vinnl opened this issue Aug 17, 2020 · 27 comments

Comments

@Vinnl
Copy link

Vinnl commented Aug 17, 2020

In other words, is it impossible to define an inheritable default that gives more access to a Container's descendants than to the Container itself?

Or with a code example: is this not a valid ACL Rule? (Syntax errors and prefixes notwithstanding.)

:ControlReadWriteDefault
    a acl:Authorization;
    acl:agentClass foaf:Agent;
    acl:default tes:;
    acl:mode acl:Control, acl:Read, acl:Write.

("Valid" meaning: it gives everyone Read, Write and Control access to children of the Container that do not have their own ACL, and there's no need for an acl:accessTo tes:; to do so.)

@nicolasmondada
Copy link

nicolasmondada commented Aug 18, 2020

Emmet and I agree this should not happen. You shoudn't be able to give more access to the descendants but I'm interested in the spec team opinion.

@csarven csarven mentioned this issue Aug 18, 2020
@bourgeoa
Copy link
Member

My own understanding is that default and accessTo are fully independent properties.
That means that your above example do not give any access rights on inheritance algorithm.

@csarven
Copy link
Member

csarven commented Aug 24, 2020

In order to address ambiguities and tighten the spec, this issue should be taken up when specifying shape of ACL: #169 .


I summarise some common knowledge:

An ACL document may have zero or more authorization policies where each policy uniquely is described based on the following dimensions:

  • acl:agent, acl:agentClass, and/or acl:agentGroup
  • acl:accessTo and/or acl:default
  • acl:mode

An authorization policy without a acl:mode entails no access.

An ACL document may have max one authorization policy with max one acl:default statement..
acl:default's range is a resource of container type, and its value is the container resource that is associated with the ACL document.

acl:default check is done in context of the ACL inheritance algorithm. If no acl:default is found in the effective container's ACL, no access is granted.

A container's ACL may have one authorization policy including both acl:accessTo and acl:default. The authorization policy will be used in different context.

A container's ACL may have two authorization policies where one only includes acl:accessTo, and the other authorization policy only includes acl:default. This effectively allows a container (via acl:accessTo) to have access privileges different than the contained resources (via acl:default) when the ACL inheritance algorithm is triggered.

Simple situations:

  • Append access to container

    • No access to contained resources (typical for profile inbox)
    • Read access to contained resources (same as above but may want to allow reading of created resource)
    • Remove access to container after resource created in container (allowing Read access to contained resource as above - to remove Read, remove acl:default or remove Read mode from authorization policy with acl:default)
  • Read access to container


Note https://solidproject.org/TR/protocol#deleting-resources :

To delete a resource, an acl:agent MUST have acl:Write privilege per the ACL inheritance algorithm on the resource and the containing container.

From another angle: if it is possible to have Write access on a resource without Write on its containing container, then clients will not be able to delete the contained resource. At least for delete operations, it entails that a contained resources can be partly controlled by authorization policies in ACL documented as per ACL inheritance algorithm. The *nix filesystem permissions works the same way.

In *nix filesystem, user is also granted to Read a file in a directory even if they don't have Read on directory.

$ mkdir foo/
$ echo "bar" > foo/bar
$ chmod -r foo/
$ cat foo/bar
bar
$ ls foo/
ls: cannot open directory 'foo/': Permission denied
$ chmod -w foo/
$ rm foo/bar
rm: cannot remove 'foo/bar': Permission denied
$ chmod +w foo/
$ rm foo/bar

So, at least, both behaviours mentioned above are compatible with *nix filesystem permissions - a significant compatibility IMO.

@bourgeoa
Copy link
Member

An ACL document may have max one authorization policy with max one acl:default statement.

Do I read this correctly : acl:default can only be used once in an ACL document.

And the following example is not valid :

<#read>
  a acl:authorization;
  acl:accessTo <./>;
  acl:default <./>;
  acl:agentGroup <any group URI>;
  acl:mode acl:Read.

<#ReadWriteControl>
  a acl:authorization;
  acl:accessTo <./>;
  acl:default <./>;
  acl:agentGroup <an other group URI>;
  acl:mode acl:Read, acl:Write, acl:Control.

@csarven
Copy link
Member

csarven commented Aug 24, 2020

@bourgeoa That's correct. As far as I'm aware, there is currently no information that would help to determine which of the authorization policies including acl:default would be applicable. It could of course be arbitrarily argued that it should be the first one when listed, at random, a union, or one with least number of access modes or whatever may be considered to be "safe", .. .and so on. Possibly even just halt altogether and deny access.

Simpler to restrict by allowing only max one acl:default per ACL document when an ACL is create or updated. We can indicate error handling.

@megoth
Copy link

megoth commented Aug 24, 2020

@bourgeoa That's correct. As far as I'm aware, there is currently no information that would help to determine which of the authorization policies including acl:default would be applicable. It could of course be arbitrarily argued that it should be the first one when listed, at random, a union, or one with least number of access modes or whatever may be considered to be "safe", .. .and so on. Possibly even just halt altogether and deny access.

Simpler to restrict by allowing only max one acl:default per ACL document when an ACL is create or updated. We can indicate error handling.

This isn't how I've understood how acl:default can be used. You can have multiple instances of acl:default for many reasons, most notably for various agents:

<#agentADefaultRead> a acl:authorization;
  acl:default <./>;
  acl:agent <Agent A WebID>;
  acl:mode acl:Read.

<#agentBDefaultAll> a acl:authorization;
  acl:default <./>;
  acl:agent <Agent B WebID>;
  acl:mode acl:Read, acl:Append, acl:Write, acl:Control.

But I think you can also have multiple acl:default for same agent as well; the server would check them all until any of them gives the requested access (or if all fails, deny access).

@csarven
Copy link
Member

csarven commented Aug 24, 2020

@megoth True! Thanks for clarifying that. I went along with the example and completely skipped over acl:agent, :agentClass, :agentGroup as a dimension. It still holds true if multiple authorization policies have the same agents listed (along with default). System either needs to prevent their creation/updates - making sure authorization policies are indeed unique, or alternatively, it needs to know what to do if/when it encounters such policies.

@csarven
Copy link
Member

csarven commented Aug 24, 2020

Updated #193 (comment) to indicate the dimensions that uniquely identifies an authorization policy. We can still edit this to frame it closer to what we like.

@bourgeoa
Copy link
Member

bourgeoa commented Aug 24, 2020

(edited : sorry I did not see @megoth above comments)
@csarven I think they could also both be true (the algorithm for default will mimic for accessTo) and when checking for an agent, look for agentGroup, then check for default in same authorization rule.

The above seems to be the actual way NSS does.
And when you create an ACL document with mashlib authorization pane default is added to all authorization rules.

ACL document created by mashlib authorization pane :

@prefix : <#>.
@prefix n0: <http://xmlns.com/foaf/0.1/>.
@prefix n1: <http://www.w3.org/ns/auth/acl#>.
@prefix test1: <./>.
@prefix c1: </profile/card#>.
@prefix c2: <https://bourgeoa.inrupt.net/profile/card#>.
@prefix c3: <https://bourgeoa.solid.community/profile/card#>.

:ControlReadWrite
    a n1:Authorization;
    n1:accessTo test1:;
    n1:agent c2:me, c1:me, c3:me;
    n1:default test1:;
    n1:mode n1:Control, n1:Read, n1:Write.
:Read
    a n1:Authorization;
    n1:accessTo test1:;
    n1:agentClass n0:Agent;
    n1:default test1:;
    n1:mode n1:Read.

@michielbdejong
Copy link
Contributor

From my understanding, I think the behaviour that's currently implemented in NSS, PSS and CSS and tested by the Solid test suite, is the correct one (i.e. acl:default and acl:accessTo are orthogonal), and that we should ask Inrupt if they can align the behaviour of https://pod-compat.inrupt.com with that.

Simple evidence of this is for instance https://github.com/solid/node-solid-server/blob/v5.6.4/default-templates/new-account/.acl#L20-L23: it's basically the way it's always been, and it probably doesn't help the eco-system if newer implementations of Solid fork away from that?

@acoburn
Copy link
Member

acoburn commented Feb 17, 2021

I'm not opposed to changing the behavior, but this statement is not compelling:

it's basically the way it's always been

Please reference the specification text the defines the correct behavior.

@michielbdejong
Copy link
Contributor

Fair enough! I don't know if it's really clear from the text of https://github.com/solid/web-access-control-spec#referring-to-resources and https://github.com/solid/web-access-control-spec#default-inherited-authorizations but I think it's at least implied from the phrase "not every document needs its own individual ACL resource" that if a document does have its own ACL resource, then you would use acl:accessTo and not acl:default.

@acoburn
Copy link
Member

acoburn commented Feb 17, 2021

"I think it's at least implied by the phrase" is also not compelling.

See the discussion above which contradicts the point you are making, particularly #193 (comment)

@michielbdejong
Copy link
Contributor

OK, marking this as disputed then. I'll skip the test for it until we reach consensus.

michielbdejong added a commit to solid-contrib/web-access-control-tests that referenced this issue Feb 23, 2021
@michielbdejong
Copy link
Contributor

michielbdejong commented Mar 4, 2021

Let's see if we can find a way out of this impasse:
Should the question in the title of this issue be answered with 'yes' or with 'no'?

The facts:

  • NSS, PSS, CSS answer it with 'yes'.
  • ESS answers it with 'no'.
  • The spec text in https://github.com/solid/web-access-control-spec#default-inherited-authorizations can be read with both 'yes' and 'no' in mind.
  • We need to choose, we can't have a situation where some servers behave differently from others.
  • If we say 'yes', we make WAC more powerful. This could be a good thing, or a bad thing.
  • Sarven does not take a 'yes' or 'no' stance, but does point out that there are use cases that are possible with 'yes' but not with 'no'. So I think we agree that 'yes' is more powerful - it gives the pod owner (or the Solid app developer, depending how you look at it) more options, and these could be useful in some situations. He also mentions some situations where both systems would work equally well.
  • I do (hereby) take a stance and propose we answer with 'yes'. I think Sarven's example about the inbox is compelling, but I can also think of more use cases. You could also for instance create a container that is not itself publicly readable, but its contents are. This serves UC 2.9.3 of the authorization panel, for instance.
  • We have worked with "yes" historically (specifically in NSS and in Solid OS, which still do, but also Trellis, historically), and I think this has a weight, although of course not in itself compelling enough to base a decision on if we currently disagree with it, as Aaron rightly commented.
  • Emmet and Nicolas answer it with 'no', IIUC this is because they feel the extra options could be a "footgun".

Option 'yes'

Then we:

  • make this clear in the spec.
  • We unskip the tests for this and keep them as testing the 'yes' behaviour.
  • We ask the ESS team to change the behaviour of their server. In the process, existing ACL docs that only have acl:default should be updated to add acl:accessTo so their behaviour stays as intended.
  • We add warnings to app developers saying "careful, you are about to give more access to the descendants, are you sure?",
  • In end-user facing GUIs we could just hide this "footgun" possibility altogether, or hide it behind some "Advanced options!" warning.

Option 'no'

Then we:

  • make this clear in the spec.
  • We unskip the tests for this and invert them so they test the 'no' behaviour.
  • We ask the NSS, PSS, and CSS teams to change the behaviour of their server. Wherever a container had less access than its descendants, the safe option is to reduce access for the descendants, to match what the container itself has.
  • Apps and user expectations will break if they currently gave more access to descendants, since we are removing that feature from Solid and it will no longer be available. We explain why we chose to do this, and hope everybody will understand.

Please vote. Also, if there are more arguments to vote 'no' other than the "it's a footgun" argument which I hope I'm attributing correctly to the proponents of 'no', then please detail these - if they are compelling, the 'yes' voters might be convinced to switch!

@michielbdejong
Copy link
Contributor

@timbl @justinwb @dmitrizagidulin @kjetilk @csarven @RubenVerborgh you are in the editorial team, your voice is obviously very welcome in this discussion.

@TallTed
Copy link
Contributor

TallTed commented Mar 4, 2021

@michielbdejong -- I'm pretty sure you meant for one of the "Option 'yes'" headers above to be "Option 'no'" (I think the second?)...

@michielbdejong
Copy link
Contributor

Thanks! Fixed.

@michielbdejong
Copy link
Contributor

Just chatted with @csarven about this. In line with what he said in #193 (comment). The description at http://www.w3.org/ns/auth/acl#default says:

        If a resource has no ACL file (it is 404),
        then access to the resource if given by the ACL of the immediately
        containing directory, or failing that (404) the ACL of the recursively next
        containing directory which has an ACL file.
        Within that ACL file,
        any Authentication which has that directory as its acl:default applies to the
        resource. (The highest directory must have an ACL file.)

So it starts with "if" and there is no "else". That means that acl:default in the resource's own ACL doc has no assigned meaning. So going by that logic, the answer would be 'yes'.

Also https://github.com/solid/web-access-control-spec#referring-to-resources says "The acl:accessTo predicate specifies which resources you're giving access to". It doesn't say "You can also use acl:default for this if you prefer".

@michielbdejong
Copy link
Contributor

@acoburn what do you think of this conclusion, can we find consensus on it?

@timbl timbl changed the title Does an Access Rule with acl:default also require an acl:accessTo? Does an Access Rule with acl:default also imply an acl:accessTo? Mar 17, 2021
@timbl
Copy link
Contributor

timbl commented Mar 17, 2021

Changed the title to reflect what @Vinnl actually meant by the text, I think.

@timbl
Copy link
Contributor

timbl commented Mar 17, 2021

I feel very strongly that the answer is "No".

This is RDF. This is logic. The default and accessTo properties have well defined separate meanings. I see absolutely no reason for saying that one should imply the other.

When something is simple and orthogonal and been so for many years, so we have to defend that against a proposal to make it more complicated and less expressive and different?

@timbl
Copy link
Contributor

timbl commented Mar 17, 2021

It also might need pointing out that the access function does not care if there is a bunch or Authentication nodes or one so long as there is the logical statement made which authorizes each mode needed.
I was asked to put in some pseudocode for the function, hough it is a bit off topic.

Suppose the Resource R in question is in container C.
To do the operation requires a set of modes. Then the operation is allowed iff

For EVERY mode M needed,

  • there is SOME node N for which all of:
    • N mode M
    • N type Authentication
    • N (accessTo R) OR (N default C)
    • N applies to the agent directly OR through Group membership, OR through Class membership

To be more explicit, you could say:
For EVERY mode M needed,

  • there is some node N (no matter what other nodes there are) for which all of:
    • N mode M (no matter what other modes it may have)
    • N type Authentication (no matter what types it may have)
    • N (accessTo R) OR (N default C) (no matter what other resources and containers are also mentioned)
    • N applies to the agent directly OR through Group membership, OR through Class membership (no matter what other agents may also be directly or indirectly also mentioned)

Do NOT assume the modes will be on the same node N. Different modes may be supplied by quite different forms -- say one through default and one through accessTo, one via the agent, one via a group and another by a class.

@Vinnl
Copy link
Author

Vinnl commented Mar 18, 2021

@michielbdejong Just a heads-up: the title does now more accurately reflect the description I wrote in the original comment, but now the "yes" and "no" in your comment (referring to "the question in the title of this issue") are in reverse :)

@michielbdejong
Copy link
Contributor

@acoburn was this fixed in ESS v1.1?

@csarven
Copy link
Member

csarven commented Jul 1, 2021

Thanks for this issue and discussion. Closing this issue as consensus is deemed to be captured in WAC Editor's Draft: https://solid.github.io/web-access-control-spec/ . See #authorization-conformance #authorization-matching . Please use https://github.com/solid/web-access-control-spec for future discussion.

@csarven csarven closed this as completed Jul 1, 2021
@RubenVerborgh RubenVerborgh changed the title Does an Access Rule with acl:default also imply an acl:accessTo? Decision: Access Rule with acl:default does not imply acl:accessTo Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants