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

KV API returning 404 for existing keys with ACLs enabled #2637

Closed
CVTJNII opened this issue Jan 5, 2017 · 5 comments · Fixed by #3340
Closed

KV API returning 404 for existing keys with ACLs enabled #2637

CVTJNII opened this issue Jan 5, 2017 · 5 comments · Fixed by #3340
Assignees
Labels
theme/api Relating to the HTTP API interface type/bug Feature does not function as expected

Comments

@CVTJNII
Copy link

CVTJNII commented Jan 5, 2017

consul version for both Client and Server

Server: Consul v0.7.2
Client: Consul v0.7.2

consul info for both Client and Server

agent:
check_monitors = 0
check_ttls = 0
checks = 0
services = 1
build:
prerelease =
revision = 'a9afa0c
version = 0.7.2
consul:
bootstrap = false
known_datacenters = 2
leader = true
leader_addr = 192.168.253.179:8300
server = true
raft:
applied_index = 6044
commit_index = 6044
fsm_pending = 0
last_contact = never
last_log_index = 6044
last_log_term = 6
last_snapshot_index = 0
last_snapshot_term = 0
latest_configuration = [{Suffrage:Voter ID:192.168.253.155:8300 Address:192.168.253.155:8300} {Suffrage:Voter ID:192.168.253.179:8300 Address:192.168.253.179:8300} {Suffrage:Voter ID:192.168.253.184:8300 Address:192.168.253.184:8300}]
latest_configuration_index = 1
num_peers = 2
protocol_version = 1
protocol_version_max = 3
protocol_version_min = 0
snapshot_version_max = 1
snapshot_version_min = 0
state = Leader
term = 6
runtime:
arch = amd64
cpu_count = 1
goroutines = 74
max_procs = 1
os = linux
version = go1.7.3
serf_lan:
encrypted = false
event_queue = 0
event_time = 6
failed = 0
health_score = 0
intent_queue = 0
left = 0
member_time = 10
members = 3
query_queue = 0
query_time = 1
serf_wan:
encrypted = false
event_queue = 0
event_time = 1
failed = 0
health_score = 0
intent_queue = 0
left = 0
member_time = 29
members = 6
query_queue = 0
query_time = 1

Operating system and Environment details

Consul 0.7.2 installed via the consul_0.7.2_linux_amd64.zip release on alpine:3.4

ACLs enabled with the following configuration:

{
  "bind_addr": "0.0.0.0",
  "client_addr": "0.0.0.0",
  "ui": true,
  "domain": "tcconsul.tjnii.local.",
  "bootstrap": false,
  "acl_datacenter": "acluster",
  "acl_master_token": "MasterToken",
  "acl_default_policy": "deny",
  "acl_enforce_version_8": true,
  "start_join_wan": [
# Snip WAN nodes
  ],
  "datacenter": "acluster",
  "bootstrap_expect": 3,
  "start_join": [

  ]
}

### Description of the Issue (and unexpected/desired result)

While performing initial tests with ACLs enabled it was noticed that existing keys are returning 404, not 403:

```bash
$ curl 'http://127.0.0.1:8500/v1/kv/TestKey' -ILX GET
HTTP/1.1 404 Not Found
X-Consul-Index: 6014
X-Consul-Knownleader: true
X-Consul-Lastcontact: 0
Date: Thu, 05 Jan 2017 15:23:30 GMT
Content-Length: 0
Content-Type: text/plain; charset=utf-8

Passing a valid token (in this case the master) results in a 200:

$ curl 'http://127.0.0.1:8500/v1/kv/TestKey?token=MasterToken' -ILX GET
HTTP/1.1 200 OK
Content-Type: application/json
X-Consul-Index: 6014
X-Consul-Knownleader: true
X-Consul-Lastcontact: 0
Date: Thu, 05 Jan 2017 15:23:34 GMT
Content-Length: 104

This is undesirable behavior because a client searching the keyspace could interpret the 404 as data not existing, not a ACL issue. In a configuration where the service consuming the keyspace doesn't know what keys to expect (i.e. registration of clients where a client writes a unique key for itself) this could cause the consumer to disable functionality as it believes there are no keys if it has the wrong token, instead of failing because it doesn't have access.

I'm assuming this was done to prevent an attacker from fuzzing the keyspace and using 404s/403s to determine what is there. However, I believe returning 404s by default is wrong. With ACLs on, and especially with a deny default policy, it should be returning 403s for all requests without a valid token as that is the proper response code.

EDIT: Clarity

@CVTJNII
Copy link
Author

CVTJNII commented Jan 5, 2017

Related to #1113 but I don't believe it is the same.

@slackpad slackpad added this to the 0.8.0 milestone Feb 8, 2017
@slackpad slackpad modified the milestones: 0.8.0, 0.8.1 Mar 30, 2017
@slackpad slackpad modified the milestones: 0.8.1, 0.8.2 Apr 12, 2017
@slackpad slackpad removed this from the 0.8.2 milestone Apr 25, 2017
@slackpad slackpad added the type/bug Feature does not function as expected label May 2, 2017
@slackpad slackpad added the theme/api Relating to the HTTP API interface label May 25, 2017
@preetapan preetapan self-assigned this Jul 31, 2017
preetapan pushed a commit that referenced this issue Jul 31, 2017
preetapan pushed a commit that referenced this issue Jul 31, 2017
@slackpad
Copy link
Contributor

slackpad commented Aug 9, 2017

Going to revert this for now and re-open this issue since our fix still didn't address the case where the key doesn't exist AND you don't have access to it (this still returns a 404). We may want to just apply the ACL policy before we fetch any keys, since this is problematic doing it from the post-filter.

@slackpad slackpad reopened this Aug 9, 2017
@slackpad
Copy link
Contributor

slackpad commented Aug 9, 2017

@slackpad
Copy link
Contributor

slackpad commented Aug 9, 2017

Linking #3383 and #3382, which have the reverts.

@preetapan
Copy link
Contributor

Closed via issue #3511

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/api Relating to the HTTP API interface type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants