Get all keys with restricted user

[hzektser]

Given a base install (with auth enabled) and some test data;

ETCDCTL_API=3 ./etcdctl --user="root:root" get --prefix t
test1.1
1
test1.2
2
test2.1
1
test2.2
2

Let's say we have a simple test user with some permissions;

ETCDCTL_API=3 ./etcdctl --user="root:root" user get test1
User: test1
Roles: test1

ETCDCTL_API=3 ./etcdctl --user="root:root" role get test1
Role test1
KV Read:
	[test1, test2) (prefix test1)
KV Write:
	[test1, test2) (prefix test1)

This works as root to get all keys as root;
ETCDCTL_API=3 ./etcdctl --user="root:root" get --prefix=true ""

but fails as the test user;

ETCDCTL_API=3 ./etcdctl --user="test1:test1" get --prefix=true ""
Error:  etcdserver: permission denied

I would have expected this to return all the keys test1 has access to.. Failing that, is there any way for test1 to discover what keys it's got access to?

[heyitsanthony]

The current auth policy seems too strict here. Probably a user should be able to issue UserGet for itself and RoleGet for the roles it has so it can see what it can access, but only root can do that now as far as I can tell.
/cc @mitake

[japhar81]

@heyitsanthony that would work, but require a separate call. In the spirit of doing what the user expects -- if I say get all, what I'm really saying is.. get all (that I have access to). I honestly think the simplest answer is just return what you're allowed to see. And if that's nothing, return an empty set.

I'd almost suggest that permission denied shouldn't happen on a get, perhaps just an empty result -- the extra benefit is the client won't know if the key exists or not. Right now I can infer key existence, if not it's value, from the return message.

[heyitsanthony]

@japhar81 do other systems behave like this?

[japhar81]

@heyitsanthony I can attest that MapR's HDFS works that way.

[heyitsanthony]

OK, I can think of some problems with that model for a system like etcd.

For instance, if a user asks for "k" without permissions and gets an empty response, despite "k" existing. The client has no way to tell whether the key does not exist or if it just lacks permission without extra RPCs. What if it's checking for a key as some part of an election protocol? Should the client then proceed as if there's no leader?

Likewise, suppose that same user issues a transaction with Compare(Version("k"), "=", 0) to check if "k" exists. Should etcd process that as true if the user lacks permissions but false if the user has permissions? Would that accurately reflect the intent of the transaction?

I'd much rather reject it outright than have these cases that will silently break.

[japhar81]

How about just a strict option of some sort? Give me the choice.. behave as you do now, or just return what I can see. Or even introduce a new command, get-allowed or something? I don't have a strong implementation preference, just a strong belief that a client should be able to discover what it's got access to see..

[mitake]

@hzektser @heyitsanthony @japhar81 I understood the motivation of reading all granted keys. etcd v3's range based permission would be a little bit complex than file system model (related discussion can be found here: #6359 (comment)). So checking that permission configuration won't result unintentional granting is important. Possibly changing the semantics of UserGet and RoleGet + a new command of etcdctl will be a good and simple solution. I'll work on it.

[mitake]

@xiang90 I'm reopening this PR because the client side change is still undergoing (I should remove Fix #8157 in the PR). The change still needs consideration but wip version can be found here: mitake@b0ecc0c .

[wenjiaswe]

/cc @jpbetz

[jpbetz]

@mitake Any interest in keeping this open? Looks like the mitake@b0ecc0c WIP was never merged.

[mitake]

@jpbetz currently I don't have emergent use cases of this feature. If there are users who wants this, I'll revive the commit. But closing this issue temporally wouldn't be a problem, I think.

[jpbetz]

Sounds good. Closing this for now since there is no activity. If there is interest or activity on this in the future we can easily reopen.