diff --git a/agent/consul/server.go b/agent/consul/server.go index 1bd52e9d0..19f973833 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -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) } diff --git a/agent/grpc-external/services/resource/list.go b/agent/grpc-external/services/resource/list.go index 263321cc1..65cc37a26 100644 --- a/agent/grpc-external/services/resource/list.go +++ b/agent/grpc-external/services/resource/list.go @@ -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 { - return nil, err - } - - resources, err := s.Backend.List(ctx, readConsistencyFrom(ctx), storage.UnversionedTypeFrom(req.Type), req.Tenancy, req.NamePrefix) + // check type + reg, err := s.resolveType(req.Type) if err != nil { return nil, err } - // filter out non-matching GroupVersion + authz, err := s.getAuthorizer(tokenFromContext(ctx)) + if err != nil { + return nil, err + } + + // 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 } diff --git a/agent/grpc-external/services/resource/list_test.go b/agent/grpc-external/services/resource/list_test.go index 3f907edd8..ef0ba96ce 100644 --- a/agent/grpc-external/services/resource/list_test.go +++ b/agent/grpc-external/services/resource/list_test.go @@ -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" @@ -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) { @@ -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() @@ -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 diff --git a/agent/grpc-external/services/resource/mock_ACLResolver.go b/agent/grpc-external/services/resource/mock_ACLResolver.go new file mode 100644 index 000000000..5e5960850 --- /dev/null +++ b/agent/grpc-external/services/resource/mock_ACLResolver.go @@ -0,0 +1,54 @@ +// Code generated by mockery v2.20.0. DO NOT EDIT. + +package resource + +import ( + acl "github.com/hashicorp/consul/acl" + mock "github.com/stretchr/testify/mock" + + resolver "github.com/hashicorp/consul/acl/resolver" +) + +// MockACLResolver is an autogenerated mock type for the ACLResolver type +type MockACLResolver struct { + mock.Mock +} + +// ResolveTokenAndDefaultMeta provides a mock function with given fields: _a0, _a1, _a2 +func (_m *MockACLResolver) ResolveTokenAndDefaultMeta(_a0 string, _a1 *acl.EnterpriseMeta, _a2 *acl.AuthorizerContext) (resolver.Result, error) { + ret := _m.Called(_a0, _a1, _a2) + + var r0 resolver.Result + var r1 error + if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (resolver.Result, error)); ok { + return rf(_a0, _a1, _a2) + } + if rf, ok := ret.Get(0).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) resolver.Result); ok { + r0 = rf(_a0, _a1, _a2) + } else { + r0 = ret.Get(0).(resolver.Result) + } + + if rf, ok := ret.Get(1).(func(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) error); ok { + r1 = rf(_a0, _a1, _a2) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type mockConstructorTestingTNewMockACLResolver interface { + mock.TestingT + Cleanup(func()) +} + +// NewMockACLResolver creates a new instance of MockACLResolver. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewMockACLResolver(t mockConstructorTestingTNewMockACLResolver) *MockACLResolver { + mock := &MockACLResolver{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/agent/grpc-external/services/resource/mock_Registry.go b/agent/grpc-external/services/resource/mock_Registry.go new file mode 100644 index 000000000..288e8bcde --- /dev/null +++ b/agent/grpc-external/services/resource/mock_Registry.go @@ -0,0 +1,59 @@ +// Code generated by mockery v2.20.0. DO NOT EDIT. + +package resource + +import ( + internalresource "github.com/hashicorp/consul/internal/resource" + mock "github.com/stretchr/testify/mock" + + pbresource "github.com/hashicorp/consul/proto-public/pbresource" +) + +// MockRegistry is an autogenerated mock type for the Registry type +type MockRegistry struct { + mock.Mock +} + +// Register provides a mock function with given fields: reg +func (_m *MockRegistry) Register(reg internalresource.Registration) { + _m.Called(reg) +} + +// Resolve provides a mock function with given fields: typ +func (_m *MockRegistry) Resolve(typ *pbresource.Type) (internalresource.Registration, bool) { + ret := _m.Called(typ) + + var r0 internalresource.Registration + var r1 bool + if rf, ok := ret.Get(0).(func(*pbresource.Type) (internalresource.Registration, bool)); ok { + return rf(typ) + } + if rf, ok := ret.Get(0).(func(*pbresource.Type) internalresource.Registration); ok { + r0 = rf(typ) + } else { + r0 = ret.Get(0).(internalresource.Registration) + } + + if rf, ok := ret.Get(1).(func(*pbresource.Type) bool); ok { + r1 = rf(typ) + } else { + r1 = ret.Get(1).(bool) + } + + return r0, r1 +} + +type mockConstructorTestingTNewMockRegistry interface { + mock.TestingT + Cleanup(func()) +} + +// NewMockRegistry creates a new instance of MockRegistry. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewMockRegistry(t mockConstructorTestingTNewMockRegistry) *MockRegistry { + mock := &MockRegistry{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/agent/grpc-external/services/resource/read.go b/agent/grpc-external/services/resource/read.go index c3eaea86e..2e8988970 100644 --- a/agent/grpc-external/services/resource/read.go +++ b/agent/grpc-external/services/resource/read.go @@ -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) + } } diff --git a/agent/grpc-external/services/resource/read_test.go b/agent/grpc-external/services/resource/read_test.go index 4c33097a3..86ebd59a4 100644 --- a/agent/grpc-external/services/resource/read_test.go +++ b/agent/grpc-external/services/resource/read_test.go @@ -13,6 +13,7 @@ import ( "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/internal/storage" @@ -30,13 +31,14 @@ func TestRead_TypeNotFound(t *testing.T) { _, err = client.Read(context.Background(), &pbresource.ReadRequest{Id: artist.Id}) 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 TestRead_ResourceNotFound(t *testing.T) { for desc, tc := range readTestCases() { t.Run(desc, func(t *testing.T) { server := testServer(t) + demo.Register(server.Registry) client := testClient(t, server) @@ -55,6 +57,7 @@ func TestRead_GroupVersionMismatch(t *testing.T) { for desc, tc := range readTestCases() { t.Run(desc, func(t *testing.T) { server := testServer(t) + demo.Register(server.Registry) client := testClient(t, server) @@ -79,6 +82,7 @@ func TestRead_Success(t *testing.T) { for desc, tc := range readTestCases() { t.Run(desc, func(t *testing.T) { server := testServer(t) + demo.Register(server.Registry) client := testClient(t, server) @@ -99,11 +103,9 @@ func TestRead_VerifyReadConsistencyArg(t *testing.T) { // Uses a mockBackend instead of the inmem Backend to verify the ReadConsistency argument is set correctly. for desc, tc := range readTestCases() { t.Run(desc, func(t *testing.T) { + server := testServer(t) mockBackend := NewMockBackend(t) - server := NewServer(Config{ - Registry: resource.NewRegistry(), - Backend: mockBackend, - }) + server.Backend = mockBackend demo.Register(server.Registry) artist, err := demo.GenerateV2Artist() @@ -120,6 +122,45 @@ func TestRead_VerifyReadConsistencyArg(t *testing.T) { } } +// N.B. Uses key ACLs for now. See demo.Register() +func TestRead_ACLs(t *testing.T) { + type testCase struct { + authz resolver.Result + code codes.Code + } + testcases := map[string]testCase{ + "read hook denied": { + authz: AuthorizerFrom(t, demo.ArtistV1ReadPolicy), + code: codes.PermissionDenied, + }, + "read hook allowed": { + authz: AuthorizerFrom(t, demo.ArtistV2ReadPolicy), + code: codes.NotFound, + }, + } + + for desc, tc := range testcases { + t.Run(desc, func(t *testing.T) { + server := testServer(t) + client := testClient(t, server) + + mockACLResolver := &MockACLResolver{} + mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). + Return(tc.authz, nil) + server.ACLResolver = mockACLResolver + demo.Register(server.Registry) + + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + + // exercise ACL + _, err = client.Read(testContext(t), &pbresource.ReadRequest{Id: artist.Id}) + require.Error(t, err) + require.Equal(t, tc.code.String(), status.Code(err).String()) + }) + } +} + type readTestCase struct { consistency storage.ReadConsistency ctx context.Context @@ -139,5 +180,4 @@ func readTestCases() map[string]readTestCase { ), }, } - } diff --git a/agent/grpc-external/services/resource/server.go b/agent/grpc-external/services/resource/server.go index 34852429d..d82e7c81b 100644 --- a/agent/grpc-external/services/resource/server.go +++ b/agent/grpc-external/services/resource/server.go @@ -13,6 +13,8 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/storage" "github.com/hashicorp/consul/proto-public/pbresource" @@ -27,7 +29,8 @@ type Config struct { Registry Registry // Backend is the storage backend that will be used for resource persistence. - Backend Backend + Backend Backend + ACLResolver ACLResolver } //go:generate mockery --name Registry --inpackage @@ -40,6 +43,11 @@ type Backend interface { storage.Backend } +//go:generate mockery --name ACLResolver --inpackage +type ACLResolver interface { + ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (resolver.Result, error) +} + func NewServer(cfg Config) *Server { return &Server{cfg} } @@ -55,7 +63,20 @@ func (s *Server) WriteStatus(ctx context.Context, req *pbresource.WriteStatusReq return &pbresource.WriteStatusResponse{}, nil } -//nolint:unparam +// Get token from grpc metadata or AnonymounsTokenId if not found +func tokenFromContext(ctx context.Context) string { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return acl.AnonymousTokenID + } + + vals := md.Get("x-consul-token") + if len(vals) == 0 { + return acl.AnonymousTokenID + } + return vals[0] +} + func (s *Server) resolveType(typ *pbresource.Type) (*resource.Registration, error) { v, ok := s.Registry.Resolve(typ) if ok { @@ -84,4 +105,12 @@ func readConsistencyFrom(ctx context.Context) storage.ReadConsistency { return storage.EventualConsistency } +func (s *Server) getAuthorizer(token string) (acl.Authorizer, error) { + authz, err := s.ACLResolver.ResolveTokenAndDefaultMeta(token, nil, nil) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed getting authorizer: %v", err) + } + return authz, nil +} + func clone[T proto.Message](v T) T { return proto.Clone(v).(T) } diff --git a/agent/grpc-external/services/resource/server_test.go b/agent/grpc-external/services/resource/server_test.go index 05d1e2d68..eef225b83 100644 --- a/agent/grpc-external/services/resource/server_test.go +++ b/agent/grpc-external/services/resource/server_test.go @@ -8,16 +8,21 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/protobuf/types/known/anypb" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" "github.com/hashicorp/consul/agent/grpc-external/testutils" + "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/storage/inmem" "github.com/hashicorp/consul/proto-public/pbresource" pbdemov2 "github.com/hashicorp/consul/proto/private/pbdemo/v2" "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/go-uuid" ) func TestWriteStatus_TODO(t *testing.T) { @@ -28,6 +33,30 @@ func TestWriteStatus_TODO(t *testing.T) { require.NotNil(t, resp) } +func randomACLIdentity(t *testing.T) structs.ACLIdentity { + id, err := uuid.GenerateUUID() + require.NoError(t, err) + + return &structs.ACLToken{AccessorID: id} +} + +func AuthorizerFrom(t *testing.T, policyStrs ...string) resolver.Result { + policies := []*acl.Policy{} + for _, policyStr := range policyStrs { + policy, err := acl.NewPolicyFromSource(policyStr, nil, nil) + require.NoError(t, err) + policies = append(policies, policy) + } + + authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), policies, nil) + require.NoError(t, err) + + return resolver.Result{ + Authorizer: authz, + ACLIdentity: randomACLIdentity(t), + } +} + func testServer(t *testing.T) *Server { t.Helper() @@ -35,10 +64,16 @@ func testServer(t *testing.T) *Server { require.NoError(t, err) go backend.Run(testContext(t)) + // Mock the ACL Resolver to allow everything for testing + mockACLResolver := &MockACLResolver{} + mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). + Return(testutils.ACLsDisabled(t), nil) + return NewServer(Config{ - Logger: testutil.Logger(t), - Registry: resource.NewRegistry(), - Backend: backend, + Logger: testutil.Logger(t), + Registry: resource.NewRegistry(), + Backend: backend, + ACLResolver: mockACLResolver, }) } diff --git a/agent/grpc-external/services/resource/watch.go b/agent/grpc-external/services/resource/watch.go index 8c76dc970..77ffe19b0 100644 --- a/agent/grpc-external/services/resource/watch.go +++ b/agent/grpc-external/services/resource/watch.go @@ -4,16 +4,35 @@ package resource 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) WatchList(req *pbresource.WatchListRequest, stream pbresource.ResourceService_WatchListServer) error { // check type exists - if _, err := s.resolveType(req.Type); err != nil { + reg, err := s.resolveType(req.Type) + if err != nil { return err } + authz, err := s.getAuthorizer(tokenFromContext(stream.Context())) + if err != nil { + return err + } + + // check acls + err = reg.ACLs.List(authz, req.Tenancy) + switch { + case acl.IsErrPermissionDenied(err): + return status.Error(codes.PermissionDenied, err.Error()) + case err != nil: + return status.Errorf(codes.Internal, "failed list acl: %v", err) + } + unversionedType := storage.UnversionedTypeFrom(req.Type) watch, err := s.Backend.WatchList( stream.Context(), @@ -29,14 +48,23 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R for { event, err := watch.Next(stream.Context()) if err != nil { - return err + return status.Errorf(codes.Internal, "failed next: %v", err) } - // drop versions that don't match + // drop group versions that don't match if event.Resource.Id.Type.GroupVersion != req.Type.GroupVersion { continue } + // filter out items that don't pass read ACLs + err = reg.ACLs.Read(authz, event.Resource.Id) + switch { + case acl.IsErrPermissionDenied(err): + continue + case err != nil: + return status.Errorf(codes.Internal, "failed read acl: %v", err) + } + if err = stream.Send(event); err != nil { return err } diff --git a/agent/grpc-external/services/resource/watch_test.go b/agent/grpc-external/services/resource/watch_test.go index a355d5a5c..af6f68e58 100644 --- a/agent/grpc-external/services/resource/watch_test.go +++ b/agent/grpc-external/services/resource/watch_test.go @@ -10,10 +10,13 @@ import ( "testing" "time" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/grpc-external/testutils" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/proto/private/prototest" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -21,6 +24,7 @@ import ( func TestWatchList_TypeNotFound(t *testing.T) { t.Parallel() + server := testServer(t) client := testClient(t, server) @@ -34,11 +38,12 @@ func TestWatchList_TypeNotFound(t *testing.T) { err = mustGetError(t, rspCh) 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 TestWatchList_GroupVersionMatches(t *testing.T) { t.Parallel() + server := testServer(t) client := testClient(t, server) demo.Register(server.Registry) @@ -83,6 +88,7 @@ func TestWatchList_GroupVersionMismatch(t *testing.T) { // When a resource of TypeArtistV2 is created/updated/deleted // Then no watch events should be emitted t.Parallel() + server := testServer(t) demo.Register(server.Registry) client := testClient(t, server) @@ -117,6 +123,81 @@ func TestWatchList_GroupVersionMismatch(t *testing.T) { mustGetNoResource(t, rspCh) } +// N.B. Uses key ACLs for now. See demo.Register() +func TestWatchList_ACL_ListDenied(t *testing.T) { + t.Parallel() + + // deny all + rspCh, _ := roundTripACL(t, testutils.ACLNoPermissions(t)) + + // verify key:list denied + err := mustGetError(t, rspCh) + 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 TestWatchList_ACL_ListAllowed_ReadDenied(t *testing.T) { + t.Parallel() + + // allow list, deny read + authz := AuthorizerFrom(t, ` + key_prefix "resource/" { policy = "list" } + key_prefix "resource/demo.v2.artist/" { policy = "deny" } + `) + rspCh, _ := roundTripACL(t, authz) + + // verify resource filtered out by key:read denied, hence no events + mustGetNoResource(t, rspCh) +} + +// N.B. Uses key ACLs for now. See demo.Register() +func TestWatchList_ACL_ListAllowed_ReadAllowed(t *testing.T) { + t.Parallel() + + // allow list, allow read + authz := AuthorizerFrom(t, ` + key_prefix "resource/" { policy = "list" } + key_prefix "resource/demo.v2.artist/" { policy = "read" } + `) + rspCh, artist := roundTripACL(t, authz) + + // verify resource not filtered out by acl + event := mustGetResource(t, rspCh) + prototest.AssertDeepEqual(t, artist, event.Resource) +} + +// roundtrip a WatchList which attempts to stream back a single write event +func roundTripACL(t *testing.T, authz acl.Authorizer) (<-chan resourceOrError, *pbresource.Resource) { + server := testServer(t) + client := testClient(t, server) + + 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) + + stream, err := client.WatchList(testContext(t), &pbresource.WatchListRequest{ + Type: artist.Id.Type, + Tenancy: artist.Id.Tenancy, + NamePrefix: "", + }) + require.NoError(t, err) + rspCh := handleResourceStream(t, stream) + + // induce single watch event + artist, err = server.Backend.WriteCAS(context.Background(), artist) + require.NoError(t, err) + + // caller to make assertions on the rspCh and written artist + return rspCh, artist +} + func mustGetNoResource(t *testing.T, ch <-chan resourceOrError) { t.Helper() diff --git a/agent/grpc-external/services/resource/write_test.go b/agent/grpc-external/services/resource/write_test.go index 7b0b324cb..35f187179 100644 --- a/agent/grpc-external/services/resource/write_test.go +++ b/agent/grpc-external/services/resource/write_test.go @@ -60,7 +60,7 @@ func TestWrite_TypeNotFound(t *testing.T) { _, err = client.Write(testContext(t), &pbresource.WriteRequest{Resource: res}) 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 TestWrite_ResourceCreation(t *testing.T) { diff --git a/agent/grpc-external/testutils/acl.go b/agent/grpc-external/testutils/acl.go index ec2d149a3..440a83768 100644 --- a/agent/grpc-external/testutils/acl.go +++ b/agent/grpc-external/testutils/acl.go @@ -84,6 +84,40 @@ func ACLServiceRead(t *testing.T, serviceName string) resolver.Result { } } +func ACLOperatorRead(t *testing.T) resolver.Result { + t.Helper() + + aclRule := &acl.Policy{ + PolicyRules: acl.PolicyRules{ + Operator: acl.PolicyRead, + }, + } + authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{aclRule}, nil) + require.NoError(t, err) + + return resolver.Result{ + Authorizer: authz, + ACLIdentity: randomACLIdentity(t), + } +} + +func ACLOperatorWrite(t *testing.T) resolver.Result { + t.Helper() + + aclRule := &acl.Policy{ + PolicyRules: acl.PolicyRules{ + Operator: acl.PolicyWrite, + }, + } + authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{aclRule}, nil) + require.NoError(t, err) + + return resolver.Result{ + Authorizer: authz, + ACLIdentity: randomACLIdentity(t), + } +} + func randomACLIdentity(t *testing.T) structs.ACLIdentity { id, err := uuid.GenerateUUID() require.NoError(t, err) diff --git a/internal/resource/demo/demo.go b/internal/resource/demo/demo.go index 7d918f18a..99600737f 100644 --- a/internal/resource/demo/demo.go +++ b/internal/resource/demo/demo.go @@ -10,6 +10,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/proto-public/pbresource" pbdemov1 "github.com/hashicorp/consul/proto/private/pbdemo/v1" @@ -53,26 +54,75 @@ var ( } ) +const ( + ArtistV1ReadPolicy = `key_prefix "resource/demo.v1.artist/" { policy = "read" }` + ArtistV1WritePolicy = `key_prefix "resource/demo.v1.artist/" { policy = "write" }` + ArtistV2ReadPolicy = `key_prefix "resource/demo.v2.artist/" { policy = "read" }` + ArtistV2WritePolicy = `key_prefix "resource/demo.v2.artist/" { policy = "write" }` + ArtistV2ListPolicy = `key_prefix "resource/" { policy = "list" }` +) + // Register demo types. Should only be called in tests and dev mode. +// acls are optional. +// +// TODO(spatel): We're standing-in key ACLs for demo resources until our ACL +// system can be more modularly extended (or support generic resource permissions). func Register(r resource.Registry) { + readACL := func(authz acl.Authorizer, id *pbresource.ID) error { + key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(id.Type), id.Name) + return authz.ToAllowAuthorizer().KeyReadAllowed(key, &acl.AuthorizerContext{}) + } + + writeACL := func(authz acl.Authorizer, res *pbresource.Resource) error { + key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(res.Id.Type), res.Id.Name) + return authz.ToAllowAuthorizer().KeyWriteAllowed(key, &acl.AuthorizerContext{}) + } + + makeListACL := func(typ *pbresource.Type) func(acl.Authorizer, *pbresource.Tenancy) error { + return func(authz acl.Authorizer, tenancy *pbresource.Tenancy) error { + key := fmt.Sprintf("resource/%s", resource.ToGVK(typ)) + return authz.ToAllowAuthorizer().KeyListAllowed(key, &acl.AuthorizerContext{}) + } + } + r.Register(resource.Registration{ Type: TypeV1Artist, Proto: &pbdemov1.Artist{}, + ACLs: &resource.ACLHooks{ + Read: readACL, + Write: writeACL, + List: makeListACL(TypeV1Artist), + }, }) r.Register(resource.Registration{ Type: TypeV1Album, Proto: &pbdemov1.Album{}, + ACLs: &resource.ACLHooks{ + Read: readACL, + Write: writeACL, + List: makeListACL(TypeV1Album), + }, }) r.Register(resource.Registration{ Type: TypeV2Artist, Proto: &pbdemov2.Artist{}, + ACLs: &resource.ACLHooks{ + Read: readACL, + Write: writeACL, + List: makeListACL(TypeV2Artist), + }, }) r.Register(resource.Registration{ Type: TypeV2Album, Proto: &pbdemov2.Album{}, + ACLs: &resource.ACLHooks{ + Read: readACL, + Write: writeACL, + List: makeListACL(TypeV2Album), + }, }) } diff --git a/internal/resource/registry.go b/internal/resource/registry.go index 46d2d2c79..b206ea86f 100644 --- a/internal/resource/registry.go +++ b/internal/resource/registry.go @@ -9,6 +9,7 @@ import ( "google.golang.org/protobuf/proto" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/proto-public/pbresource" ) @@ -27,12 +28,30 @@ type Registration struct { // Proto is the resource's protobuf message type. Proto proto.Message + // ACLs are hooks called to perform authorization on RPCs. + ACLs *ACLHooks + // In the future, we'll add hooks, the controller etc. here. // TODO: https://github.com/hashicorp/consul/pull/16622#discussion_r1134515909 } -// Hashable key for a resource type -type TypeKey string +type ACLHooks struct { + // Read is used to authorize Read RPCs and to filter results in List + // RPCs. + // + // If it is omitted, `operator:read` permission is assumed. + Read func(acl.Authorizer, *pbresource.ID) error + + // Write is used to authorize Write and Delete RPCs. + // + // If it is omitted, `operator:write` permission is assumed. + Write func(acl.Authorizer, *pbresource.Resource) error + + // List is used to authorize List RPCs. + // + // If it is omitted, we only filter the results using Read. + List func(acl.Authorizer, *pbresource.Tenancy) error +} // Resource type registry type TypeRegistry struct { @@ -61,6 +80,25 @@ func (r *TypeRegistry) Register(registration Registration) { panic(fmt.Sprintf("resource type %s already registered", key)) } + // set default acl hooks for those not provided + if registration.ACLs == nil { + registration.ACLs = &ACLHooks{} + } + if registration.ACLs.Read == nil { + registration.ACLs.Read = func(authz acl.Authorizer, id *pbresource.ID) error { + return authz.ToAllowAuthorizer().OperatorReadAllowed(&acl.AuthorizerContext{}) + } + } + if registration.ACLs.Write == nil { + registration.ACLs.Write = func(authz acl.Authorizer, resource *pbresource.Resource) error { + return authz.ToAllowAuthorizer().OperatorWriteAllowed(&acl.AuthorizerContext{}) + } + } + if registration.ACLs.List == nil { + registration.ACLs.List = func(authz acl.Authorizer, tenancy *pbresource.Tenancy) error { + return authz.ToAllowAuthorizer().OperatorReadAllowed(&acl.AuthorizerContext{}) + } + } r.registrations[key] = registration } @@ -75,5 +113,5 @@ func (r *TypeRegistry) Resolve(typ *pbresource.Type) (reg Registration, ok bool) } func ToGVK(resourceType *pbresource.Type) string { - return fmt.Sprintf("%s/%s/%s", resourceType.Group, resourceType.GroupVersion, resourceType.Kind) + return fmt.Sprintf("%s.%s.%s", resourceType.Group, resourceType.GroupVersion, resourceType.Kind) } diff --git a/internal/resource/registry_test.go b/internal/resource/registry_test.go index f510c58f4..3fe8d9537 100644 --- a/internal/resource/registry_test.go +++ b/internal/resource/registry_test.go @@ -1,17 +1,22 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -package resource +package resource_test import ( "testing" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/grpc-external/testutils" + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRegister(t *testing.T) { - r := NewRegistry() + r := resource.NewRegistry() serviceType := &pbresource.Type{ Group: "mesh", @@ -19,15 +24,15 @@ func TestRegister(t *testing.T) { Kind: "service", } - // register - serviceRegistration := Registration{Type: serviceType} + // register success + serviceRegistration := resource.Registration{Type: serviceType} r.Register(serviceRegistration) // register existing should panic - assertRegisterPanics(t, r.Register, serviceRegistration, "resource type mesh/v1/service already registered") + assertRegisterPanics(t, r.Register, serviceRegistration, "resource type mesh.v1.service already registered") // register empty Group should panic - assertRegisterPanics(t, r.Register, Registration{ + assertRegisterPanics(t, r.Register, resource.Registration{ Type: &pbresource.Type{ Group: "", GroupVersion: "v1", @@ -36,7 +41,7 @@ func TestRegister(t *testing.T) { }, "type field(s) cannot be empty") // register empty GroupVersion should panic - assertRegisterPanics(t, r.Register, Registration{ + assertRegisterPanics(t, r.Register, resource.Registration{ Type: &pbresource.Type{ Group: "mesh", GroupVersion: "", @@ -45,7 +50,7 @@ func TestRegister(t *testing.T) { }, "type field(s) cannot be empty") // register empty Kind should panic - assertRegisterPanics(t, r.Register, Registration{ + assertRegisterPanics(t, r.Register, resource.Registration{ Type: &pbresource.Type{ Group: "mesh", GroupVersion: "v1", @@ -54,7 +59,32 @@ func TestRegister(t *testing.T) { }, "type field(s) cannot be empty") } -func assertRegisterPanics(t *testing.T, registerFn func(reg Registration), registration Registration, panicString string) { +func TestRegister_DefaultACLs(t *testing.T) { + r := resource.NewRegistry() + r.Register(resource.Registration{ + Type: demo.TypeV2Artist, + // intentionally don't provide ACLs so defaults kick in + }) + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + + reg, ok := r.Resolve(demo.TypeV2Artist) + require.True(t, ok) + + // verify default read hook requires operator:read + require.NoError(t, reg.ACLs.Read(testutils.ACLOperatorRead(t), artist.Id)) + require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Read(testutils.ACLNoPermissions(t), artist.Id))) + + // verify default write hook requires operator:write + require.NoError(t, reg.ACLs.Write(testutils.ACLOperatorWrite(t), artist)) + require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Write(testutils.ACLNoPermissions(t), artist))) + + // verify default list hook requires operator:read + require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), artist.Id.Tenancy)) + require.True(t, acl.IsErrPermissionDenied(reg.ACLs.List(testutils.ACLNoPermissions(t), artist.Id.Tenancy))) +} + +func assertRegisterPanics(t *testing.T, registerFn func(reg resource.Registration), registration resource.Registration, panicString string) { defer func() { if r := recover(); r == nil { t.Errorf("expected panic, but none occurred") @@ -72,7 +102,7 @@ func assertRegisterPanics(t *testing.T, registerFn func(reg Registration), regis } func TestResolve(t *testing.T) { - r := NewRegistry() + r := resource.NewRegistry() serviceType := &pbresource.Type{ Group: "mesh", @@ -85,7 +115,7 @@ func TestResolve(t *testing.T) { assert.False(t, ok) // found - r.Register(Registration{Type: serviceType}) + r.Register(resource.Registration{Type: serviceType}) registration, ok := r.Resolve(serviceType) assert.True(t, ok) assert.Equal(t, registration.Type, serviceType)