acl_endpoint: permission denied for unauthenticated requests

If ACL Request is unauthenticated, we should honor the anonymous token.
This PR makes few changes:

* `GetPolicy` endpoints may return policy if anonymous policy allows it,
or return permission denied otherwise.
* `ListPolicies` returns an empty policy list, or one with anonymous
policy if one exists.

Without this PR, the we return an incomprehensible error.

Before:
```
$ curl http://localhost:4646/v1/acl/policy/doesntexist; echo
acl token lookup failed: index error: UUID must be 36 characters
$ curl http://localhost:4646/v1/acl/policies; echo
acl token lookup failed: index error: UUID must be 36 characters
```

After:
```
$ curl http://localhost:4646/v1/acl/policy/doesntexist; echo
Permission denied
$ curl http://localhost:4646/v1/acl/policies; echo
[]
```

nomad/acl_endpoint.go

@@ -119,6 +119,7 @@ func (a *ACL) ListPolicies(args *structs.ACLPolicyListRequest, reply *structs.AC
 	if !a.srv.config.ACLEnabled {
 		return aclDisabled
 	}
+
 	if done, err := a.srv.forward("ACL.ListPolicies", args, args, reply); done {
 		return err
 	}
@@ -136,12 +137,7 @@ func (a *ACL) ListPolicies(args *structs.ACLPolicyListRequest, reply *structs.AC
 	mgt := acl.IsManagement()
 	var policies map[string]struct{}
 	if !mgt {
-		snap, err := a.srv.fsm.State().Snapshot()
-		if err != nil {
-			return err
-		}
-
-		token, err := snap.ACLTokenBySecretID(nil, args.AuthToken)
+		token, err := a.requestAuthToken(args.AuthToken)
 		if err != nil {
 			return err
 		}
@@ -207,6 +203,7 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S
 	if !a.srv.config.ACLEnabled {
 		return aclDisabled
 	}
+
 	if done, err := a.srv.forward("ACL.GetPolicy", args, args, reply); done {
 		return err
 	}
@@ -224,12 +221,7 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S
 	// If it is not a management token determine if it can get this policy
 	mgt := acl.IsManagement()
 	if !mgt && args.Name != "anonymous" {
-		snap, err := a.srv.fsm.State().Snapshot()
-		if err != nil {
-			return err
-		}
-
-		token, err := snap.ACLTokenBySecretID(nil, args.AuthToken)
+		token, err := a.requestAuthToken(args.AuthToken)
 		if err != nil {
 			return err
 		}
@@ -284,6 +276,19 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S
 	return a.srv.blockingRPC(&opts)
 }
 
+func (a *ACL) requestAuthToken(secretID string) (*structs.ACLToken, error) {
+	if secretID == "" {
+		return structs.AnonymousACLToken, nil
+	}
+
+	snap, err := a.srv.fsm.State().Snapshot()
+	if err != nil {
+		return nil, err
+	}
+
+	return snap.ACLTokenBySecretID(nil, secretID)
+}
+
 // GetPolicies is used to get a set of policies
 func (a *ACL) GetPolicies(args *structs.ACLPolicySetRequest, reply *structs.ACLPolicySetResponse) error {
 	if !a.srv.config.ACLEnabled {
@@ -294,19 +299,12 @@ func (a *ACL) GetPolicies(args *structs.ACLPolicySetRequest, reply *structs.ACLP
 	}
 	defer metrics.MeasureSince([]string{"nomad", "acl", "get_policies"}, time.Now())
 
-	var token *structs.ACLToken
-	var err error
-	if args.AuthToken == "" {
-		// No need to look up the anonymous token
-		token = structs.AnonymousACLToken
-	} else {
-		// For client typed tokens, allow them to query any policies associated with that token.
-		// This is used by clients which are resolving the policies to enforce. Any associated
-		// policies need to be fetched so that the client can determine what to allow.
-		token, err = a.srv.State().ACLTokenBySecretID(nil, args.AuthToken)
-		if err != nil {
-			return err
-		}
+	// For client typed tokens, allow them to query any policies associated with that token.
+	// This is used by clients which are resolving the policies to enforce. Any associated
+	// policies need to be fetched so that the client can determine what to allow.
+	token, err := a.requestAuthToken(args.AuthToken)
+	if err != nil {
+		return err
 	}
 
 	if token == nil {

nomad/acl_endpoint_test.go

@@ -89,6 +89,18 @@ func TestACLEndpoint_GetPolicy(t *testing.T) {
 	}
 	assert.EqualValues(t, 1001, resp3.Index)
 	assert.Equal(t, anonymousPolicy, resp3.Policy)
+
+	// Lookup non-anonoymous policy with no token
+	get = &structs.ACLPolicySpecificRequest{
+		Name: policy.Name,
+		QueryOptions: structs.QueryOptions{
+			Region: "global",
+		},
+	}
+	var resp4 structs.SingleACLPolicyResponse
+	err := msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", get, &resp4)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), structs.ErrPermissionDenied.Error())
 }
 
 func TestACLEndpoint_GetPolicy_Blocking(t *testing.T) {
@@ -395,6 +407,57 @@ func TestACLEndpoint_ListPolicies(t *testing.T) {
 	}
 }
 
+// TestACLEndpoint_ListPolicies_Unauthenticated asserts that
+// unauthenticated ListPolicies returns anonymous policy if one
+// exists, otherwise, empty
+func TestACLEndpoint_ListPolicies_Unauthenticated(t *testing.T) {
+	t.Parallel()
+	s1, _ := TestACLServer(t, nil)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	testutil.WaitForLeader(t, s1.RPC)
+
+	listPolicies := func() (*structs.ACLPolicyListResponse, error) {
+		// Lookup the policies
+		get := &structs.ACLPolicyListRequest{
+			QueryOptions: structs.QueryOptions{
+				Region: "global",
+			},
+		}
+
+		var resp structs.ACLPolicyListResponse
+		err := msgpackrpc.CallWithCodec(codec, "ACL.ListPolicies", get, &resp)
+		if err != nil {
+			return nil, err
+		}
+		return &resp, nil
+	}
+
+	p1 := mock.ACLPolicy()
+	p1.Name = "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"
+	s1.fsm.State().UpsertACLPolicies(1000, []*structs.ACLPolicy{p1})
+
+	t.Run("no anonymous policy", func(t *testing.T) {
+		resp, err := listPolicies()
+		require.NoError(t, err)
+		require.Empty(t, resp.Policies)
+		require.Equal(t, uint64(1000), resp.Index)
+	})
+
+	// now try with anonymous policy
+	p2 := mock.ACLPolicy()
+	p2.Name = "anonymous"
+	s1.fsm.State().UpsertACLPolicies(1001, []*structs.ACLPolicy{p2})
+
+	t.Run("with anonymous policy", func(t *testing.T) {
+		resp, err := listPolicies()
+		require.NoError(t, err)
+		require.Len(t, resp.Policies, 1)
+		require.Equal(t, "anonymous", resp.Policies[0].Name)
+		require.Equal(t, uint64(1001), resp.Index)
+	})
+}
+
 func TestACLEndpoint_ListPolicies_Blocking(t *testing.T) {
 	t.Parallel()
 	s1, root := TestACLServer(t, nil)

nomad/state/state_store.go

@@ -3769,6 +3769,10 @@ func (s *StateStore) DeleteACLTokens(index uint64, ids []string) error {
 
 // ACLTokenByAccessorID is used to lookup a token by accessor ID
 func (s *StateStore) ACLTokenByAccessorID(ws memdb.WatchSet, id string) (*structs.ACLToken, error) {
+	if id == "" {
+		return nil, fmt.Errorf("acl token lookup failed: missing accessor id")
+	}
+
 	txn := s.db.Txn(false)
 
 	watchCh, existing, err := txn.FirstWatch("acl_token", "id", id)
@@ -3785,6 +3789,10 @@ func (s *StateStore) ACLTokenByAccessorID(ws memdb.WatchSet, id string) (*struct
 
 // ACLTokenBySecretID is used to lookup a token by secret ID
 func (s *StateStore) ACLTokenBySecretID(ws memdb.WatchSet, secretID string) (*structs.ACLToken, error) {
+	if secretID == "" {
+		return nil, fmt.Errorf("acl token lookup failed: missing secret id")
+	}
+
 	txn := s.db.Txn(false)
 
 	watchCh, existing, err := txn.FirstWatch("acl_token", "secret", secretID)