Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check acls on resource Read, List, and WatchList #16842

Merged
merged 22 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions agent/consul/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1269,9 +1269,10 @@ func (s *Server) setupExternalGRPC(config *Config, backend storage.Backend, logg
}

resourcegrpc.NewServer(resourcegrpc.Config{
Registry: registry,
Backend: backend,
Logger: logger.Named("grpc-api.resource"),
Registry: registry,
Backend: backend,
ACLResolver: s.ACLResolver,
Logger: logger.Named("grpc-api.resource"),
}).Register(s.externalGRPCServer)
}

Expand Down
46 changes: 41 additions & 5 deletions agent/grpc-external/services/resource/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,62 @@ package resource
import (
"context"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/internal/storage"
"github.com/hashicorp/consul/proto-public/pbresource"
)

func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbresource.ListResponse, error) {
if _, err := s.resolveType(req.Type); err != nil {
// check type
reg, err := s.resolveType(req.Type)
if err != nil {
return nil, err
}

resources, err := s.Backend.List(ctx, readConsistencyFrom(ctx), storage.UnversionedTypeFrom(req.Type), req.Tenancy, req.NamePrefix)
authz, err := s.getAuthorizer(tokenFromContext(ctx))
if err != nil {
return nil, err
}

// filter out non-matching GroupVersion
// check acls
err = reg.ACLs.List(authz, req.Tenancy)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())
case err != nil:
return nil, status.Errorf(codes.Internal, "failed list acl: %v", err)
}

resources, err := s.Backend.List(
ctx,
readConsistencyFrom(ctx),
storage.UnversionedTypeFrom(req.Type),
req.Tenancy,
req.NamePrefix,
)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed list: %v", err)
}

result := make([]*pbresource.Resource, 0)
for _, resource := range resources {
if resource.Id.Type.GroupVersion == req.Type.GroupVersion {
result = append(result, resource)
// filter out non-matching GroupVersion
if resource.Id.Type.GroupVersion != req.Type.GroupVersion {
continue
}

// filter out items that don't pass read ACLs
err = reg.ACLs.Read(authz, resource.Id)
switch {
case acl.IsErrPermissionDenied(err):
continue
case err != nil:
return nil, status.Errorf(codes.Internal, "failed read acl: %v", err)
}
result = append(result, resource)
}
return &pbresource.ListResponse{Resources: result}, nil
}
82 changes: 76 additions & 6 deletions agent/grpc-external/services/resource/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
"fmt"
"testing"

"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/grpc-external/testutils"
"github.com/hashicorp/consul/internal/resource/demo"
"github.com/hashicorp/consul/internal/storage"
"github.com/hashicorp/consul/proto-public/pbresource"
Expand All @@ -32,7 +33,7 @@ func TestList_TypeNotFound(t *testing.T) {
})
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource type demo/v2/artist not registered")
require.Contains(t, err.Error(), "resource type demo.v2.artist not registered")
}

func TestList_Empty(t *testing.T) {
Expand Down Expand Up @@ -113,10 +114,8 @@ func TestList_VerifyReadConsistencyArg(t *testing.T) {
for desc, tc := range listTestCases() {
t.Run(desc, func(t *testing.T) {
mockBackend := NewMockBackend(t)
server := NewServer(Config{
Registry: resource.NewRegistry(),
Backend: mockBackend,
})
server := testServer(t)
server.Backend = mockBackend
demo.Register(server.Registry)

artist, err := demo.GenerateV2Artist()
Expand All @@ -134,6 +133,77 @@ func TestList_VerifyReadConsistencyArg(t *testing.T) {
}
}

// N.B. Uses key ACLs for now. See demo.Register()
func TestList_ACL_ListDenied(t *testing.T) {
t.Parallel()

// deny all
_, _, err := roundTripList(t, testutils.ACLNoPermissions(t))

// verify key:list denied
require.Error(t, err)
require.Equal(t, codes.PermissionDenied.String(), status.Code(err).String())
require.Contains(t, err.Error(), "lacks permission 'key:list'")
}

// N.B. Uses key ACLs for now. See demo.Register()
func TestList_ACL_ListAllowed_ReadDenied(t *testing.T) {
t.Parallel()

// allow list, deny read
authz := AuthorizerFrom(t, demo.ArtistV2ListPolicy,
`key_prefix "resource/demo.v2.artist/" { policy = "deny" }`)
_, rsp, err := roundTripList(t, authz)

// verify resource filtered out by key:read denied hence no results
require.NoError(t, err)
require.Empty(t, rsp.Resources)
}

// N.B. Uses key ACLs for now. See demo.Register()
func TestList_ACL_ListAllowed_ReadAllowed(t *testing.T) {
t.Parallel()

// allow list, allow read
authz := AuthorizerFrom(t, demo.ArtistV2ListPolicy, demo.ArtistV2ReadPolicy)
artist, rsp, err := roundTripList(t, authz)

// verify resource not filtered out by acl
require.NoError(t, err)
require.Len(t, rsp.Resources, 1)
prototest.AssertDeepEqual(t, artist, rsp.Resources[0])
}

// roundtrip a List which attempts to return a single resource
func roundTripList(t *testing.T, authz acl.Authorizer) (*pbresource.Resource, *pbresource.ListResponse, error) {
server := testServer(t)
client := testClient(t, server)
ctx := testContext(t)

mockACLResolver := &MockACLResolver{}
mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(authz, nil)
server.ACLResolver = mockACLResolver
demo.Register(server.Registry)

artist, err := demo.GenerateV2Artist()
require.NoError(t, err)

artist, err = server.Backend.WriteCAS(ctx, artist)
require.NoError(t, err)

rsp, err := client.List(
ctx,
&pbresource.ListRequest{
Type: artist.Id.Type,
Tenancy: artist.Id.Tenancy,
NamePrefix: "",
},
)

return artist, rsp, err
}

type listTestCase struct {
consistency storage.ReadConsistency
ctx context.Context
Expand Down
54 changes: 54 additions & 0 deletions agent/grpc-external/services/resource/mock_ACLResolver.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 59 additions & 0 deletions agent/grpc-external/services/resource/mock_Registry.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 25 additions & 9 deletions agent/grpc-external/services/resource/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,41 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/internal/storage"
"github.com/hashicorp/consul/proto-public/pbresource"
)

func (s *Server) Read(ctx context.Context, req *pbresource.ReadRequest) (*pbresource.ReadResponse, error) {
// check type exists
if _, err := s.resolveType(req.Id.Type); err != nil {
reg, err := s.resolveType(req.Id.Type)
if err != nil {
return nil, err
}

resource, err := s.Backend.Read(ctx, readConsistencyFrom(ctx), req.Id)
authz, err := s.getAuthorizer(tokenFromContext(ctx))
if err != nil {
if errors.Is(err, storage.ErrNotFound) {
return nil, status.Error(codes.NotFound, err.Error())
}
if errors.As(err, &storage.GroupVersionMismatchError{}) {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
return nil, err
}
return &pbresource.ReadResponse{Resource: resource}, nil

// check acls
err = reg.ACLs.Read(authz, req.Id)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())
case err != nil:
return nil, status.Errorf(codes.Internal, "failed read acl: %v", err)
}

resource, err := s.Backend.Read(ctx, readConsistencyFrom(ctx), req.Id)
switch {
case err == nil:
return &pbresource.ReadResponse{Resource: resource}, nil
case errors.Is(err, storage.ErrNotFound):
return nil, status.Error(codes.NotFound, err.Error())
case errors.As(err, &storage.GroupVersionMismatchError{}):
return nil, status.Error(codes.InvalidArgument, err.Error())
default:
return nil, status.Errorf(codes.Internal, "failed read: %v", err)
}
}
Loading