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

Edge cases require all implementations to couple authorization and storage #97

Closed
RubenVerborgh opened this issue Aug 16, 2021 · 36 comments
Assignees

Comments

@RubenVerborgh
Copy link
Contributor

RubenVerborgh commented Aug 16, 2021

No description provided.

@csarven
Copy link
Member

csarven commented Aug 16, 2021

An agent needs Read on C/R for a server to return 200 (when resource exists) or 404 (when resource does not exist). How else are typical 200 or 404 are returned?

If agent does not have Read on C/R, the server returns 403. It doesn't have to do anything further. That'd be a conforming implementation.

If however, a server wants to return a more specific/useful code by taking in other information into account, eg. can the agent actually know C/R exists by viewing C/ if they have Read on C/? Those servers wanting to do more would still be conformant if they returned 404 (IMO). That's one of the reasons why there is a difference between the codes. solid/specification#288 (comment) mentions they are not fixed for various reasons.


Don't we have some existing issues/discussions in Protocol on locking? Happy to revise the text in WAC so that it doesn't necessarily mean that WAC has to check with the system for each container or whatever. I'd expect the system to only query (like ASK) for Authorization rules that's relevant as opposed to for each path segment eg. if /a/ exists but /a/a/ doesn't exist, don't need to work from the lowest in path to highest (towards root).

@justinwb
Copy link
Member

I believe the ideal situation for scalability purposes would be as follows:

  • WAC can be implemented as a layer that either:

    • returns 403 OR
    • sends the request through to the (LDP) backend, which then generates the status code (which is not 403)
  • This means that, to make a decision, WAC only needs as input:

    • the request
    • the ACL state
  • Specifically, WAC does not need any non-ACL Pod document state as input.

+1 - I believe that this decoupling is essential to scale and to good implementation design.

Also, would like to mention the value of having explicit modes for create / delete on a container in this context, as detailed in these authorization use cases (and associated requirements):

Having this added specificity in permissions would allow the authorization subsystem to make better and more accurate choices about what is does / doesn't pass through, while at the same time applying principle of least privilege to operations that could/would update the non-server-managed graph of the container.

@kjetilk
Copy link
Member

kjetilk commented Aug 17, 2021

I am very sympathetic to the goal of this issue. I will just record an opinion that I also voiced in the Solid Editor's meeting today:

I had a design for an auth proxy that could sit in front of the rest of the interaction, but in my design, I had an optimization that would record creates and deletions with the intention of keeping track of the existence of a resource (specifically into an in-memory trie). Originally, I didn't have this in mind, but simply to make it very cheap to reject a request with a 404 or 410, if the resource did not exist, as I see this as a protection against a DoS attack that seeks to overwhelm the relatively expensive auth operations on non-existent resources. With this design, the above wouldn't be so much of a concern. Then, I feel that this is something that should be done anyway, and so, I don't see quite the importance of this issue.

Nevertheless, I acknowledge the concern, and that more access modes would be helpful, and that it should be something that we consider. Urgently, even.

@TallTed

This comment has been minimized.

@csarven
Copy link
Member

csarven commented Sep 5, 2021

The tables in that comment are (obviously) populated from the point of the Protocol, not WAC's response codes.

PUT- and PATCH-to-create have the same requirements. I brought back the C/ column to the PATCH table and added the note: solid/specification#14 (comment) . I've also added a row including DELETE + INSERT payload that we should review:

|-|Read,Write|DELETE+INSERT|false|?|?|

"false" if no match for DELETE.


WAC ED says that server manages the association between resources and ACL resources so that applicable Authorization rules can be located (after effective-acl-resource) and evaluated. Let's pick on that: the backend, the WAC system, or something else needs to track that association. Conceptually similar to the "auth proxy" that Kjetil mentioned above but without tracking creates and deletes.

https://github.com/solid/web-access-control-tests/issues/41#issuecomment-898295543 :

Typically there is no ACL resource for a non-existing resource, but the WAC spec doesn't prohibit that. (If there is an ACL resource for a non-existing resource, server would apply that ACL resource and checks for accessTo).

I'm curious to know how Solid servers + WAC implementations currently determine the effective-acl-resource.


When an operation requests to create a resource as a member of a container resource

How can the server know an HTTP request strictly entails a Create operation? Here are some examples (there may be others):

Request Operation
POST /C/ Create
PUT /C/R + If-None-Match Create, Read
PATCH /C/R + If-None-Match + INSERT in message body Create, Read

Edit: See table WIP at #85 (comment) for reference.


A WAC system cannot tell for a given PUT or PATCH request whether it is a creation by looking at the request itself. It has to go to the backend and check whether that resource exists.

WAC doesn't need to. Server does (as per tables above). WAC is asked about [2] and [3]. That can be one or more queries.

WAC can be implemented as a layer that either:
returns 403 OR

This means that, to make a decision, WAC only needs as input:
the request

Not strictly. For example - just to illustrate an idea for a possible implementation - the backend can send data about the collection hierarchy (or however collection hierarchy is determined) and the associated ACL resources. The WAC system somehow needs to know how a resource and its containers are connected, whether it is for example informed about slash semantics, checks for reverse relation from resource to its container, or something else. It also needs to be informed about (or keeps track of it) how resources and ACL resources are associated, which is required to determine the effective-acl-resource.

For PUT /foo/bar/baz, WAC in context of Solid will/can get to know the following paths and the associated ACL resources (if any):

/
/foo/
/foo/bar/
/foo/bar/baz

/.acl
/foo/.acl
/foo/bar/.acl
/foo/bar/baz.acl

WAC goes through the authorization-process, responds to the backend with allowed or denied:

For PUT, it checks for Authorization rule with Write access on /foo/bar/baz and Append (or Write) access on closest container (heading towards the root container) with an associated ACL resource that has an Authorization rule with said permission.

If allowed, backend determines if resource state is to be created or replaced, as well as any intermediate containers that needs to be created.

@csarven
Copy link
Member

csarven commented Sep 5, 2021

Again, the final status codes are determined by the server. For GET /C/R, the backend must:

1: Check WAC for Read on /C/R.

1a: If there is a matching Authorization rule (=server can check storage):
1ai: If /C/R exists, return 200, otherwise return 404.

1b: If there is no matching Authorization rule (=server does not need to check storage):
1bi: Return 403.

The backend may consider checking the following (in the same query with 1 above) as an alternative to 1b above:

2: Check WAC for Read on /C/:

2a: If there is a matching Authorization rule (=server does not need to check storage):
2ai: Return 404.

2b: If there is no matching Authorization rule (=server does not need to check storage):
2bi: Return 403.

Single query (snippet) below -- you'll have to excuse me as I'm probably getting the query wrong... it is Sunday night and I'm having sage tea, my own crop, fyi:

SELECT ?authorizationA ?authorizationB
WHERE {
  ?authorizationA
    acl:mode acl:Read ;
    acl:accessTo </C/R> ;
    acl:agent foo:#bar .

  OPTIONAL {
    ?authorizationB
      acl:mode acl:Read ;
      acl:accessTo </C/> ;
      acl:agent foo:#bar .
  }
}

Before I think through the other table rows, does the above bring us closer to a possible solution/perspective?

I said one or more queries because I think a single ASK or a SELECT query (with desired cols, rows) will do, but since it is an implementation detail, multiple ASKs or SELECTs can also work.

Marked with "Create?" because at some point the storage needs to be checked. It is not on WAC to know if resource exists. While I agree on the architecture to check access control before visiting the storage, I don't think an implementation choosing or perhaps even needing to visit the storage before and during with multiple trips makes it wrong - probably undesirable, inefficient, and things can potentially go wrong but it is still an implementation detail as far as the specs are concerned. Based on my understanding... but happy to be corrected on that or if there is an assumption somewhere that I'm not considering at the moment.

@csarven
Copy link
Member

csarven commented Sep 5, 2021

My mental model is a pipeline in which a request goes through the sequential stages:
So once a request has left WAC stage, it cannot go back.

I'm saying that's a fine implementation detail. I don't quite see how the spec prevents that possibility.

You seem to be assuming an architecture in which some subsystem is asking questions to an ACL store and an LDP store.
So the system you call "WAC" seems to be an ACL store to which you can ask questions; the system I call "WAC" is a processing stage which makes a decision: 401, 403, or PASS.

Can you break it down for me as to what's wrong with the pseudocode I wrote above? I thought it showed exactly what you wanted. Check if required Authorization rule exists (=exit WAC), and check storage only when need to (=before HTTP response).

For the WAC subsystem to decide whether it should return 403, it first needs to check whether the LDP system would have given a 404. We're instead interested to have a WAC subsystem whose decision for 403 is not influenced by LDP.

I think that's a misinterpretation of the pseudocode above.

@acoburn
Copy link
Member

acoburn commented Sep 5, 2021

My mental model is a pipeline in which a request goes through the sequential stages:
WAC: either returns 401/403 (skipping the next stage) or passes on to the next stage
LDP: serves documents and returns status codes such as 2xx, 400, 404

I approach this slightly differently.

The key point I would make is that the WAC layer would not be responsible for producing a 403. (it may generate a 401)

Rather, the WAC layer would be responsible only for identifying what operations a particular agent can perform on a particular resource. This stage might produce an array: [read, write, append]. But these do not need to correspond to the WAC modes. More likely they would be [read, create, write, append, delete] or [read, create, append] or a null set: [], indicating that the agent has no access. Either way, this is all internal impl detail: only the Pod server needs to understand it. Importantly, this is where you need to define things like: can the agent create a new resource (e.g. create) or merely add triples to an existing resource (e.g. append). Can the agent delete the resource?

I would emphasize that the data at this stage is purely implementation detail.

Once that data arrives at the pod server / HTTP layer, you can inspect conditional headers (If-Match, etc) or whether the intended resource exists or not (is this a create or update operation?). That is when the 403 would be returned on error or 200/201/204/etc on success.

This structure will work with a completely externalized authorization server or with a single server that uses layers, as described above.

@kjetilk
Copy link
Member

kjetilk commented Sep 6, 2021

I think @RubenVerborgh and I share a mental model on this, since we worked this out in the early F2F while designing a NSS replacement. I think your design is very interesting, @acoburn , the reason why I/we (or whoever, don't remember) didn't further entertain that model was that it was perceived as a heavier than the approach taken by acl-check.js, which would return a boolean "authorized or not".

Of course, there has always been some tension for our idea too, in particular around PATCH where the request body would have to be parsed. In your design, @acoburn , the authz evaluation could sit entirely within the authz component, and still let subsequent layers return the actual status codes.

In practical terms, how much of a burden do you think this would put on other implementers, @acoburn ?

@acoburn
Copy link
Member

acoburn commented Sep 6, 2021

In practical terms, how much of a burden do you think this would put on other implementers?

@kjetilk it is hard to say unequivocally whether a certain pattern puts too much burden on implementers. I would say that the model described by you and @RubenVerborgh has the advantage of being very straight-forward (I've implemented variations on that model numerous times, and it is certainly the one that is more obvious).

One additional consideration, though, is the requirement for a WAC-Allow response header. For that, a server will need to know the modes for a given user (and the public user), which may require additional checks on the ACL layer (after a boolean access/no-access has been determined). By computing that access data once and then passing it down to the Pod layer, a server can avoid that additional check. Of course, if the impl translates ACL vocabulary terms into an internal data structure (e.g. [read, create, append]) that would need to be converted back to the standard WAC-Allow terms.

In summary, I would say that there are advantages and disadvantages to both approaches. In my comment, I was mostly trying to point out that it is possible to construct a system where multiple round-trips are not required. Whether that model is desirable for a given implementation is very much dependent on context.

@csarven
Copy link
Member

csarven commented Sep 6, 2021

One or both of us is not understanding the other. Let me re-quote from earlier because I think it is still a misunderstanding and I want us to be on the same page on this:

For the WAC subsystem to decide whether it should return 403, it first needs to check whether the LDP system would have given a 404.

No. WAC strictly decides what's allowed and/or denied based on what's being queried. When the system that gets an answer from WAC, it proceeds by deciding whether the storage needs to be checked or not, and then decides on the status code.

1: Check WAC for Read on /C/R.

Done with the Authorization layer.

1a: If there is a matching Authorization rule (=server can check storage):

Access is allowed so use the storage.

1ai: If /C/R exists, return 200, otherwise return 404.

Done.

1b: If there is no matching Authorization rule (=server does not need to check storage):

Access is denied. There is no need to use the storage.

1bi: Return 403.

Done.


Now, let me respond to this because alternatively a query to WAC can check for permissions on /C/R and /C/ (that was the rough SELECT example above). This is entirely optional. It is just a way to be smart/realistic about the response code.

2: Check WAC for Read on /C/:

Done with the Authorization layer.

2a: If there is a matching Authorization rule (=server does not need to check storage):

Access is allowed i.e., /C/R can be seen in /C/. There is no need to use the storage.

2ai: Return 404.

Done.

2b: If there is no matching Authorization rule (=server does not need to check storage):

Access is denied i.e., /C/ can not be read. There is no need to use the storage.

2bi: Return 403.

Done.


Is there anything you disagree with the above or that's not clear?

@kjetilk
Copy link
Member

kjetilk commented Sep 8, 2021

You know, I think this is whiteboard matter stuff... We aren't communicating well enough because our medium doesn't have the emotional and conversational bandwidth we need. Everyone should keep that in mind, and so, we try our best.

So, that being said, I just wonder if the problem arises simply by that there is a Create operation? I prefer to think in terms of operations, and then there are access modes that allow certain operations to be performed.

Take the case of a PUT, which may be either a Create or a Modify operation. You can ensure that it is a Create operation by using the If-None-Match header, but you don't have to, so in principle, when the server sees a PUT it can be either, you can't tell for sure unless you know the state of the target resource.

Now, say that we have a create access mode, and the client sends a PUT request and is authorized with that create access mode, but not modify. Then, the authz layer would have to know the state of the resource to determine the result.

I very much like a layered architecture, but I thought about it in slightly different terms. To me, it was OK to have the layers have bi-directional communication if the communication happened in the normal request-response cycle (thus calling it a proxy). I had another restriction though, and that was that the request body should only be parsed once (so the PATCH "handler" would have to deal with authz too).

If you allow for the response to be seen by the top layers, then you can record the state of the resource. Not only whether it exists, but you can also have mtime and etags recorded in the top layer, lock a resource representation to deal with concurrent requests and so on.

So, that seems to me as an alternative architecture.

If I was writing a server now, I would probably adopt @acoburn 's idea of having the second layer determine the allowed access modes, and let that communicate them down. At the bottom, I would then code based on operations and have methods that checked the access modes for each operation. Then, the lowermost layer would not actually have to evaluate the authorization that was done in the second layer, but it would need to know them, but that seems like a small price to pay.

@csarven
Copy link
Member

csarven commented Sep 17, 2021

Ruben, is the architecture you're referring to reserves 403 to the authorization layer?

What's (currently) included in that processing stage besides an access control system?

Would any feature that could potentially conclude 403 is to be included in the authorization layer?


"403,404" (or even "200,201") is discussed from the point of the Protocol: 403 is a ~MUST whereas 404 would be a ~MAY.

403 could be decided based on the evaluation of an access control system but it is not necessarily confined to it (or a particular architecture's authorization layer). For example, the protocol or even a particular server may disallow allocating certain URIs or pattern without needing to check actor's permissions.

I think this concern is not specific to WAC, or any other access control or capability-based security -- unless they wilfully violate or intended to override protocol's response codes.

I propose that we consider discussing this issue from the point of the solid/specification. Related: solid/specification#288 , solid/specification#146 , solid/specification#219 (comment) .


I'm sensing that you want the server to be able to return a 404 but that would mean that the 403 possibility is still there in case when agent doesn't have Read on container. But the architecture seemed to reserve 403 to the authorization layer - after all, if it PASSed the authorization layer, 403 should not arise from the following stages. If 403 is still possible in stages after (or even before) the authorization layer, then it can be used failing the check for 404 - this is what I've been trying to communicate in that, if you want the 404 possibility, you need to check storage [1].

Is this an accurate summary?

One way out of this is to not bother with the 404 possibility obviously, but let's continue..


ensure that the request itself unambiguously states whether it is a creation request (that would be a change to the Solid protocol)

Right. We know for some cases but not all - the table in #85 (comment) distinguishes some.

I do not think that 1 is desirable, so that seems to leave us with 2.

Right, we are not going in that direction at this point in time. FWIW, alternative approaches, e.g., POST with Activity Streams 2.0 activities such as Create, Delete, Add, Remove.

make the status codes for "Create?" cases the same, independently of whether the answer turns out to be "yes" or "no"

That would prohibit implementations capable of enhancing their product by providing specific/useful codes.


I find it strange that the differences in status codes are potentially an issue for some architectures - as you seem to be suggesting - but what's in the table is not above and beyond vanilla RFC 7231.


an acceptable solution for 2 could be that either a) Append on the container is required for both creation and update

For creation, that's possible (having Append (or Write) on /C/ is required in order to create /C/R, so that's possible.) However, for updating, I don't think Append on /C/ should be required if only Append is desired on /C/R.

or b) Append on the container is required neither for creation nor for update.

Append on /C/ is useful to allow processing operations entailing POST (as per the resource semantics of C/) and to have the server to allocate the URI of a resource (creating) that's to be a member of /C/. As for updating /C/R, possible to just Append on it.


Re Kjetil's:

Now, say that we have a create access mode, and the client sends a PUT request and is authorized with that create access mode, but not modify. Then, the authz layer would have to know the state of the resource to determine the result.

Exactly. This also shows why throwing in new access modes doesn't address the concern at hand.

@kjetilk
Copy link
Member

kjetilk commented Sep 21, 2021

@RubenVerborgh , I agree with you in principle but not in detail.

I agree that it is important that the spec makes possible architectures with good quality attributes and encourages implementations of such.

However, when you say that:

But the argument above shows that we cannot. Because of the currently listed status code choices, all current and future implementations will either have scalability challenges (NSS) or force storage developers to know about access control (ESS). We've proven above that there is no other way.

You have proven no such thing. You haven't responded to my proposal of a reverse proxy, which would resolve the issue, and would be minor modification both to the architecture and implementation.

I might be old, but I would seriously never consider going into production without a reverse proxy, even on my personal box. It appears that is also best practice for nodejs and express. It also looks simple to get started. I just don't see why you would resist this idea.

So, you can keep the stacked architecture and the reductive processing, you just need to have a reverse proxy that tracks the existence of a resource, and let that proxy do the first reductive step (i.e. return a 404 (should also do 304 etc)). It will also communicate whether the resource existed for requests like PUT. The rest remains unchanged. With this, you don't need to roundtrip to the backend.

The key problem with your current architecture is that it cannot support create operations with the MUSTs of RFC7231, and it really has to.

I suggest that you make a simple node-based reverse proxy. We should also implement a library in C that can be integrated in stuff like nginx, HAproxy or Varnish at some point. The reverse proxy could also be offloaded to a CDN. I think the benefits of this architectural change is great, much greater than the downsides.

BTW, I just found me architecture brain dump from waaaaay, back.
next-gen-solid-server

The "Resource list cache" is that architectural component. I still think it is required.

@kjetilk
Copy link
Member

kjetilk commented Sep 22, 2021

I would seriously never consider going into production without a reverse proxy

Me neither; CSS has explicit support for this.

you just need to have a reverse proxy that tracks the existence of a resource

This assumes that there would be only one interface to the data, quod non.

Well, obviously, if you add interfaces, you should be prepared the tackle the consequences, including authentication, authorization,
concurrency control, etc. I don't see it as appropriate to tackle the requirements of all possible different interfaces, even though I certainly see them coming in the IoT space, and where many of them aren't even neatly layered.

Examples where this breaks:

I would not say that, I would say "Examples where I may choose to break this". Now, it can be done, but what if we add end-to-end encryption to the protocol, which is certainly an important feature?

That said, it isn't usually hard:

* file-based backend, files are added directly to the filesystem
* sensor-based backend, new observations are created by the sensor

Then, the implementation will have to notify the front proxy.

* the server has LDP, SPARQL and GraphQL interfaces
* the server exposes the same data through multiple virtual folders via LDP

They should be going through the same proxy on the way out.

So I rest my case that we have to choose between scalability challenges or a ACL-aware storage, which is a real problem.
Happy to put reverse proxies on top of pod servers, but that is an orthogonal issue to me.

I'm sorry that I wasn't present when you started writing CSS, because I could have commented at an early stage, I understand that you are pretty deeply tied to the architecture now. However, I do not think you have made a good case here, it is very clear that this is not a matter of what is possible or not, when it comes to this, it is a matter of trade-offs, and I think you have made a wrong trade-off, as one of the key reasons for using a reductive model is the ensure that you respond in the the cheapest way first. You do the opposite, you first do the auth, and so make the server susceptible to a trivial DoS attack. An attacker can simply flood the server with requests with random target URLs, and it will end up doing authz for all of them. That has to be prevented, trust me, I've been there, done that, and not been rewarded a t-shirt for it (well, it wasn't me, but it was my team). Returning a 404 cheaply is really your first line of defense. Having to go to storage for that will have very bad consequences. You'd have to mitigate with fail2ban or something like that on the front-end from very early on.

What is not a trade-off however, is that you can't support PUT properly.

@csarven
Copy link
Member

csarven commented Sep 23, 2021

WAC ED says that server manages the association between resources and ACL resources so that applicable Authorization rules can be located (after effective-acl-resource) and evaluated.

Isn't the existence of a resource known?

@csarven
Copy link
Member

csarven commented Sep 23, 2021

Ruben, we are trying to get answers just as you're! When I ask a simple yes or no question, I'd like a simple response as well. It is not always obvious what the answer is by working through a long response.

In any case, I've been revisiting some of the fundamentals. So, yes, I'm looking into eliminating the differences. I'll have to get back to you on a longer response, but I believe the row for GET can be 403 only. (Even though I mentioned several times that the 404 path is completely optional.) The reasoning is that only the request target is relevant since the method is safe and idempotent. The resource state is not relevant.

I'm looking into the other requests/rows.

@csarven
Copy link
Member

csarven commented Sep 24, 2021

So is it "known": you'd have to access that system to make it known. That access might be cheap or expensive. But you need to do it.

Right, wherever that information is. My main point was that the knowledge of the association is necessary to apply the Authorization rules. And knowing that association also reveals the existence of a resource.

@csarven
Copy link
Member

csarven commented Sep 24, 2021

What's determining the effective-acl-resource?

@csarven
Copy link
Member

csarven commented Sep 24, 2021

We need to move this issue to solid/specification. I'm okay to keep it here as we reach consensus but with the understanding that it is not a WAC bug. Any objections or preferences?

@csarven csarven removed the bug label Sep 24, 2021
@dmitrizagidulin
Copy link
Member

@RubenVerborgh - thanks for detailed writeup of the problem. And, I agree with you, getting the WAC spec to a point where it can be handled via the reductive architecture, would be very advantageous.

I'd love to hear from @kjetilk and @csarven with regards to - so what's downside of refactoring WAC slightly (to remove those 3 exceptions)? Is it just the general friction of making a substantial change to the protocol?

If I'm understanding the 3 cases correctly, we'd get a potentially serious boost in implementation simplicity and performance, in exchange for only minor protocol-change-pains.

@kjetilk
Copy link
Member

kjetilk commented Sep 27, 2021

@dmitrizagidulin I have decided to not comment any further on this until I have had a more high bandwidth conversation with @RubenVerborgh . The short story is that I do not think this is a WAC issue at all, the impact on future Solid is great, and that we need to take into account what kind of environment we are deploying into.

@elf-pavlik
Copy link
Member

elf-pavlik commented Sep 29, 2021

A WAC system cannot tell for a given PUT or PATCH request whether it is a creation by looking at the request itself.

If we require that create always uses If-None-Match: * it would be possible, just based on that header, to tell if it is creation.

@kjetilk
Copy link
Member

kjetilk commented Sep 29, 2021

A WAC system cannot tell for a given PUT or PATCH request whether it is a creation by looking at the request itself.

If we require that create always uses If-None-Match: * it would be possible, just based on that header, to tell if it is creattion.

Indeed. Do we want to do that? To me, it seems like it puts a big expectation on a client, but then again, we currently have just a write and append, so if we do that, it could be very helpful.

@csarven
Copy link
Member

csarven commented Sep 30, 2021

Revisiting the assumptions. Fun times.

Wrote this earlier ( #97 (comment) ) which I think clarifies the situation we are in:

https://github.com/solid/web-access-control-tests/issues/41#issuecomment-898295543

Typically there is no ACL resource for a non-existing resource, but the WAC spec doesn't prohibit that. (If there is an ACL resource for a non-existing resource, server would apply that ACL resource and checks for accessTo).

That clarifies everything right? Okay, one more moment..

Taking the following row as an example:

PUT C/R
C/ C/R C/R exists C/R doesn't exist
- Write 200 403

I'll expand:

  1. There is an Authorization rule for C/R and C/R exists.
  2. There is an Authorization rule for C/R and C/R does not exist.

So, this depends on how a system is designed. It is not even a Protocol issue because we haven't imposed anyting beyond what RFC 7231 says! I've entered the codes as accurately as I can, and I trust that you've verified their accuracy - (I don't mean to imply that there are no other errors.)

Scenario 1 is a non-issue. Authorization is approved. Storage is checked, C/R exists, happy response.

Scenario 2 is an edge-case weird possibility, but still correct as per RFC. It shows that the system has (allowed!) an Authorization rule for C/R where C/R doesn't exist. The system got approval to write C/R. Storage is checked, C/R does not exist, sad response.

The system should either a) not have the request proceed with full range of allows or disallows for resources, or b) not have Authorization rules for resources that do not exist.

The table row wasn't accidental. I wrote it fully knowing that possibility, where there is an authorization rule for something that does not exist. It was intended to demonstrate the status codes. Systems don't have to be designed to cover that case. They can work around it. But if they want to allow scenario 2 and conform to RFC, that's how it is. I think.

That's the best explanation I can give right now.

Computer, exit.
Computer, arch.

@kjetilk
Copy link
Member

kjetilk commented Oct 19, 2021

After having a more informal video chat with @RubenVerborgh we ended up thinking if we can resolve this simply for 0.9 and then return to it.

I think the case of GET is pretty straightforward, the server could respond with a 404 for both for privacy reasons.

The PUT and PATCH case is harder, lets just think aloud around it.

First, I just realized that I might not have fully understood acl:default. The spec says:

The acl:default predicate denotes the container resource whose Authorization can be applied to a resource lower in the collection hierarchy.

I guess it is possible to interprete that as acl:default not being applicable to the container itself, but I suppose not. In which case, it would illustrate the corner case, say that C/.acl contains

[...]
   acl:accessTo <./R>

i.e., this situation would only occur if there is no acl:default in the hierarchy (or there is a separate Authorization that only grants read), and yet, an acl:accessTo for a resource that does not yet exist. Right?

But that could happen, there's nothing to prohibit that.

So, then, there are two things that would resolve this, one is to make sure that a 403 would result from PUTting to an existing resource that you have write to. That's a non-starter of course.

The other thing is to make it OK to create a resource on a container without having write permissions on the container if you have write permissions on the resource. That would violate the principle that no state change should happen without authorization, as a create operation does change the state of the container by adding a containment triple. Which again is rooted on Tim's idea that "if I remove Write access to something it won't change". I think that's hard to swallow, but lets explore.

This is basically the same debate as we have had for delete operations, where it was resolved in solid/specification#197 that DELETE requires write on the container, a decision that is controversial, and is still being discussed. It would be pretty strange though, to have this principle apply for DELETE but not PUT and PATCH.

I have proposed a middle way, in which the server becomes an agent in its own right, and that since the server is actually managing the containment triples, you don't need an authorization for the client to do it, you'd only need authorization for the server, and that could in simple cases be hard coded. That would resolve both these issues.

So, I suppose that what we could do, and I think there would be a few 👍 's it to revert the decision to require write on the container for creations or deletions if the client has write on the resource itself for 0.9, but to get that back in for 1.0. I suppose that also reflects current NSS behavior?

Or we could have a more elaborate mechanism, like my proposal to make the server an agent. Anyway, this is not just about WAC, it is about the core concepts.

@csarven
Copy link
Member

csarven commented Oct 20, 2021

I didn't process the last two comments in full but I'll respond to one thing that stood out, and get back to the rest later:

I think the case of GET is pretty straightforward, the server could respond with a 404 for both for privacy reasons.

is not a proper response to:

the row for GET can be 403 only. (Even though I mentioned several times that the 404 path is completely optional.) The reasoning is that only the request target is relevant since the method is safe and idempotent. The resource state is not relevant.

I would add:

  • if requester has the knowledge of their permission to Write on C/R (e.g., by way of wac-allow indicating public or user has "write") then 404 is an incorrect (if not a misleading) response.

  • 403 response also properly ties with possibility to inform the recipient that they can request desired access, and the next steps are clear.

403 is precisely what's going on.

@kjetilk
Copy link
Member

kjetilk commented Oct 20, 2021

@RubenVerborgh , right but how do you want to resolve it?

Also, I don't think this has consequences for a reductive architecture in general, but it has consequences for an Express-style or Express-inspired architecture that actually needs to throw HTTP status codes in a reductive manner.

Anyway, I think we can try to find a short-term solution for 0.9, I'm just not sure how that solution would look, as the only way that I can see that we can fix this is to abandon the principle that no state change should happen without authorization.

@kjetilk
Copy link
Member

kjetilk commented Oct 21, 2021

For me, don't require Append on the container to create/update a resource for which a specific ACL already grants Write or Append.

But if you do that, wouldn't that also be an argument to revert the decision in solid/specification#197 ?

@kjetilk
Copy link
Member

kjetilk commented Oct 22, 2021

OK.

First of all, it is important that the status codes are derived, not decided, and that is important because I (for one) don't want to see the kind of incoherence we see in e.g. LDP.

Therefore, I think it is important that all these things are derived from the same principles. I think we could consider not requiring write on the container for 0.9, I would want to have a principled approach to it for 1.0.

Given that solid/specification#197 has been decided, I think it is a difficult thing to change though.

@RubenVerborgh RubenVerborgh changed the title Proposed access rules cannot be implemented without roundtrips to the backend Proposed access rules require authentication to know about backend or vice versa Nov 1, 2021
@RubenVerborgh RubenVerborgh changed the title Proposed access rules require authentication to know about backend or vice versa Proposed access rules require coupling between authentication and backend in edge cases Dec 1, 2021
@RubenVerborgh RubenVerborgh changed the title Proposed access rules require coupling between authentication and backend in edge cases Proposed access rules require coupling between authentication and backend to cover edge cases Dec 1, 2021
@michielbdejong

This comment has been minimized.

@michielbdejong

This comment has been minimized.

@RubenVerborgh RubenVerborgh changed the title Proposed access rules require coupling between authentication and backend to cover edge cases Edge cases require all implementations to couple authorization and storage Dec 30, 2021
@csarven
Copy link
Member

csarven commented Feb 9, 2022

@RubenVerborgh , it was explained that this is not a WAC bug and the label was removed - something you've agreed with: #97 (comment) - but I'm now noting that you've introduced the label without discussion.

@csarven csarven removed the bug label Feb 9, 2022
@timbl
Copy link

timbl commented Feb 9, 2022

Happy for this discussion to be moved to another issue. But while I am here I want to leave a short rant against false economies. Solid works well when the protocol allows powerful operations. If you shirk doing something like iterating over a path /a/b/c/d/e/f in the server in order to save milliseconds on the server, remember how it is orders of magnitude slower to force the client to iterate over them as each client iteration is an operation across the global internet and is limited at the end of the day by the speed of light, and can take seconds. That that the server creates intermediate directories automatically with a PATCH INSERT to a non-existing file is great speed-up, for which no amount of internal server optimization can compete.

Another example - If by giving insufficient data about container contents to the client forcing it to iterate over the folder contents doing a HEAD, that's massively slower.

In this case, if you you don't tell a client whether a potentially forbidden resource exists, only that it is forbidden, forcing the client to get the user to log in and retry with credentials to find actually it does not exist, that's not an optimization. (Yes it might be more secure but it is dysfunctional). So if giving better service to the client and the user involves running the Authorization function in parallel with the Storage function every time (for GET/HEAD) rather than starting the storage only of the auth gives an ok, maybe that makes solid a more useful platform. To make the server faster but less powerful would maybe be a false economy. For PATCH, clearly you can't do them in parallel, and maybe the corner cases are different, but still beware of false economies.

@timbl
Copy link

timbl commented Feb 9, 2022

For comparison here is Google docs shows you a file which does not exist
Screen Shot 2022-02-09 at 12 55 24
:

compared to one which does exist but you don't have access to:
Screen Shot 2022-02-09 at 12 55 47

The distinction is important. People expect it I think. It does provide an onboarding path with the protected document , but it does not waste user's time with the one which does not exist.

@csarven
Copy link
Member

csarven commented Feb 10, 2022

@RubenVerborgh solid/specification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants