From 5ff191ad993281b7c7e98bc6c09a12d2da4f173f Mon Sep 17 00:00:00 2001 From: Evan Culver Date: Thu, 8 Jul 2021 15:13:08 -0700 Subject: [PATCH] Add support for returning ACL secret IDs for accessors with acl:write (#10546) --- .changelog/10546.txt | 3 ++ agent/acl_endpoint_test.go | 1 + agent/consul/acl.go | 5 ++++ agent/consul/acl_endpoint_test.go | 30 +++++++++++++++++++ agent/structs/acl.go | 2 ++ agent/structs/acl_test.go | 2 ++ api/acl.go | 1 + api/acl_test.go | 3 ++ command/acl/token/formatter.go | 1 + command/acl/token/formatter_test.go | 3 ++ .../FormatTokenList/basic.json.golden | 1 + .../FormatTokenList/basic.pretty-meta.golden | 1 + .../FormatTokenList/basic.pretty.golden | 1 + .../FormatTokenList/complex.json.golden | 1 + .../complex.pretty-meta.golden | 1 + .../FormatTokenList/complex.pretty.golden | 1 + .../FormatTokenList/legacy.json.golden | 1 + .../FormatTokenList/legacy.pretty-meta.golden | 1 + .../FormatTokenList/legacy.pretty.golden | 1 + website/content/api-docs/acl/tokens.mdx | 10 +++++-- 20 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 .changelog/10546.txt diff --git a/.changelog/10546.txt b/.changelog/10546.txt new file mode 100644 index 000000000..ee260f253 --- /dev/null +++ b/.changelog/10546.txt @@ -0,0 +1,3 @@ +```release-note:improvement +acl: Return secret ID when listing tokens if accessor has `acl:write` +``` diff --git a/agent/acl_endpoint_test.go b/agent/acl_endpoint_test.go index 3c67757a9..131005f78 100644 --- a/agent/acl_endpoint_test.go +++ b/agent/acl_endpoint_test.go @@ -859,6 +859,7 @@ func TestACL_HTTP(t *testing.T) { found := false for _, actual := range tokens { if actual.AccessorID == tokenID { + require.Equal(t, expected.SecretID, actual.SecretID) require.Equal(t, expected.Description, actual.Description) require.Equal(t, expected.Policies, actual.Policies) require.Equal(t, expected.Local, actual.Local) diff --git a/agent/consul/acl.go b/agent/consul/acl.go index c0b0436a1..b440e7899 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -1767,6 +1767,11 @@ func (f *aclFilter) filterTokenStub(token **structs.ACLTokenListStub) { if f.authorizer.ACLRead(&entCtx) != acl.Allow { *token = nil + } else if f.authorizer.ACLWrite(&entCtx) != acl.Allow { + // no write permissions - redact secret + clone := *(*token) + clone.SecretID = redactedToken + *token = &clone } } diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 094e1931d..591d95f4e 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -2066,6 +2066,36 @@ func TestACLEndpoint_TokenList(t *testing.T) { } require.ElementsMatch(t, gatherIDs(t, resp.Tokens), tokens) }) + + t.Run("filter SecretID for acl:read", func(t *testing.T) { + rules := ` + acl = "read" + ` + readOnlyToken, err := upsertTestTokenWithPolicyRules(codec, TestDefaultMasterToken, "dc1", rules) + require.NoError(t, err) + + req := structs.ACLTokenListRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: readOnlyToken.SecretID}, + } + + resp := structs.ACLTokenListResponse{} + + err = acl.TokenList(&req, &resp) + require.NoError(t, err) + + tokens := []string{ + masterTokenAccessorID, + structs.ACLTokenAnonymousID, + readOnlyToken.AccessorID, + t1.AccessorID, + t2.AccessorID, + } + require.ElementsMatch(t, gatherIDs(t, resp.Tokens), tokens) + for _, token := range resp.Tokens { + require.Equal(t, redactedToken, token.SecretID) + } + }) } func TestACLEndpoint_TokenBatchRead(t *testing.T) { diff --git a/agent/structs/acl.go b/agent/structs/acl.go index ed2b0791a..7f45b4398 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -557,6 +557,7 @@ type ACLTokens []*ACLToken type ACLTokenListStub struct { AccessorID string + SecretID string Description string Policies []ACLTokenPolicyLink `json:",omitempty"` Roles []ACLTokenRoleLink `json:",omitempty"` @@ -578,6 +579,7 @@ type ACLTokenListStubs []*ACLTokenListStub func (token *ACLToken) Stub() *ACLTokenListStub { return &ACLTokenListStub{ AccessorID: token.AccessorID, + SecretID: token.SecretID, Description: token.Description, Policies: token.Policies, Roles: token.Roles, diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go index 86e8dfe20..f1e27867c 100644 --- a/agent/structs/acl_test.go +++ b/agent/structs/acl_test.go @@ -265,6 +265,7 @@ func TestStructs_ACLToken_Stub(t *testing.T) { stub := token.Stub() require.Equal(t, token.AccessorID, stub.AccessorID) + require.Equal(t, token.SecretID, stub.SecretID) require.Equal(t, token.Description, stub.Description) require.Equal(t, token.Policies, stub.Policies) require.Equal(t, token.Local, stub.Local) @@ -286,6 +287,7 @@ func TestStructs_ACLToken_Stub(t *testing.T) { stub := token.Stub() require.Equal(t, token.AccessorID, stub.AccessorID) + require.Equal(t, token.SecretID, stub.SecretID) require.Equal(t, token.Description, stub.Description) require.Equal(t, token.Policies, stub.Policies) require.Equal(t, token.Local, stub.Local) diff --git a/api/acl.go b/api/acl.go index d94c2807a..465e256e2 100644 --- a/api/acl.go +++ b/api/acl.go @@ -58,6 +58,7 @@ type ACLTokenListEntry struct { CreateIndex uint64 ModifyIndex uint64 AccessorID string + SecretID string Description string Policies []*ACLTokenPolicyLink `json:",omitempty"` Roles []*ACLTokenRoleLink `json:",omitempty"` diff --git a/api/acl_test.go b/api/acl_test.go index 956d60417..ceabe905d 100644 --- a/api/acl_test.go +++ b/api/acl_test.go @@ -595,6 +595,7 @@ func TestAPI_ACLToken_List(t *testing.T) { token1, ok := tokenMap[created1.AccessorID] require.True(t, ok) require.NotNil(t, token1) + require.Equal(t, created1.SecretID, token1.SecretID) require.Equal(t, created1.Description, token1.Description) require.Equal(t, created1.CreateIndex, token1.CreateIndex) require.Equal(t, created1.ModifyIndex, token1.ModifyIndex) @@ -604,6 +605,7 @@ func TestAPI_ACLToken_List(t *testing.T) { token2, ok := tokenMap[created2.AccessorID] require.True(t, ok) require.NotNil(t, token2) + require.Equal(t, created2.SecretID, token2.SecretID) require.Equal(t, created2.Description, token2.Description) require.Equal(t, created2.CreateIndex, token2.CreateIndex) require.Equal(t, created2.ModifyIndex, token2.ModifyIndex) @@ -613,6 +615,7 @@ func TestAPI_ACLToken_List(t *testing.T) { token3, ok := tokenMap[created3.AccessorID] require.True(t, ok) require.NotNil(t, token3) + require.Equal(t, created3.SecretID, token3.SecretID) require.Equal(t, created3.Description, token3.Description) require.Equal(t, created3.CreateIndex, token3.CreateIndex) require.Equal(t, created3.ModifyIndex, token3.ModifyIndex) diff --git a/command/acl/token/formatter.go b/command/acl/token/formatter.go index 790b9f73b..f80906b61 100644 --- a/command/acl/token/formatter.go +++ b/command/acl/token/formatter.go @@ -125,6 +125,7 @@ func (f *prettyFormatter) formatTokenListEntry(token *api.ACLTokenListEntry) str var buffer bytes.Buffer buffer.WriteString(fmt.Sprintf("AccessorID: %s\n", token.AccessorID)) + buffer.WriteString(fmt.Sprintf("SecretID: %s\n", token.SecretID)) if token.Namespace != "" { buffer.WriteString(fmt.Sprintf("Namespace: %s\n", token.Namespace)) } diff --git a/command/acl/token/formatter_test.go b/command/acl/token/formatter_test.go index 54daea161..2dfa68301 100644 --- a/command/acl/token/formatter_test.go +++ b/command/acl/token/formatter_test.go @@ -155,6 +155,7 @@ func TestFormatTokenList(t *testing.T) { tokens: []*api.ACLTokenListEntry{ { AccessorID: "fbd2447f-7479-4329-ad13-b021d74f86ba", + SecretID: "257ade69-748c-4022-bafd-76d27d9143f8", Description: "test token", Local: false, CreateTime: time.Date(2020, 5, 22, 18, 52, 31, 0, time.UTC), @@ -168,6 +169,7 @@ func TestFormatTokenList(t *testing.T) { tokens: []*api.ACLTokenListEntry{ { AccessorID: "8acc7486-ca54-4d3c-9aed-5cd85651b0ee", + SecretID: "257ade69-748c-4022-bafd-76d27d9143f8", Description: "legacy", Legacy: true, }, @@ -177,6 +179,7 @@ func TestFormatTokenList(t *testing.T) { tokens: []*api.ACLTokenListEntry{ { AccessorID: "fbd2447f-7479-4329-ad13-b021d74f86ba", + SecretID: "257ade69-748c-4022-bafd-76d27d9143f8", Namespace: "foo", Description: "test token", Local: false, diff --git a/command/acl/token/testdata/FormatTokenList/basic.json.golden b/command/acl/token/testdata/FormatTokenList/basic.json.golden index 180f84c52..8e0b768de 100644 --- a/command/acl/token/testdata/FormatTokenList/basic.json.golden +++ b/command/acl/token/testdata/FormatTokenList/basic.json.golden @@ -3,6 +3,7 @@ "CreateIndex": 42, "ModifyIndex": 100, "AccessorID": "fbd2447f-7479-4329-ad13-b021d74f86ba", + "SecretID": "257ade69-748c-4022-bafd-76d27d9143f8", "Description": "test token", "Local": false, "CreateTime": "2020-05-22T18:52:31Z", diff --git a/command/acl/token/testdata/FormatTokenList/basic.pretty-meta.golden b/command/acl/token/testdata/FormatTokenList/basic.pretty-meta.golden index ab0060774..6454b4f4c 100644 --- a/command/acl/token/testdata/FormatTokenList/basic.pretty-meta.golden +++ b/command/acl/token/testdata/FormatTokenList/basic.pretty-meta.golden @@ -1,4 +1,5 @@ AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba +SecretID: 257ade69-748c-4022-bafd-76d27d9143f8 Description: test token Local: false Create Time: 2020-05-22 18:52:31 +0000 UTC diff --git a/command/acl/token/testdata/FormatTokenList/basic.pretty.golden b/command/acl/token/testdata/FormatTokenList/basic.pretty.golden index c13d1babb..013a904bb 100644 --- a/command/acl/token/testdata/FormatTokenList/basic.pretty.golden +++ b/command/acl/token/testdata/FormatTokenList/basic.pretty.golden @@ -1,4 +1,5 @@ AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba +SecretID: 257ade69-748c-4022-bafd-76d27d9143f8 Description: test token Local: false Create Time: 2020-05-22 18:52:31 +0000 UTC diff --git a/command/acl/token/testdata/FormatTokenList/complex.json.golden b/command/acl/token/testdata/FormatTokenList/complex.json.golden index abaaac0ca..7b3f36d82 100644 --- a/command/acl/token/testdata/FormatTokenList/complex.json.golden +++ b/command/acl/token/testdata/FormatTokenList/complex.json.golden @@ -3,6 +3,7 @@ "CreateIndex": 5, "ModifyIndex": 10, "AccessorID": "fbd2447f-7479-4329-ad13-b021d74f86ba", + "SecretID": "257ade69-748c-4022-bafd-76d27d9143f8", "Description": "test token", "Policies": [ { diff --git a/command/acl/token/testdata/FormatTokenList/complex.pretty-meta.golden b/command/acl/token/testdata/FormatTokenList/complex.pretty-meta.golden index 17cd91243..0dd88ef77 100644 --- a/command/acl/token/testdata/FormatTokenList/complex.pretty-meta.golden +++ b/command/acl/token/testdata/FormatTokenList/complex.pretty-meta.golden @@ -1,4 +1,5 @@ AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba +SecretID: 257ade69-748c-4022-bafd-76d27d9143f8 Namespace: foo Description: test token Local: false diff --git a/command/acl/token/testdata/FormatTokenList/complex.pretty.golden b/command/acl/token/testdata/FormatTokenList/complex.pretty.golden index e6965f33b..ea5cd3efe 100644 --- a/command/acl/token/testdata/FormatTokenList/complex.pretty.golden +++ b/command/acl/token/testdata/FormatTokenList/complex.pretty.golden @@ -1,4 +1,5 @@ AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba +SecretID: 257ade69-748c-4022-bafd-76d27d9143f8 Namespace: foo Description: test token Local: false diff --git a/command/acl/token/testdata/FormatTokenList/legacy.json.golden b/command/acl/token/testdata/FormatTokenList/legacy.json.golden index d639b8025..1a6622838 100644 --- a/command/acl/token/testdata/FormatTokenList/legacy.json.golden +++ b/command/acl/token/testdata/FormatTokenList/legacy.json.golden @@ -3,6 +3,7 @@ "CreateIndex": 0, "ModifyIndex": 0, "AccessorID": "8acc7486-ca54-4d3c-9aed-5cd85651b0ee", + "SecretID": "257ade69-748c-4022-bafd-76d27d9143f8", "Description": "legacy", "Local": false, "CreateTime": "0001-01-01T00:00:00Z", diff --git a/command/acl/token/testdata/FormatTokenList/legacy.pretty-meta.golden b/command/acl/token/testdata/FormatTokenList/legacy.pretty-meta.golden index 855321900..76107205c 100644 --- a/command/acl/token/testdata/FormatTokenList/legacy.pretty-meta.golden +++ b/command/acl/token/testdata/FormatTokenList/legacy.pretty-meta.golden @@ -1,4 +1,5 @@ AccessorID: 8acc7486-ca54-4d3c-9aed-5cd85651b0ee +SecretID: 257ade69-748c-4022-bafd-76d27d9143f8 Description: legacy Local: false Create Time: 0001-01-01 00:00:00 +0000 UTC diff --git a/command/acl/token/testdata/FormatTokenList/legacy.pretty.golden b/command/acl/token/testdata/FormatTokenList/legacy.pretty.golden index ee2a5299b..d0410e10a 100644 --- a/command/acl/token/testdata/FormatTokenList/legacy.pretty.golden +++ b/command/acl/token/testdata/FormatTokenList/legacy.pretty.golden @@ -1,4 +1,5 @@ AccessorID: 8acc7486-ca54-4d3c-9aed-5cd85651b0ee +SecretID: 257ade69-748c-4022-bafd-76d27d9143f8 Description: legacy Local: false Create Time: 0001-01-01 00:00:00 +0000 UTC diff --git a/website/content/api-docs/acl/tokens.mdx b/website/content/api-docs/acl/tokens.mdx index 70634cbf4..4bdc026ae 100644 --- a/website/content/api-docs/acl/tokens.mdx +++ b/website/content/api-docs/acl/tokens.mdx @@ -594,13 +594,17 @@ $ curl -X GET http://127.0.0.1:8500/v1/acl/tokens ### Sample Response --> **Note** - The token secret IDs are not included in the listing and must be -retrieved by the [token reading endpoint](#read-a-token) +-> **Note** If the token used for accessing the API has `acl:write` permissions, +then the `SecretID` will contain the tokens real value. Only when accessed with +a token with only `acl:read` permissions will the `SecretID` be redacted. This +is to prevent privilege escalation whereby having `acl:read` privileges allows +for reading other secrets which given even more permissions. ```json [ { "AccessorID": "6a1253d2-1785-24fd-91c2-f8e78c745511", + "SecretID": "", "Description": "Agent token for 'my-agent'", "Policies": [ { @@ -620,6 +624,7 @@ retrieved by the [token reading endpoint](#read-a-token) }, { "AccessorID": "00000000-0000-0000-0000-000000000002", + "SecretID": "", "Description": "Anonymous Token", "Policies": null, "Local": false, @@ -630,6 +635,7 @@ retrieved by the [token reading endpoint](#read-a-token) }, { "AccessorID": "3328f9a6-433c-02d0-6649-7d07268dfec7", + "SecretID": "", "Description": "Bootstrap Token (Global Management)", "Policies": [ {