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

Improve K/V store end points to always return appropriate response codes for ACL violations #3511

Closed
preetapan opened this issue Sep 28, 2017 · 0 comments · Fixed by #3523
Closed
Assignees
Labels
theme/acls ACL and token generation type/enhancement Proposed improvement or new feature

Comments

@preetapan
Copy link
Contributor

preetapan commented Sep 28, 2017

Consul's ACL system currently has two permissions - read and write, and lacks a "list" permission. This leads to confusing and incorrect response codes.

  • When endpoints that set recurse or read keys, are called with a token that does not have read permissions to any suffixes of a specific key, we return a 404 rather than a 403.
  • If the recurse endpoint is called with a token that has no read permissions to any keys, and using the empty prefix, we return a 404.
  • It is possible to detect the presence of a key without a valid ACL, by trying to read it first, seeing a 403, and then trying to recurse on its prefix and getting a 404.

To address all this, this issue captures the introduction of a new list permission. This will replace the current implementation of applying read ACLs as a post-filtering step. Instead, if a token does not have list permissions, the recurse/list keys end points will return a 403. The list permission will apply to all suffixes of a key. This behavior will be controlled by a boolean config and be opt in to start with.

This will address #2637 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/acls ACL and token generation type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants