Add support for returning ACL secret IDs for accessors with acl:write (#10546)

2021-07-08 15:13:08 -07:00 · 2021-07-08 15:13:08 -07:00 · 13bd86527b
parent ec6da0859d
commit 13bd86527b
20 changed files with 68 additions and 2 deletions
--- a/.changelog/10546.txt
+++ b/.changelog/10546.txt
@ -0,0 +1,3 @@
+```release-note:improvement
+acl: Return secret ID when listing tokens if accessor has `acl:write`
+```
--- 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)
--- 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
 	}
 }

--- 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) {
--- 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,
--- 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)
--- 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"`
--- 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)
--- 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))
 	}
--- 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,
--- 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",
--- 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
--- 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
--- 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": [
            {
--- 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
--- 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
--- 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",
--- 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
--- 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
--- 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": "<hidden>",
    "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": "<hidden>",
    "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": "<hidden>",
    "Description": "Bootstrap Token (Global Management)",
    "Policies": [
      {