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

store: design and implement access control #70

Closed
adg opened this issue Sep 19, 2016 · 14 comments
Closed

store: design and implement access control #70

adg opened this issue Sep 19, 2016 · 14 comments

Comments

@adg
Copy link
Collaborator

adg commented Sep 19, 2016

We need to figure out how ACLs will work for store servers. We can't just let anyone Put/Delete.

Related to issue #23.

@robpike
Copy link
Contributor

robpike commented Sep 19, 2016

Delete is by far the worse problem. But what if we didn't have Delete in the API?

@adg
Copy link
Collaborator Author

adg commented Sep 19, 2016

Put("your-reference", "lol your data is gone") is just as worrisome as Delete. 😄

@robpike
Copy link
Contributor

robpike commented Sep 19, 2016

You need to break SHA256 for that to scare me.

@adg
Copy link
Collaborator Author

adg commented Sep 19, 2016

Why? The references aren't secret. Anyone with read access can know them.

@robpike
Copy link
Contributor

robpike commented Sep 20, 2016

Because (at least for dir/server) the store server gives you the reference, you don't get to pick it.

@adg
Copy link
Collaborator Author

adg commented Sep 20, 2016

Of the ideas thrown around at yesterday's meeting, the most favored appeared to be adding a method to StoreServer that takes an Access file, which determines who has access to the store.

@edpin
Copy link
Collaborator

edpin commented Sep 20, 2016

Does it need to be a new method or can Configure fulfill that role?

@adg
Copy link
Collaborator Author

adg commented Sep 20, 2016

IIUC we are removing Configure soon, and good riddance I say.

On 21 September 2016 at 08:33, Eduardo Pinheiro notifications@github.com
wrote:

Does it need to be a new method or can Configure fulfill that role?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#70 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AIDilSO3SZAn2v5za8MO2ekCO3v6sigLks5qsF8egaJpZM4KAEa2
.

adg pushed a commit that referenced this issue Oct 19, 2016
Useful in various places, including one being fixed here, in client.go.

Updates #70

All tests pass.

Change-Id: Ief7f15e649648f1c09884433d299dc5798b3d05e
Reviewed-on: https://upspin-review.googlesource.com/5872
Reviewed-by: Rob Pike <r@google.com>
adg pushed a commit that referenced this issue Oct 20, 2016
Gives the storeserver an Upspin user name and key, with a corresponding
entry in the keyserver.

This is in support of the storeserver being able to query the dirserver
in the future (in particular, for Group files for internal
configuration).

Updates #70

All tests pass.
Deployed to upspin-test to verify.
Pushed new deploy.tar.gz.

Change-Id: I8090a031503f93494adc2ea6903e3d4e7b8892f3
Reviewed-on: https://upspin-review.googlesource.com/5900
Reviewed-by: Eric Grosse <ehg@google.com>
Reviewed-by: Andrew Gerrand <adg@google.com>
@edpin
Copy link
Collaborator

edpin commented Oct 20, 2016

In thinking more about the proposal from yesterday, a few questions arose.

  1. Should the control file be semantically equivalent to a Group file or be exactly an Access file?

The argument for an Access file is that it would enable more fine-grained control for the truly paranoid. For example, it could limit reads to only a whilelist of users. While regular Access files already do that, they don't prevent what I'm calling a "proof of ownership" attack. In this attack, an entity that knows the contents of a file can probe your store server for its hash to prove you have it stored.

Another advantage to having an Access file is in differentiating between Deletes and Writes. Since the Store API has a Delete method, anyone with write permission could potentially (accidentally or not) delete entries from the store. Some users may want to limit that.

The List right wouldn't make sense.

Differentiating between Create and Write is probably irrelevant for a content-addressable Store, but might be useful for other kinds.

The current plan is to go with Group file syntax. But the above is an interesting issue to ponder.

  1. Assuming Group file syntax, what should it be called?

I was thinking the main file would live in the root and be called Owners or Writers. The contents of the file would have Group-like syntax: if a username is listed, it's part of the Owners/Writers. If it's another group name, then the lookup would continue inside a /Group/name file, as usual.

So is "Writers" ok? As in upspin-store@upspin.io/Writers.

  1. The code for this will likely be shared among the various store implementations. Should it live in store/common? store/permission? Somewhere else? If no one has an opinion, I'll write it as part of store/gcp and put a TODO to move out if/when it's re-used. (Yes, it's simple, but there's the refreshing code to fetch updates from time to time, a few initialization functions and a simple interfce to ask "is this user allowed to do this mutating operation")

Thanks.

@robpike
Copy link
Contributor

robpike commented Oct 20, 2016

I don't believe an Access file is a good fit. The list right makes no sense, and the others can be made to work but that is too complicated.

Here's my counterproposal.

The owner of the server has administration rights. No one else does. This includes the right to change who can access and the right to delete data: basically, to administer the server. If a person wants to administer, they "become" that user by using a context file with the keys for the owner. This seems very straightforward and easy to understand.

Everyone else is a user of the server. Those users who are allowed to write blocks there are listed in a group-format file. I would even suggest we make it a /Group/ file because why not?

The remaining question is read, and for dir/storeservers, which is what we are talking about, it's possible all the protection we need is that the references are cryptographically opaque.

So it comes down to: owner is administrator, group file says who can write.

@edpin
Copy link
Collaborator

edpin commented Oct 20, 2016

Sounds fine by me.

Do you care to pick a name for the /Group file? There needs to be an entry point, otherwise if there are several group files, are they all combined together? Which one do we expand first is my question 2.

My suggestion is to pick a name such as Owners or Writers or AllowedMutators or something funnier that only Dave can come up with (but would be too long anyway :-))

Then, I suggest putting this file in the root and if it mentions other groups, then we expand /Group/groupname as usual. We can put this file in /Group/Writers as well (or whatever the entry name is). I just need an initial name to look for, so the Store does not have to glob /Group.

Thoughts?

@robpike
Copy link
Contributor

robpike commented Oct 20, 2016

/Group/users

@adg
Copy link
Collaborator Author

adg commented Oct 20, 2016

Here's how I imagined it should work:

There's a dirserver at dir.example.com that runs as jess+dir@example.org and a storeserver at store.example.com that runs as jess+store@example.org.

When the storeserver starts for the first time, it is wide open and accepts writes from any user in the keyserver. This permits the dirserver to write to the storeserver when setting up the Group file. After the server has started for the first time, it keeps a record of the last known Group file contents, and uses that as its ACL on startup.

The administrator, as jess+store@example.com, does this to set up the storeserver ACL:

$ upspin mkdir jess+store@example.org
$ upspin mkdir jess+store@example.org/Group
$ upspin put jess+store@example.org/Group/store.example.com
jess@example.org
jess+dir@example.org
^D

This gives both the user and the dirserver write access to the storeserver. The storeserver periodically reads the Group file named store.example.com to update the ACL.

@robpike
Copy link
Contributor

robpike commented Oct 21, 2016

It should be explicit here that the store server should restart immediately once this is set up. Otherwise your design LGTM

@adg adg closed this as completed in 7db7afe Oct 28, 2016
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

3 participants