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

Warn when the token query param is used for auth #16009

Merged
merged 8 commits into from
Jan 24, 2023
3 changes: 3 additions & 0 deletions .changelog/16009.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:deprecation
acl: Deprecate the `token` query parameter and warn when it is used for authentication.
```
211 changes: 141 additions & 70 deletions agent/acl_endpoint_test.go

Large diffs are not rendered by default.

265 changes: 169 additions & 96 deletions agent/agent_endpoint_test.go

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion agent/coordinate_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ func TestCoordinate_Update_ACLDeny(t *testing.T) {
})

t.Run("valid token", func(t *testing.T) {
req, _ := http.NewRequest("PUT", "/v1/coordinate/update?token=root", jsonReader(body))
req, _ := http.NewRequest("PUT", "/v1/coordinate/update", jsonReader(body))
req.Header.Add("X-Consul-Token", "root")
if _, err := a.srv.CoordinateUpdate(nil, req); err != nil {
t.Fatalf("err: %v", err)
}
Expand Down
9 changes: 6 additions & 3 deletions agent/event_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ func TestEventFire_token(t *testing.T) {
}
for _, c := range tcases {
// Try to fire the event over the HTTP interface
url := fmt.Sprintf("/v1/event/fire/%s?token=%s", c.event, token)
url := fmt.Sprintf("/v1/event/fire/%s", c.event)
req, _ := http.NewRequest("PUT", url, nil)
req.Header.Add("X-Consul-Token", token)
resp := httptest.NewRecorder()
_, err := a.srv.EventFire(resp, req)

Expand Down Expand Up @@ -236,7 +237,8 @@ func TestEventList_ACLFilter(t *testing.T) {
}
`)

req := httptest.NewRequest("GET", fmt.Sprintf("/v1/event/list?token=%s", token), nil)
req := httptest.NewRequest("GET", "/v1/event/list", nil)
req.Header.Add("X-Consul-Token", token)
resp := httptest.NewRecorder()

obj, err := a.srv.EventList(resp, req)
Expand All @@ -252,7 +254,8 @@ func TestEventList_ACLFilter(t *testing.T) {

t.Run("root token", func(t *testing.T) {
retry.Run(t, func(r *retry.R) {
req := httptest.NewRequest("GET", "/v1/event/list?token=root", nil)
req := httptest.NewRequest("GET", "/v1/event/list", nil)
req.Header.Add("X-Consul-Token", "root")
resp := httptest.NewRecorder()

obj, err := a.srv.EventList(resp, req)
Expand Down
3 changes: 3 additions & 0 deletions agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc
}
logURL = strings.Replace(logURL, token, "<hidden>", -1)
}
httpLogger.Warn("This request used the token query parameter "+
"which is deprecated and will be removed in Consul 1.17",
"logUrl", logURL)
roncodingenthusiast marked this conversation as resolved.
Show resolved Hide resolved
}
logURL = aclEndpointRE.ReplaceAllString(logURL, "$1<hidden>$4")

Expand Down
33 changes: 22 additions & 11 deletions agent/prepared_query_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ func TestPreparedQuery_Create(t *testing.T) {
t.Fatalf("err: %v", err)
}

req, _ := http.NewRequest("POST", "/v1/query?token=my-token", body)
req, _ := http.NewRequest("POST", "/v1/query", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQueryGeneral(resp, req)
if err != nil {
Expand Down Expand Up @@ -234,7 +235,8 @@ func TestPreparedQuery_List(t *testing.T) {
}

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query?token=my-token&consistent=true", body)
req, _ := http.NewRequest("GET", "/v1/query?consistent=true", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQueryGeneral(resp, req)
if err != nil {
Expand Down Expand Up @@ -329,7 +331,8 @@ func TestPreparedQuery_Execute(t *testing.T) {
}

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query/my-id/execute?token=my-token&consistent=true&near=my-node&limit=5", body)
req, _ := http.NewRequest("GET", "/v1/query/my-id/execute?consistent=true&near=my-node&limit=5", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQuerySpecific(resp, req)
if err != nil {
Expand Down Expand Up @@ -385,7 +388,8 @@ func TestPreparedQuery_Execute(t *testing.T) {
}

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query/my-id/execute?token=my-token&consistent=true&near=_ip&limit=5", body)
req, _ := http.NewRequest("GET", "/v1/query/my-id/execute?consistent=true&near=_ip&limit=5", body)
req.Header.Add("X-Consul-Token", "my-token")
req.Header.Add("X-Forwarded-For", "127.0.0.1")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQuerySpecific(resp, req)
Expand Down Expand Up @@ -442,7 +446,8 @@ func TestPreparedQuery_Execute(t *testing.T) {
}

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query/my-id/execute?token=my-token&consistent=true&near=_ip&limit=5", body)
req, _ := http.NewRequest("GET", "/v1/query/my-id/execute?consistent=true&near=_ip&limit=5", body)
req.Header.Add("X-Consul-Token", "my-token")
req.Header.Add("X-Forwarded-For", "198.18.0.1")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQuerySpecific(resp, req)
Expand All @@ -460,7 +465,8 @@ func TestPreparedQuery_Execute(t *testing.T) {
t.Fatalf("bad: %v", r)
}

req, _ = http.NewRequest("GET", "/v1/query/my-id/execute?token=my-token&consistent=true&near=_ip&limit=5", body)
req, _ = http.NewRequest("GET", "/v1/query/my-id/execute?consistent=true&near=_ip&limit=5", body)
req.Header.Add("X-Consul-Token", "my-token")
req.Header.Add("X-Forwarded-For", "198.18.0.1, 198.19.0.1")
resp = httptest.NewRecorder()
obj, err = a.srv.PreparedQuerySpecific(resp, req)
Expand Down Expand Up @@ -735,7 +741,8 @@ func TestPreparedQuery_Explain(t *testing.T) {
}

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query/my-id/explain?token=my-token&consistent=true&near=my-node&limit=5", body)
req, _ := http.NewRequest("GET", "/v1/query/my-id/explain?consistent=true&near=my-node&limit=5", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQuerySpecific(resp, req)
if err != nil {
Expand Down Expand Up @@ -828,7 +835,8 @@ func TestPreparedQuery_Get(t *testing.T) {
}

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query/my-id?token=my-token&consistent=true", body)
req, _ := http.NewRequest("GET", "/v1/query/my-id?consistent=true", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQuerySpecific(resp, req)
if err != nil {
Expand Down Expand Up @@ -936,7 +944,8 @@ func TestPreparedQuery_Update(t *testing.T) {
t.Fatalf("err: %v", err)
}

req, _ := http.NewRequest("PUT", "/v1/query/my-id?token=my-token", body)
req, _ := http.NewRequest("PUT", "/v1/query/my-id", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
if _, err := a.srv.PreparedQuerySpecific(resp, req); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -988,7 +997,8 @@ func TestPreparedQuery_Delete(t *testing.T) {
t.Fatalf("err: %v", err)
}

req, _ := http.NewRequest("DELETE", "/v1/query/my-id?token=my-token", body)
req, _ := http.NewRequest("DELETE", "/v1/query/my-id", body)
req.Header.Add("X-Consul-Token", "my-token")
resp := httptest.NewRecorder()
if _, err := a.srv.PreparedQuerySpecific(resp, req); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -1087,7 +1097,8 @@ func TestPreparedQuery_Integration(t *testing.T) {
// List them all.
{
body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/query?token=root", body)
req, _ := http.NewRequest("GET", "/v1/query", body)
req.Header.Add("X-Consul-Token", "root")
resp := httptest.NewRecorder()
obj, err := a.srv.PreparedQueryGeneral(resp, req)
if err != nil {
Expand Down
12 changes: 8 additions & 4 deletions agent/snapshot_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ func TestSnapshot(t *testing.T) {
testrpc.WaitForTestAgent(t, a.RPC, "dc1")

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest("GET", "/v1/snapshot?token=root", body)
req, _ := http.NewRequest("GET", "/v1/snapshot", body)
req.Header.Add("X-Consul-Token", "root")
resp := httptest.NewRecorder()
if _, err := a.srv.Snapshot(resp, req); err != nil {
t.Fatalf("err: %v", err)
Expand All @@ -51,7 +52,8 @@ func TestSnapshot(t *testing.T) {
defer a.Shutdown()
testrpc.WaitForTestAgent(t, a.RPC, "dc1")

req, _ := http.NewRequest("PUT", "/v1/snapshot?token=root", snap)
req, _ := http.NewRequest("PUT", "/v1/snapshot", snap)
req.Header.Add("X-Consul-Token", "root")
resp := httptest.NewRecorder()
if _, err := a.srv.Snapshot(resp, req); err != nil {
t.Fatalf("err: %v", err)
Expand All @@ -71,7 +73,8 @@ func TestSnapshot_Options(t *testing.T) {
defer a.Shutdown()

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest(method, "/v1/snapshot?token=anonymous", body)
req, _ := http.NewRequest(method, "/v1/snapshot", body)
req.Header.Add("X-Consul-Token", "anonymous")
resp := httptest.NewRecorder()
_, err := a.srv.Snapshot(resp, req)
if !acl.IsErrPermissionDenied(err) {
Expand All @@ -97,7 +100,8 @@ func TestSnapshot_Options(t *testing.T) {
defer a.Shutdown()

body := bytes.NewBuffer(nil)
req, _ := http.NewRequest(method, "/v1/snapshot?token=root&stale", body)
req, _ := http.NewRequest(method, "/v1/snapshot?stale", body)
req.Header.Add("X-Consul-Token", "root")
resp := httptest.NewRecorder()
_, err := a.srv.Snapshot(resp, req)
if method == "GET" {
Expand Down
6 changes: 3 additions & 3 deletions website/content/api-docs/api-structure.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ $ curl \
http://127.0.0.1:8500/v1/agent/members
```

Previously this was provided via a `?token=` query parameter. This functionality
exists on many endpoints for backwards compatibility, but its use is **highly
discouraged**, since it can show up in access logs as part of the URL.
**Security Note:** Though you could pass the token through the `?token=` query parameter,
this method is highly discouraged because the token can show up in access logs as part of the URL.
The `?token=` query parameter is deprecated and will be removed in Consul 1.17.

To learn more about the ACL system read the [documentation](/docs/security/acl).

Expand Down
6 changes: 4 additions & 2 deletions website/content/docs/security/acl/acl-legacy.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -663,25 +663,27 @@ Here's a sample request using the HCL form:
```shell-session
$ curl \
--request PUT \
--header "X-Consul-Token: <management token>" \
--data \
'{
"Name": "my-app-token",
"Type": "client",
"Rules": "key \"\" { policy = \"read\" } key \"foo/\" { policy = \"write\" } key \"foo/private/\" { policy = \"deny\" } operator = \"read\""
}' http://127.0.0.1:8500/v1/acl/create?token=<management token>
}' http://127.0.0.1:8500/v1/acl/create
```

Here's an equivalent request using the JSON form:

```shell-session
$ curl \
--request PUT \
--header "X-Consul-Token: <management token>" \
--data \
'{
"Name": "my-app-token",
"Type": "client",
"Rules": "{\"key\":{\"\":{\"policy\":\"read\"},\"foo/\":{\"policy\":\"write\"},\"foo/private\":{\"policy\":\"deny\"}},\"operator\":\"read\"}"
}' http://127.0.0.1:8500/v1/acl/create?token=<management token>
}' http://127.0.0.1:8500/v1/acl/create
```

On success, the token ID is returned:
Expand Down
6 changes: 4 additions & 2 deletions website/content/docs/security/acl/acl-policies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -286,23 +286,25 @@ The following example adds a set of rules to a policy called `my-app-policy`. Th
```shell-session
$ curl \
--request PUT \
--header "X-Consul-Token: <token with ACL 'write' access>" \
--data \
'{
"Name": "my-app-policy",
"Rules": "key \"\" { policy = \"read\" } key \"foo/\" { policy = \"write\" } key \"foo/private/\" { policy = \"deny\" } operator = \"read\""
}' http://127.0.0.1:8500/v1/acl/policy?token=<token with ACL "write" access>
}' http://127.0.0.1:8500/v1/acl/policy
```

The following call performs the same operation as the previous example using JSON:

```shell-session
$ curl \
--request PUT \
--header "X-Consul-Token: <management token>" \
--data \
'{
"Name": "my-app-policy",
"Rules": "{\"key\":{\"\":{\"policy\":\"read\"},\"foo/\":{\"policy\":\"write\"},\"foo/private\":{\"policy\":\"deny\"}},\"operator\":\"read\"}"
}' http://127.0.0.1:8500/v1/acl/policy?token=<management token>
}' http://127.0.0.1:8500/v1/acl/policy
```

The policy configuration is returned when the call is successfully performed:
Expand Down
26 changes: 26 additions & 0 deletions website/content/docs/upgrading/upgrade-specific.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,32 @@ upgrade flow.

The `connect.enable_serverless_plugin` configuration option was removed. Lambda integration is now enabled by default.

#### Deprecating authentication via token query parameter

Providing a Consul ACL token in API requests using the `token` query parameter is deprecated and will be removed in Consul 1.17.
Instead, you should provide the token through the `X-Consul-Token` header or with the Bearer scheme in the authorization header as described in the [API authentication documentation](/consul/api-docs/api-structure#authentication).

Check whether you are using a `token` query parameter by searching your Consul agent logs for the message:

```shell-session hideClipboard
$ This request used the token query parameter which is deprecated and will be removed in Consul 1.17
```

Deprecated authentication using the `token` query parameter:

```shell-session
$ curl \
http://127.0.0.1:8500/v1/agent/members?token=<consul token>
```

Recommended authentication method:

```shell-session
$ curl \
--header "X-Consul-Token: <consul token>" \
http://127.0.0.1:8500/v1/agent/members
```

#### Lambda Configuration

Instead of configuring Lambda functions in the `Meta` field of `service-defaults` configuration entries, configure them with the `EnvoyExtensions` field.
Expand Down