diff --git a/agent/consul/acl.go b/agent/consul/acl.go index 99711cb27..dd90d267e 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -1244,6 +1244,10 @@ func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs return identity, acl.NewChainedAuthorizer(chain), nil } +// TODO: rename to AccessorIDFromToken. This method is only used to retrieve the +// ACLIdentity.ID, so we don't need to return a full ACLIdentity. We could +// return a much smaller type (instad of just a string) to allow for changes +// in the future. func (r *ACLResolver) ResolveTokenToIdentity(token string) (structs.ACLIdentity, error) { if !r.ACLsEnabled() { return nil, nil @@ -1928,12 +1932,11 @@ func (f *aclFilter) filterGatewayServices(mappings *structs.GatewayServices) { *mappings = ret } -func (r *ACLResolver) filterACLWithAuthorizer(authorizer acl.Authorizer, subj interface{}) error { +func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, subj interface{}) { if authorizer == nil { - return nil + return } - // Create the filter - filt := newACLFilter(authorizer, r.logger) + filt := newACLFilter(authorizer, logger) switch v := subj.(type) { case *structs.CheckServiceNodes: @@ -2028,23 +2031,17 @@ func (r *ACLResolver) filterACLWithAuthorizer(authorizer acl.Authorizer, subj in default: panic(fmt.Errorf("Unhandled type passed to ACL filter: %T %#v", subj, subj)) } - - return nil } -// filterACL is used to filter results from our service catalog based on the -// rules configured for the provided token. -func (r *ACLResolver) filterACL(token string, subj interface{}) error { +// filterACL uses the ACLResolver to resolve the token in an acl.Authorizer, +// then uses the acl.Authorizer to filter subj. Any entities in subj that are +// not authorized for read access will be removed from subj. +func filterACL(r *ACLResolver, token string, subj interface{}) error { // Get the ACL from the token _, authorizer, err := r.ResolveTokenToIdentityAndAuthorizer(token) if err != nil { return err } - - // Fast path if ACLs are not enabled - if authorizer == nil { - return nil - } - - return r.filterACLWithAuthorizer(authorizer, subj) + filterACLWithAuthorizer(r.logger, authorizer, subj) + return nil } diff --git a/agent/consul/acl_client.go b/agent/consul/acl_client.go index 21106f37c..bc86916ea 100644 --- a/agent/consul/acl_client.go +++ b/agent/consul/acl_client.go @@ -93,6 +93,7 @@ func (c *Client) ResolveTokenToIdentity(token string) (structs.ACLIdentity, erro return c.acls.ResolveTokenToIdentity(token) } +// TODO: Server has an identical implementation, remove duplication func (c *Client) ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) { identity, authz, err := c.acls.ResolveTokenToIdentityAndAuthorizer(token) if err != nil { diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index 8800cb0ec..66dd7c91a 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -956,9 +956,7 @@ func (a *ACL) TokenList(args *structs.ACLTokenListRequest, reply *structs.ACLTok } // filter down to just the tokens that the requester has permissions to read - if err := a.srv.filterACLWithAuthorizer(authz, &stubs); err != nil { - return err - } + a.srv.filterACLWithAuthorizer(authz, &stubs) reply.Index, reply.Tokens = index, stubs return nil diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index 57c7a24e5..0351c2724 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -224,7 +224,7 @@ func (s *Server) ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error } func (s *Server) ResolveToken(token string) (acl.Authorizer, error) { - _, authz, err := s.ResolveTokenToIdentityAndAuthorizer(token) + _, authz, err := s.acls.ResolveTokenToIdentityAndAuthorizer(token) return authz, err } @@ -235,16 +235,11 @@ func (s *Server) ResolveTokenToIdentity(token string) (structs.ACLIdentity, erro return s.acls.ResolveTokenToIdentity(token) } -func (s *Server) ResolveTokenToIdentityAndAuthorizer(token string) (structs.ACLIdentity, acl.Authorizer, error) { - return s.acls.ResolveTokenToIdentityAndAuthorizer(token) -} - -// ResolveTokenIdentityAndDefaultMeta retrieves an identity and authorizer for the caller, -// and populates the EnterpriseMeta based on the AuthorizerContext. -func (s *Server) ResolveTokenIdentityAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (structs.ACLIdentity, acl.Authorizer, error) { - identity, authz, err := s.ResolveTokenToIdentityAndAuthorizer(token) +// TODO: Client has an identical implementation, remove duplication +func (s *Server) ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) { + identity, authz, err := s.acls.ResolveTokenToIdentityAndAuthorizer(token) if err != nil { - return nil, nil, err + return nil, err } // Default the EnterpriseMeta based on the Tokens meta or actual defaults @@ -258,19 +253,13 @@ func (s *Server) ResolveTokenIdentityAndDefaultMeta(token string, entMeta *struc // Use the meta to fill in the ACL authorization context entMeta.FillAuthzContext(authzContext) - return identity, authz, err -} - -// ResolveTokenAndDefaultMeta passes through to ResolveTokenIdentityAndDefaultMeta, eliding the identity from its response. -func (s *Server) ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error) { - _, authz, err := s.ResolveTokenIdentityAndDefaultMeta(token, entMeta, authzContext) return authz, err } func (s *Server) filterACL(token string, subj interface{}) error { - return s.acls.filterACL(token, subj) + return filterACL(s.acls, token, subj) } -func (s *Server) filterACLWithAuthorizer(authorizer acl.Authorizer, subj interface{}) error { - return s.acls.filterACLWithAuthorizer(authorizer, subj) +func (s *Server) filterACLWithAuthorizer(authorizer acl.Authorizer, subj interface{}) { + filterACLWithAuthorizer(s.acls.logger, authorizer, subj) } diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index 2152b099e..6699469c1 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -3276,7 +3276,7 @@ func TestACL_redactPreparedQueryTokens(t *testing.T) { } } -func TestACL_redactTokenSecret(t *testing.T) { +func TestFilterACL_redactTokenSecret(t *testing.T) { t.Parallel() delegate := &ACLResolverTestDelegate{ enabled: true, @@ -3293,16 +3293,16 @@ func TestACL_redactTokenSecret(t *testing.T) { SecretID: "6a5e25b3-28f2-4085-9012-c3fb754314d1", } - err := r.filterACL("acl-wr", &token) + err := filterACL(r, "acl-wr", &token) require.NoError(t, err) require.Equal(t, "6a5e25b3-28f2-4085-9012-c3fb754314d1", token.SecretID) - err = r.filterACL("acl-ro", &token) + err = filterACL(r, "acl-ro", &token) require.NoError(t, err) require.Equal(t, redactedToken, token.SecretID) } -func TestACL_redactTokenSecrets(t *testing.T) { +func TestFilterACL_redactTokenSecrets(t *testing.T) { t.Parallel() delegate := &ACLResolverTestDelegate{ enabled: true, @@ -3321,11 +3321,11 @@ func TestACL_redactTokenSecrets(t *testing.T) { }, } - err := r.filterACL("acl-wr", &tokens) + err := filterACL(r, "acl-wr", &tokens) require.NoError(t, err) require.Equal(t, "6a5e25b3-28f2-4085-9012-c3fb754314d1", tokens[0].SecretID) - err = r.filterACL("acl-ro", &tokens) + err = filterACL(r, "acl-ro", &tokens) require.NoError(t, err) require.Equal(t, redactedToken, tokens[0].SecretID) } diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 26eb39417..c3f9b9daa 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -545,7 +545,8 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I return nil } - return c.srv.filterACLWithAuthorizer(authz, reply) + c.srv.filterACLWithAuthorizer(authz, reply) + return nil }) } @@ -573,7 +574,8 @@ func (c *Catalog) ServiceList(args *structs.DCSpecificRequest, reply *structs.In } reply.Index, reply.Services = index, services - return c.srv.filterACLWithAuthorizer(authz, reply) + c.srv.filterACLWithAuthorizer(authz, reply) + return nil }) } diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index 247ba8560..67f000f5f 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -100,15 +100,18 @@ func (s *Intention) Apply(args *structs.IntentionRequest, reply *string) error { } // Get the ACL token for the request for the checks below. - var entMeta structs.EnterpriseMeta - ident, authz, err := s.srv.ResolveTokenIdentityAndDefaultMeta(args.Token, &entMeta, nil) + identity, authz, err := s.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } var accessorID string - if ident != nil { - accessorID = ident.ID() + var entMeta structs.EnterpriseMeta + if identity != nil { + entMeta.Merge(identity.EnterpriseMetadata()) + accessorID = identity.ID() + } else { + entMeta.Merge(structs.DefaultEnterpriseMetaInDefaultPartition()) } var ( diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index 06746aa82..bcfa3064f 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -222,7 +222,8 @@ func (m *Internal) IntentionUpstreams(args *structs.ServiceSpecificRequest, repl } reply.Index, reply.Services = index, services - return m.srv.filterACLWithAuthorizer(authz, reply) + m.srv.filterACLWithAuthorizer(authz, reply) + return nil }) } @@ -439,7 +440,7 @@ func (m *Internal) KeyringOperation( } // Check ACLs - identity, rule, err := m.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := m.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } diff --git a/agent/consul/operator_autopilot_endpoint.go b/agent/consul/operator_autopilot_endpoint.go index 53babd0e8..767e898a6 100644 --- a/agent/consul/operator_autopilot_endpoint.go +++ b/agent/consul/operator_autopilot_endpoint.go @@ -17,7 +17,7 @@ func (op *Operator) AutopilotGetConfiguration(args *structs.DCSpecificRequest, r } // This action requires operator read access. - identity, rule, err := op.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := op.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } @@ -49,7 +49,7 @@ func (op *Operator) AutopilotSetConfiguration(args *structs.AutopilotSetConfigRe } // This action requires operator write access. - identity, rule, err := op.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := op.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } @@ -84,7 +84,7 @@ func (op *Operator) ServerHealth(args *structs.DCSpecificRequest, reply *structs } // This action requires operator read access. - identity, rule, err := op.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := op.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } @@ -151,7 +151,7 @@ func (op *Operator) AutopilotState(args *structs.DCSpecificRequest, reply *autop } // This action requires operator read access. - identity, rule, err := op.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := op.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } diff --git a/agent/consul/operator_raft_endpoint.go b/agent/consul/operator_raft_endpoint.go index 72cd7a3ff..06a487b47 100644 --- a/agent/consul/operator_raft_endpoint.go +++ b/agent/consul/operator_raft_endpoint.go @@ -81,7 +81,7 @@ func (op *Operator) RaftRemovePeerByAddress(args *structs.RaftRemovePeerRequest, // This is a super dangerous operation that requires operator write // access. - identity, rule, err := op.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := op.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } @@ -134,7 +134,7 @@ func (op *Operator) RaftRemovePeerByID(args *structs.RaftRemovePeerRequest, repl // This is a super dangerous operation that requires operator write // access. - identity, rule, err := op.srv.ResolveTokenToIdentityAndAuthorizer(args.Token) + identity, rule, err := op.srv.acls.ResolveTokenToIdentityAndAuthorizer(args.Token) if err != nil { return err } diff --git a/agent/consul/session_endpoint.go b/agent/consul/session_endpoint.go index 9d43aef71..e15b05227 100644 --- a/agent/consul/session_endpoint.go +++ b/agent/consul/session_endpoint.go @@ -199,9 +199,7 @@ func (s *Session) Get(args *structs.SessionSpecificRequest, } else { reply.Sessions = nil } - if err := s.srv.filterACLWithAuthorizer(authz, reply); err != nil { - return err - } + s.srv.filterACLWithAuthorizer(authz, reply) return nil }) } @@ -233,9 +231,7 @@ func (s *Session) List(args *structs.SessionSpecificRequest, } reply.Index, reply.Sessions = index, sessions - if err := s.srv.filterACLWithAuthorizer(authz, reply); err != nil { - return err - } + s.srv.filterACLWithAuthorizer(authz, reply) return nil }) } @@ -267,9 +263,7 @@ func (s *Session) NodeSessions(args *structs.NodeSpecificRequest, } reply.Index, reply.Sessions = index, sessions - if err := s.srv.filterACLWithAuthorizer(authz, reply); err != nil { - return err - } + s.srv.filterACLWithAuthorizer(authz, reply) return nil }) }