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

When a resource is deleted, should the acl be too? #251

Open
melvincarvalho opened this issue Mar 3, 2019 · 14 comments
Open

When a resource is deleted, should the acl be too? #251

melvincarvalho opened this issue Mar 3, 2019 · 14 comments

Comments

@melvincarvalho
Copy link
Member

When a resource is deleted, should the ACL be too?

Looking at :

https://github.com/solid/node-solid-server/blob/master/lib/ldp.js#L533

Perhaps the answer is "No" for node solid server. Not a criticism, just clarifying and exploring.

What is desirable at the solid level.

So my mental model is that solid webizes the unix file system, and that the .acl is part of the inode. In UNIX both get deleted together. Any thoughts?

cc @timbl (as this seems architecturally significant?)

@csarven
Copy link
Member

csarven commented Mar 3, 2019

Also deleting the ACL makes sense to me.

@justinwb
Copy link
Member

justinwb commented Mar 3, 2019

I think it has to be deleted, because there is a security problem here too. A file that is removed and has its ACL persist may seem harmless, but then if another file (or folder) with the same name is put back in its place it will have the same permissions as the prior. In a lot of cases this would be harmless, but there could be certain cases where this exposes private data. Regardless, I’m not aware of a good reason to keep an ACL around if the resource tied to it is gone.

@melvincarvalho
Copy link
Member Author

So possibly a solution here could be.

Set a flag on the request via the client, if you want to "DELETE ALL", meaning delete meta data with the content.

Or on the client delete both.

Possibly this could be more easy to do after we get atomic operations, which I think there is consensus for?

@elf-pavlik
Copy link
Member

instead, disallow PUT if a file-specific ACL exists.

I haven't checked how using POST and Slug HTTP header currently work but it might also need to get taken into account for this scenario.

@melvincarvalho
Copy link
Member Author

I haven't checked how using POST and Slug HTTP header currently work but it might also need to get taken into account for this scenario.

I dont think this works. Lets say you have an app that wants to POST a private resource to a public container. First it will POST the .acl, then it will POST the file.

If you do it the other way round someone watching the container could see the file, which is a breach of privacy.

@melvincarvalho
Copy link
Member Author

Good point. I think it would apply to PUT though.

@csarven
Copy link
Member

csarven commented Mar 7, 2019

A file that is removed and has its ACL persist may seem harmless, but then if another file (or folder) with the same name is put back in its place it will have the same permissions as the prior.

We could argue the exact opposite. Accidentally deleting and restoring a file, could lead to it suddenly becoming visible to the world.

It is not "restoring" as like an "undo" action. It would be creating a new file.

Either way, we acknowledge that a dangling .acl is problematic for subsequent operations. Forcing applications to work around a dangling .acl would be too expensive. Not to mention trying to figure out who previously controlled or created a file/resource or even if that should be disclosed - that's a security issue in and itself.

The simplest design is to not let it dangle. DELETE /foo should trigger the server to delete /foo.acl automatically, if it exists. If this base case can be further improved, that's all fine, but the most intuitive thing here I think is to just let it go.

I acknowledge the convenience that comes with being able to reuse the ACL rules of a destroyed file, but I'm not convinced that is a common design pattern on the Web or on operating systems. If it is or if anyone knows how some of those systems work, we should understand what they are doing and maybe borrow ideas where applicable. Which brings me to:

Certainly the "undo" like behaviour is useful, so that both the file and corresponding ACL rules can indeed be restored. However, for that to happen, the association with the file (resource) they are making needs be well intact - after all, the rules originally applied to the original (now deleted) file, and nothing else. So, this scenario is a subset of an Undo/Restore action - desktop recycling bin or an email will be sent in 10 seconds unless you stop it, or Gitter allowing you to edit up to a certain time.. etc. comes to mind. Whether that's a desirable feature for Solid can be investigated, and deserves its own issue (note: refer to this issue, whoever creates one).

@melvincarvalho
Copy link
Member Author

What about apps that put the .acl first then the file?

@csarven
Copy link
Member

csarven commented Mar 8, 2019

I don't think there is an issue with that. Is there?

@TallTed
Copy link

TallTed commented Mar 8, 2019

I'm struggling to conceive of the scenario you're imagining, @RubenVerborgh.

If you don't have Control permission, how can you take an action that requires Control permission?

If you cannot delete a Resource (because you lack Control permission on that Resource), you should not be able to delete its ACL -- and if you can delete a Resource (because you have Control permission on that Resource), you should likewise be able to delete its ACL, shouldn't you?

@TallTed
Copy link

TallTed commented Mar 8, 2019

Deleting a resource requires Write access on its parent folder (not Control access on that resource).

That seems to me like a broken permissions model to me.

As I understand it, Control permission on a Resource amounts to Write permission on that Resource's ACL.

Therefore, deleting a Resource should require Write and/or Control (which implies that you can grant yourself explicit Write) permission on the Resource, not on its Container.

Creating a Resource should require Write and/or Control (which implies that you can grant yourself explicit Write) permission on its Container.

@TallTed
Copy link

TallTed commented Mar 8, 2019

I stand corrected on the model being implemented. It remains absolutely broken to my mind.

@csarven
Copy link
Member

csarven commented Mar 9, 2019

Should DELETE /foo return 403 if the user doesn't have the required privileges? If yes, no further action required on /foo.acl.

Can a user have the required privileges to DELETE /foo but not /foo.acl (in)directly? If yes, what is the scenario?

@melvincarvalho
Copy link
Member Author

Copying the linux model seems reasonable. I think timbl commented once that the unix file system model has been around for decades and is quite well tested.

So, I dont see an issue with 'dangling' acls. Indeed you'll need to put the acl first before the content if you want to protect privacy.

What I think is in progress is atomic operations to do both. So that will help

What can also happen is you can have implementation specific optimizations. So that you can ask for an acl to be deleted with the file, by setting, say a header.

In short, I think we can leave things the way they are until we get atomic operations of allowing 2 files to change at once with locking. Other than that as far as possible keep it aligned to the linux / unix model -- does that throw up any new issues?

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

6 participants