Fixes for CVE-2019-8336

Fix error in detecting raft replication errors.

Detect redacted token secrets and prevent attempting to insert.

Add a Redacted field to the TokenBatchRead and TokenRead RPC endpoints

This will indicate whether token secrets have been redacted.

Ensure any token with a redacted secret in secondary datacenters is removed.

Test that redacted tokens cannot be replicated.

agent/consul/acl_endpoint.go

@@ -213,6 +213,9 @@ func (a *ACL) TokenRead(args *structs.ACLTokenGetRequest, reply *structs.ACLToke
 				index, token, err = state.ACLTokenGetByAccessor(ws, args.TokenID)
 				if token != nil {
 					a.srv.filterACLWithAuthorizer(rule, &token)
+					if !rule.ACLWrite() {
+						reply.Redacted = true
+					}
 				}
 			} else {
 				index, token, err = state.ACLTokenGetBySecret(ws, args.TokenID)
@@ -589,6 +592,7 @@ func (a *ACL) TokenBatchRead(args *structs.ACLTokenBatchGetRequest, reply *struc
 			a.srv.filterACLWithAuthorizer(rule, &tokens)
 
 			reply.Index, reply.Tokens = index, tokens
+			reply.Redacted = !rule.ACLWrite()
 			return nil
 		})
 }

agent/consul/acl_replication.go

@@ -113,7 +113,7 @@ func (s *Server) updateLocalACLPolicies(policies structs.ACLPolicies, ctx contex
 		if err != nil {
 			return false, fmt.Errorf("Failed to apply policy upserts: %v", err)
 		}
-		if respErr, ok := resp.(error); ok && err != nil {
+		if respErr, ok := resp.(error); ok && respErr != nil {
 			return false, fmt.Errorf("Failed to apply policy upsert: %v", respErr)
 		}
 		s.logger.Printf("[DEBUG] acl: policy replication - upserted 1 batch with %d policies of size %d", batchEnd-batchStart, batchSize)
@@ -283,6 +283,9 @@ func (s *Server) updateLocalACLTokens(tokens structs.ACLTokens, ctx context.Cont
 		batchSize := 0
 		batchEnd := batchStart
 		for ; batchEnd < len(tokens) && batchSize < aclBatchUpsertSize; batchEnd += 1 {
+			if tokens[batchEnd].SecretID == redactedToken {
+				return false, fmt.Errorf("Detected redacted token secrets: stopping token update round - verify that the replication token in use has acl:write permissions.")
+			}
 			batchSize += tokens[batchEnd].EstimateSize()
 		}
 
@@ -295,7 +298,7 @@ func (s *Server) updateLocalACLTokens(tokens structs.ACLTokens, ctx context.Cont
 		if err != nil {
 			return false, fmt.Errorf("Failed to apply token upserts: %v", err)
 		}
-		if respErr, ok := resp.(error); ok && err != nil {
+		if respErr, ok := resp.(error); ok && respErr != nil {
 			return false, fmt.Errorf("Failed to apply token upserts: %v", respErr)
 		}
 
@@ -491,6 +494,8 @@ func (s *Server) replicateACLTokens(lastRemoteIndex uint64, ctx context.Context)
 		tokens, err = s.fetchACLTokensBatch(res.LocalUpserts)
 		if err != nil {
 			return 0, false, fmt.Errorf("failed to retrieve ACL token updates: %v", err)
+		} else if tokens.Redacted {
+			return 0, false, fmt.Errorf("failed to retrieve unredacted tokens - replication token in use does not grant acl:write")
 		}
 
 		s.logger.Printf("[DEBUG] acl: token replication - downloaded %d tokens", len(tokens.Tokens))

agent/consul/acl_replication_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/structs"
@@ -563,3 +564,148 @@ func TestACLReplication_Policies(t *testing.T) {
 		checkSame(r)
 	})
 }
+
+func TestACLReplication_TokensRedacted(t *testing.T) {
+	t.Parallel()
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+	client := rpcClient(t, s1)
+	defer client.Close()
+
+	// Create the ACL Write Policy
+	policyArg := structs.ACLPolicySetRequest{
+		Datacenter: "dc1",
+		Policy: structs.ACLPolicy{
+			Name:        "token-replication-redacted",
+			Description: "token-replication-redacted",
+			Rules:       `acl = "write"`,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	var policy structs.ACLPolicy
+	require.NoError(t, s1.RPC("ACL.PolicySet", &policyArg, &policy))
+
+	// Create the dc2 replication token
+	tokenArg := structs.ACLTokenSetRequest{
+		Datacenter: "dc1",
+		ACLToken: structs.ACLToken{
+			Description: "dc2-replication",
+			Policies: []structs.ACLTokenPolicyLink{
+				structs.ACLTokenPolicyLink{
+					ID: policy.ID,
+				},
+			},
+			Local: false,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+
+	var token structs.ACLToken
+	require.NoError(t, s1.RPC("ACL.TokenSet", &tokenArg, &token))
+
+	dir2, s2 := testServerWithConfig(t, func(c *Config) {
+		c.Datacenter = "dc2"
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLTokenReplication = true
+		c.ACLReplicationRate = 100
+		c.ACLReplicationBurst = 100
+		c.ACLReplicationApplyLimit = 1000000
+	})
+	s2.tokens.UpdateReplicationToken(token.SecretID, tokenStore.TokenSourceConfig)
+	testrpc.WaitForLeader(t, s2.RPC, "dc2")
+	defer os.RemoveAll(dir2)
+	defer s2.Shutdown()
+
+	// Try to join.
+	joinWAN(t, s2, s1)
+	testrpc.WaitForLeader(t, s2.RPC, "dc2")
+	testrpc.WaitForLeader(t, s2.RPC, "dc1")
+	waitForNewACLs(t, s2)
+
+	// ensures replication is working ok
+	retry.Run(t, func(r *retry.R) {
+		var tokenResp structs.ACLTokenResponse
+		req := structs.ACLTokenGetRequest{
+			Datacenter:   "dc2",
+			TokenID:      "root",
+			TokenIDType:  structs.ACLTokenSecret,
+			QueryOptions: structs.QueryOptions{Token: "root"},
+		}
+		err := s2.RPC("ACL.TokenRead", &req, &tokenResp)
+		require.NoError(r, err)
+		require.Equal(r, "root", tokenResp.Token.SecretID)
+
+		var status structs.ACLReplicationStatus
+		statusReq := structs.DCSpecificRequest{
+			Datacenter: "dc2",
+		}
+		require.NoError(r, s2.RPC("ACL.ReplicationStatus", &statusReq, &status))
+		// ensures that tokens are not being synced
+		require.True(r, status.ReplicatedTokenIndex > 0, "ReplicatedTokenIndex not greater than 0")
+
+	})
+
+	// modify the replication policy to change to only granting read privileges
+	policyArg = structs.ACLPolicySetRequest{
+		Datacenter: "dc1",
+		Policy: structs.ACLPolicy{
+			ID:          policy.ID,
+			Name:        "token-replication-redacted",
+			Description: "token-replication-redacted",
+			Rules:       `acl = "read"`,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	require.NoError(t, s1.RPC("ACL.PolicySet", &policyArg, &policy))
+
+	// Create the another token so that replication will attempt to read it.
+	tokenArg = structs.ACLTokenSetRequest{
+		Datacenter: "dc1",
+		ACLToken: structs.ACLToken{
+			Description: "management",
+			Policies: []structs.ACLTokenPolicyLink{
+				structs.ACLTokenPolicyLink{
+					ID: structs.ACLPolicyGlobalManagementID,
+				},
+			},
+			Local: false,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	var token2 structs.ACLToken
+
+	// record the time right before we are touching the token
+	minErrorTime := time.Now()
+	require.NoError(t, s1.RPC("ACL.TokenSet", &tokenArg, &token2))
+
+	retry.Run(t, func(r *retry.R) {
+		var tokenResp structs.ACLTokenResponse
+		req := structs.ACLTokenGetRequest{
+			Datacenter:   "dc2",
+			TokenID:      redactedToken,
+			TokenIDType:  structs.ACLTokenSecret,
+			QueryOptions: structs.QueryOptions{Token: redactedToken},
+		}
+		err := s2.RPC("ACL.TokenRead", &req, &tokenResp)
+		// its not an error for the secret to not be found.
+		require.NoError(r, err)
+		require.Nil(r, tokenResp.Token)
+
+		var status structs.ACLReplicationStatus
+		statusReq := structs.DCSpecificRequest{
+			Datacenter: "dc2",
+		}
+		require.NoError(r, s2.RPC("ACL.ReplicationStatus", &statusReq, &status))
+		// ensures that tokens are not being synced
+		require.True(r, status.ReplicatedTokenIndex < token2.CreateIndex, "ReplicatedTokenIndex is not less than the token2s create index")
+		// ensures that token replication is erroring
+		require.True(r, status.LastError.After(minErrorTime), "Replication LastError not after the minErrorTime")
+	})
+}

agent/consul/leader.go

@@ -410,6 +410,21 @@ func (s *Server) initializeACLs(upgrade bool) error {
 	// leader.
 	s.acls.cache.Purge()
 
+	// Remove any token affected by CVE-2019-8336
+	if !s.InACLDatacenter() {
+		_, token, err := s.fsm.State().ACLTokenGetBySecret(nil, redactedToken)
+		if err == nil && token != nil {
+			req := structs.ACLTokenBatchDeleteRequest{
+				TokenIDs: []string{token.AccessorID},
+			}
+
+			_, err := s.raftApply(structs.ACLTokenDeleteRequestType, &req)
+			if err != nil {
+				return fmt.Errorf("failed to remove token with a redacted secret: %v", err)
+			}
+		}
+	}
+
 	if s.InACLDatacenter() {
 		if s.UseLegacyACLs() && !upgrade {
 			s.logger.Printf("[INFO] acl: initializing legacy acls")

agent/consul/server_test.go

@@ -239,13 +239,12 @@ func TestServer_JoinLAN(t *testing.T) {
 }
 
 func TestServer_LANReap(t *testing.T) {
-	t.Parallel()
 	dir1, s1 := testServerWithConfig(t, func(c *Config) {
 		c.Datacenter = "dc1"
 		c.Bootstrap = true
 		c.SerfFloodInterval = 100 * time.Millisecond
 		c.SerfLANConfig.ReconnectTimeout = 250 * time.Millisecond
-		c.SerfLANConfig.ReapInterval = 500 * time.Millisecond
+		c.SerfLANConfig.ReapInterval = 300 * time.Millisecond
 	})
 	defer os.RemoveAll(dir1)
 	defer s1.Shutdown()
@@ -255,7 +254,7 @@ func TestServer_LANReap(t *testing.T) {
 		c.Bootstrap = false
 		c.SerfFloodInterval = 100 * time.Millisecond
 		c.SerfLANConfig.ReconnectTimeout = 250 * time.Millisecond
-		c.SerfLANConfig.ReapInterval = 500 * time.Millisecond
+		c.SerfLANConfig.ReapInterval = 300 * time.Millisecond
 	})
 	defer os.RemoveAll(dir2)
 
@@ -264,7 +263,7 @@ func TestServer_LANReap(t *testing.T) {
 		c.Bootstrap = false
 		c.SerfFloodInterval = 100 * time.Millisecond
 		c.SerfLANConfig.ReconnectTimeout = 250 * time.Millisecond
-		c.SerfLANConfig.ReapInterval = 500 * time.Millisecond
+		c.SerfLANConfig.ReapInterval = 300 * time.Millisecond
 	})
 	defer os.RemoveAll(dir3)
 	defer s3.Shutdown()
@@ -273,6 +272,8 @@ func TestServer_LANReap(t *testing.T) {
 	joinLAN(t, s2, s1)
 	joinLAN(t, s3, s1)
 
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+	testrpc.WaitForLeader(t, s2.RPC, "dc1")
 	testrpc.WaitForLeader(t, s3.RPC, "dc1")
 
 	retry.Run(t, func(r *retry.R) {

agent/structs/acl.go

@@ -621,13 +621,15 @@ type ACLTokenBootstrapRequest struct {
 
 // ACLTokenResponse returns a single Token + metadata
 type ACLTokenResponse struct {
-	Token *ACLToken
+	Token    *ACLToken
+	Redacted bool // whether the token's secret was redacted
 	QueryMeta
 }
 
 // ACLTokenBatchResponse returns multiple Tokens associated with the same metadata
 type ACLTokenBatchResponse struct {
-	Tokens []*ACLToken
+	Tokens   []*ACLToken
+	Redacted bool // whether the token secrets were redacted.
 	QueryMeta
 }