From e134e43da6f9550df8f11043c35d4a8f2eed7b89 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sat, 22 Jan 2022 15:05:42 -0500 Subject: [PATCH] acl: remove calls to ResolveIdentityFromToken We already have an ACLResolveResult, so we can get the accessor ID from it. --- agent/consul/intention_endpoint.go | 31 ++++++------------------------ agent/consul/internal_endpoint.go | 22 ++------------------- 2 files changed, 8 insertions(+), 45 deletions(-) diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index 57d3fc7667..2031641caa 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -433,7 +433,8 @@ func (s *Intention) Get(args *structs.IntentionQueryRequest, reply *structs.Inde // Get the ACL token for the request for the checks below. var entMeta structs.EnterpriseMeta - if _, err := s.srv.ResolveTokenAndDefaultMeta(args.Token, &entMeta, nil); err != nil { + authz, err := s.srv.ResolveTokenAndDefaultMeta(args.Token, &entMeta, nil) + if err != nil { return err } @@ -480,13 +481,11 @@ func (s *Intention) Get(args *structs.IntentionQueryRequest, reply *structs.Inde reply.Intentions = structs.Intentions{ixn} // Filter - if err := s.srv.filterACL(args.Token, reply); err != nil { - return err - } + s.srv.filterACLWithAuthorizer(authz, reply) // If ACLs prevented any responses, error if len(reply.Intentions) == 0 { - accessorID := s.aclAccessorID(args.Token) + accessorID := authz.AccessorID() // todo(kit) Migrate intention access denial logging over to audit logging when we implement it s.logger.Warn("Request to get intention denied due to ACLs", "intention", args.IntentionID, "accessorID", accessorID) return acl.ErrPermissionDenied @@ -619,7 +618,7 @@ func (s *Intention) Match(args *structs.IntentionQueryRequest, reply *structs.In for _, entry := range args.Match.Entries { entry.FillAuthzContext(&authzContext) if prefix := entry.Name; prefix != "" && authz.IntentionRead(prefix, &authzContext) != acl.Allow { - accessorID := s.aclAccessorID(args.Token) + accessorID := authz.AccessorID() // todo(kit) Migrate intention access denial logging over to audit logging when we implement it s.logger.Warn("Operation on intention prefix denied due to ACLs", "prefix", prefix, "accessorID", accessorID) return acl.ErrPermissionDenied @@ -709,7 +708,7 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In var authzContext acl.AuthorizerContext query.FillAuthzContext(&authzContext) if authz.ServiceRead(prefix, &authzContext) != acl.Allow { - accessorID := s.aclAccessorID(args.Token) + accessorID := authz.AccessorID() // todo(kit) Migrate intention access denial logging over to audit logging when we implement it s.logger.Warn("test on intention denied due to ACLs", "prefix", prefix, "accessorID", accessorID) return acl.ErrPermissionDenied @@ -761,24 +760,6 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In return nil } -// aclAccessorID is used to convert an ACLToken's secretID to its accessorID for non- -// critical purposes, such as logging. Therefore we interpret all errors as empty-string -// so we can safely log it without handling non-critical errors at the usage site. -func (s *Intention) aclAccessorID(secretID string) string { - _, ident, err := s.srv.ResolveIdentityFromToken(secretID) - if acl.IsErrNotFound(err) { - return "" - } - if err != nil { - s.logger.Debug("non-critical error resolving acl token accessor for logging", "error", err) - return "" - } - if ident == nil { - return "" - } - return ident.ID() -} - func (s *Intention) validateEnterpriseIntention(ixn *structs.Intention) error { if err := s.srv.validateEnterpriseIntentionPartition(ixn.SourcePartition); err != nil { return fmt.Errorf("Invalid source partition %q: %v", ixn.SourcePartition, err) diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index dea21cfc6f..5213349e79 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -401,13 +401,13 @@ func (m *Internal) EventFire(args *structs.EventFireRequest, } // Check ACLs - authz, err := m.srv.ResolveToken(args.Token) + authz, err := m.srv.ResolveTokenAndDefaultMeta(args.Token, nil, nil) if err != nil { return err } if authz.EventWrite(args.Name, nil) != acl.Allow { - accessorID := m.aclAccessorID(args.Token) + accessorID := authz.AccessorID() m.logger.Warn("user event blocked by ACLs", "event", args.Name, "accessorID", accessorID) return acl.ErrPermissionDenied } @@ -545,21 +545,3 @@ func (m *Internal) executeKeyringOpMgr( return serfResp, err } - -// aclAccessorID is used to convert an ACLToken's secretID to its accessorID for non- -// critical purposes, such as logging. Therefore we interpret all errors as empty-string -// so we can safely log it without handling non-critical errors at the usage site. -func (m *Internal) aclAccessorID(secretID string) string { - _, ident, err := m.srv.ResolveIdentityFromToken(secretID) - if acl.IsErrNotFound(err) { - return "" - } - if err != nil { - m.logger.Debug("non-critical error resolving acl token accessor for logging", "error", err) - return "" - } - if ident == nil { - return "" - } - return ident.ID() -}