diff --git a/cmd/bb_remote_asset/main.go b/cmd/bb_remote_asset/main.go index 93eeaf0..e27b92d 100644 --- a/cmd/bb_remote_asset/main.go +++ b/cmd/bb_remote_asset/main.go @@ -70,6 +70,7 @@ func main() { if err != nil { return util.StatusWrap(err, "Failed to create CAS blob access") } + var assetStore storage.AssetStore if config.AssetCache != nil { assetStore, err = configuration.NewAssetStoreFromConfiguration( @@ -78,8 +79,6 @@ func main() { grpcClientFactory, int(config.MaximumMessageSizeBytes), dependenciesGroup, - fetchAuthorizer, - pushAuthorizer, ) if err != nil { return util.StatusWrap(err, "Failed to create asset store") @@ -113,12 +112,13 @@ func main() { pushServer := push.NewAssetPushServer( assetStore, allowUpdatesForInstances) + pushServer = push.NewAuthorizingPushServer(pushServer, pushAuthorizer) metricsPushServer = push.NewMetricsAssetPushServer(pushServer, clock.SystemClock, "push") } else { - metricsPushServer = push.NewErrorPushServer(&protostatus.Status{ + metricsPushServer = push.NewAuthorizingPushServer(push.NewErrorPushServer(&protostatus.Status{ Code: int32(codes.FailedPrecondition), Message: "Server is not configured to allow pushing assets", - }) + }), pushAuthorizer) } // Spawn gRPC servers for client and worker traffic. diff --git a/internal/mock/BUILD.bazel b/internal/mock/BUILD.bazel index 767d135..53070cd 100644 --- a/internal/mock/BUILD.bazel +++ b/internal/mock/BUILD.bazel @@ -40,6 +40,16 @@ gomock( package = "mock", ) +gomock( + name = "push", + out = "push.go", + interfaces = [ + "PushServer", + ], + library = "@bazel_remote_apis//build/bazel/remote/asset/v1:remote_asset_go_proto", + package = "mock", +) + gomock( name = "storage", out = "storage.go", @@ -56,6 +66,7 @@ go_library( "blobstore.go", "dummy.go", "fetcher.go", + "push.go", "storage.go", ], importpath = "github.com/buildbarn/bb-remote-asset/internal/mock", diff --git a/pkg/configuration/new_asset_store.go b/pkg/configuration/new_asset_store.go index 72e2c8d..a2132e8 100644 --- a/pkg/configuration/new_asset_store.go +++ b/pkg/configuration/new_asset_store.go @@ -4,7 +4,6 @@ import ( pb "github.com/buildbarn/bb-remote-asset/pkg/proto/configuration/bb_remote_asset" "github.com/buildbarn/bb-remote-asset/pkg/storage" asset_configuration "github.com/buildbarn/bb-remote-asset/pkg/storage/blobstore" - "github.com/buildbarn/bb-storage/pkg/auth" blobstore_configuration "github.com/buildbarn/bb-storage/pkg/blobstore/configuration" "github.com/buildbarn/bb-storage/pkg/grpc" "github.com/buildbarn/bb-storage/pkg/program" @@ -21,8 +20,6 @@ func NewAssetStoreFromConfiguration( grpcClientFactory grpc.ClientFactory, maximumMessageSizeBytes int, dependenciesGroup program.Group, - fetchAuthorizer auth.Authorizer, - pushAuthorizer auth.Authorizer, ) (storage.AssetStore, error) { var assetStore storage.AssetStore switch backend := configuration.Backend.(type) { @@ -56,5 +53,5 @@ func NewAssetStoreFromConfiguration( default: return nil, status.Errorf(codes.InvalidArgument, "Asset Cache configuration is invalid as no supported Asset Cache is defined.") } - return storage.NewAuthorizingAssetStore(assetStore, fetchAuthorizer, pushAuthorizer), nil + return assetStore, nil } diff --git a/pkg/push/BUILD.bazel b/pkg/push/BUILD.bazel index 3b0732b..dcc3130 100644 --- a/pkg/push/BUILD.bazel +++ b/pkg/push/BUILD.bazel @@ -3,6 +3,7 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "push", srcs = [ + "authorizing_push_server.go", "error_push_server.go", "metrics_push_server.go", "push_server.go", @@ -13,6 +14,7 @@ go_library( "//pkg/storage", "@bazel_remote_apis//build/bazel/remote/asset/v1:remote_asset_go_proto", "@bazel_remote_apis//build/bazel/remote/execution/v2:remote_execution_go_proto", + "@com_github_buildbarn_bb_storage//pkg/auth", "@com_github_buildbarn_bb_storage//pkg/clock", "@com_github_buildbarn_bb_storage//pkg/digest", "@com_github_buildbarn_bb_storage//pkg/util", @@ -25,7 +27,10 @@ go_library( go_test( name = "push_test", - srcs = ["push_server_test.go"], + srcs = [ + "authorizing_push_server_test.go", + "push_server_test.go", + ], deps = [ ":push", "//internal/mock", @@ -35,6 +40,7 @@ go_test( "@bazel_remote_apis//build/bazel/remote/execution/v2:remote_execution_go_proto", "@com_github_buildbarn_bb_storage//pkg/blobstore/buffer", "@com_github_buildbarn_bb_storage//pkg/digest", + "@com_github_buildbarn_bb_storage//pkg/util", "@com_github_golang_mock//gomock", "@com_github_stretchr_testify//require", "@org_golang_google_grpc//codes", diff --git a/pkg/push/authorizing_push_server.go b/pkg/push/authorizing_push_server.go new file mode 100644 index 0000000..5044e06 --- /dev/null +++ b/pkg/push/authorizing_push_server.go @@ -0,0 +1,47 @@ +package push + +import ( + "context" + + remoteasset "github.com/bazelbuild/remote-apis/build/bazel/remote/asset/v1" + "github.com/buildbarn/bb-storage/pkg/auth" + "github.com/buildbarn/bb-storage/pkg/digest" +) + +// AuthorizingPushServer decorates a PushServer and validates the requests against an Authorizer +type AuthorizingPushServer struct { + remoteasset.PushServer + authorizer auth.Authorizer +} + +// NewAuthorizingPushServer wraps a PushServer into an AuthorizingPushServer +func NewAuthorizingPushServer(p remoteasset.PushServer, authorizer auth.Authorizer) *AuthorizingPushServer { + return &AuthorizingPushServer{ + p, + authorizer, + } +} + +// PushBlob authorizes a PushBlob request and, if successful, passes it along to the wrapped PushServer +func (ap *AuthorizingPushServer) PushBlob(ctx context.Context, req *remoteasset.PushBlobRequest) (*remoteasset.PushBlobResponse, error) { + instanceName, err := digest.NewInstanceName(req.InstanceName) + if err != nil { + return nil, err + } + if err = auth.AuthorizeSingleInstanceName(ctx, ap.authorizer, instanceName); err != nil { + return nil, err + } + return ap.PushServer.PushBlob(ctx, req) +} + +// PushDirectory authorizes a PushDirectory request and, if successful, passes it along to the wrapped PushServer +func (ap *AuthorizingPushServer) PushDirectory(ctx context.Context, req *remoteasset.PushDirectoryRequest) (*remoteasset.PushDirectoryResponse, error) { + instanceName, err := digest.NewInstanceName(req.InstanceName) + if err != nil { + return nil, err + } + if err = auth.AuthorizeSingleInstanceName(ctx, ap.authorizer, instanceName); err != nil { + return nil, err + } + return ap.PushServer.PushDirectory(ctx, req) +} diff --git a/pkg/push/authorizing_push_server_test.go b/pkg/push/authorizing_push_server_test.go new file mode 100644 index 0000000..5d04fd4 --- /dev/null +++ b/pkg/push/authorizing_push_server_test.go @@ -0,0 +1,91 @@ +package push_test + +import ( + "context" + "testing" + + remoteasset "github.com/bazelbuild/remote-apis/build/bazel/remote/asset/v1" + remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" + "github.com/buildbarn/bb-remote-asset/internal/mock" + "github.com/buildbarn/bb-remote-asset/pkg/push" + bb_digest "github.com/buildbarn/bb-storage/pkg/digest" + "github.com/buildbarn/bb-storage/pkg/util" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestPushBlobAuthorization(t *testing.T) { + ctrl, ctx := gomock.WithContext(context.Background(), t) + + basePushServer := mock.NewMockPushServer(ctrl) + authorizer := mock.NewMockAuthorizer(ctrl) + ap := push.NewAuthorizingPushServer(basePushServer, authorizer) + + instanceName := util.Must(bb_digest.NewInstanceName("ithilien")) + instanceSlice := []bb_digest.InstanceName{instanceName} + + uri := "source.test" + blobDigest := &remoteexecution.Digest{Hash: "d0d829c4c0ce64787cb1c998a9c29a109f8ed005633132fda4f29982487b04db", SizeBytes: 123} + request := &remoteasset.PushBlobRequest{ + InstanceName: "ithilien", + Uris: []string{uri}, + BlobDigest: blobDigest, + } + + wantResp := &remoteasset.PushBlobResponse{} + + t.Run("Allowed", func(t *testing.T) { + authorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{nil}) + basePushServer.EXPECT().PushBlob(ctx, request).Return(wantResp, nil) + + gotResp, err := ap.PushBlob(ctx, request) + require.NoError(t, err) + require.Equal(t, wantResp, gotResp) + }) + + t.Run("Rejected", func(t *testing.T) { + authorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{status.Error(codes.PermissionDenied, "You shall not pass")}) + + _, err := ap.PushBlob(ctx, request) + require.Equal(t, err, status.Error(codes.PermissionDenied, "You shall not pass")) + }) +} + +func TestPushDirectoryAuthorization(t *testing.T) { + ctrl, ctx := gomock.WithContext(context.Background(), t) + + basePushServer := mock.NewMockPushServer(ctrl) + authorizer := mock.NewMockAuthorizer(ctrl) + ap := push.NewAuthorizingPushServer(basePushServer, authorizer) + + instanceName := util.Must(bb_digest.NewInstanceName("ithilien")) + instanceSlice := []bb_digest.InstanceName{instanceName} + + uri := "source.test" + directoryDigest := &remoteexecution.Digest{Hash: "d0d829c4c0ce64787cb1c998a9c29a109f8ed005633132fda4f29982487b04db", SizeBytes: 123} + request := &remoteasset.PushDirectoryRequest{ + InstanceName: "ithilien", + Uris: []string{uri}, + RootDirectoryDigest: directoryDigest, + } + + wantResp := &remoteasset.PushDirectoryResponse{} + + t.Run("Allowed", func(t *testing.T) { + authorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{nil}) + basePushServer.EXPECT().PushDirectory(ctx, request).Return(wantResp, nil) + + gotResp, err := ap.PushDirectory(ctx, request) + require.NoError(t, err) + require.Equal(t, wantResp, gotResp) + }) + + t.Run("Rejected", func(t *testing.T) { + authorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{status.Error(codes.PermissionDenied, "You shall not pass")}) + + _, err := ap.PushDirectory(ctx, request) + require.Equal(t, err, status.Error(codes.PermissionDenied, "You shall not pass")) + }) +} diff --git a/pkg/storage/BUILD.bazel b/pkg/storage/BUILD.bazel index d599410..9fd1859 100644 --- a/pkg/storage/BUILD.bazel +++ b/pkg/storage/BUILD.bazel @@ -7,7 +7,6 @@ go_library( "asset.go", "asset_reference.go", "asset_store.go", - "authorizing_asset_store.go", "blob_access_asset_store.go", "digest.go", ], @@ -18,7 +17,6 @@ go_library( "//pkg/qualifier", "@bazel_remote_apis//build/bazel/remote/asset/v1:remote_asset_go_proto", "@bazel_remote_apis//build/bazel/remote/execution/v2:remote_execution_go_proto", - "@com_github_buildbarn_bb_storage//pkg/auth", "@com_github_buildbarn_bb_storage//pkg/blobstore", "@com_github_buildbarn_bb_storage//pkg/blobstore/buffer", "@com_github_buildbarn_bb_storage//pkg/digest", @@ -35,7 +33,6 @@ go_test( srcs = [ "action_cache_asset_store_test.go", "asset_reference_test.go", - "authorizing_asset_store_test.go", "blob_access_asset_store_test.go", ], deps = [ diff --git a/pkg/storage/authorizing_asset_store.go b/pkg/storage/authorizing_asset_store.go deleted file mode 100644 index a133e4b..0000000 --- a/pkg/storage/authorizing_asset_store.go +++ /dev/null @@ -1,41 +0,0 @@ -package storage - -import ( - "context" - - "github.com/buildbarn/bb-remote-asset/pkg/proto/asset" - "github.com/buildbarn/bb-storage/pkg/auth" - "github.com/buildbarn/bb-storage/pkg/digest" -) - -// AuthorizingAssetStore wraps an asset store and validates requests against the authorizers -type AuthorizingAssetStore struct { - AssetStore - fetchAuthorizer auth.Authorizer - pushAuthorizer auth.Authorizer -} - -// NewAuthorizingAssetStore creates a new authorizing asset store -func NewAuthorizingAssetStore(as AssetStore, fetchAuthorizer, pushAuthorizer auth.Authorizer) *AuthorizingAssetStore { - return &AuthorizingAssetStore{ - as, - fetchAuthorizer, - pushAuthorizer, - } -} - -// Get is a wrapper that validates credentials against FetchAuthorizer -func (aas *AuthorizingAssetStore) Get(ctx context.Context, ref *asset.AssetReference, digestFunction digest.Function) (*asset.Asset, error) { - if err := auth.AuthorizeSingleInstanceName(ctx, aas.fetchAuthorizer, digestFunction.GetInstanceName()); err != nil { - return nil, err - } - return aas.AssetStore.Get(ctx, ref, digestFunction) -} - -// Put is a wrapper that validates credentials against PushAuthorizer -func (aas *AuthorizingAssetStore) Put(ctx context.Context, ref *asset.AssetReference, data *asset.Asset, digestFunction digest.Function) error { - if err := auth.AuthorizeSingleInstanceName(ctx, aas.pushAuthorizer, digestFunction.GetInstanceName()); err != nil { - return err - } - return aas.AssetStore.Put(ctx, ref, data, digestFunction) -} diff --git a/pkg/storage/authorizing_asset_store_test.go b/pkg/storage/authorizing_asset_store_test.go deleted file mode 100644 index 8b29995..0000000 --- a/pkg/storage/authorizing_asset_store_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package storage_test - -import ( - "context" - "testing" - - remoteasset "github.com/bazelbuild/remote-apis/build/bazel/remote/asset/v1" - remoteexecution "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" - "github.com/buildbarn/bb-remote-asset/internal/mock" - "github.com/buildbarn/bb-remote-asset/pkg/storage" - bb_digest "github.com/buildbarn/bb-storage/pkg/digest" - "github.com/buildbarn/bb-storage/pkg/util" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "google.golang.org/protobuf/types/known/timestamppb" -) - -func TestAuthorizingBlobAccessGet(t *testing.T) { - ctrl, ctx := gomock.WithContext(context.Background(), t) - - instanceName := util.Must(bb_digest.NewInstanceName("rohan")) - instanceSlice := []bb_digest.InstanceName{instanceName} - digestFunction, err := instanceName.GetDigestFunction(remoteexecution.DigestFunction_SHA256, 0) - require.NoError(t, err) - - blobDigest := &remoteexecution.Digest{Hash: "b27cad931e1ef0a520887464127055ffd6db82c7b36bfea5cd832db65b8f816b", SizeBytes: 24} - uri := "https://raapi.test/blob" - assetRef := storage.NewAssetReference([]string{uri}, []*remoteasset.Qualifier{}) - assetData := storage.NewBlobAsset(blobDigest, timestamppb.Now()) - - baseStore := mock.NewMockAssetStore(ctrl) - fetchAuthorizer := mock.NewMockAuthorizer(ctrl) - pushAuthorizer := mock.NewMockAuthorizer(ctrl) - aas := storage.NewAuthorizingAssetStore(baseStore, fetchAuthorizer, pushAuthorizer) - - t.Run("Allowed", func(t *testing.T) { - fetchAuthorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{nil}) - baseStore.EXPECT().Get(ctx, assetRef, digestFunction).Return(assetData, nil) - - gotAsset, err := aas.Get(ctx, assetRef, digestFunction) - require.NoError(t, err) - require.Equal(t, assetData, gotAsset) - }) - - t.Run("Rejected", func(t *testing.T) { - fetchAuthorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{status.Error(codes.PermissionDenied, "None shall pass")}) - - _, err := aas.Get(ctx, assetRef, digestFunction) - require.Equal(t, status.Error(codes.PermissionDenied, "None shall pass"), err) - }) -} - -func TestAuthorizingBlobAccessPut(t *testing.T) { - ctrl, ctx := gomock.WithContext(context.Background(), t) - - instanceName := util.Must(bb_digest.NewInstanceName("rohan")) - instanceSlice := []bb_digest.InstanceName{instanceName} - digestFunction, err := instanceName.GetDigestFunction(remoteexecution.DigestFunction_SHA256, 0) - require.NoError(t, err) - - blobDigest := &remoteexecution.Digest{Hash: "b27cad931e1ef0a520887464127055ffd6db82c7b36bfea5cd832db65b8f816b", SizeBytes: 24} - uri := "https://raapi.test/blob" - assetRef := storage.NewAssetReference([]string{uri}, []*remoteasset.Qualifier{}) - assetData := storage.NewBlobAsset(blobDigest, timestamppb.Now()) - - baseStore := mock.NewMockAssetStore(ctrl) - fetchAuthorizer := mock.NewMockAuthorizer(ctrl) - pushAuthorizer := mock.NewMockAuthorizer(ctrl) - aas := storage.NewAuthorizingAssetStore(baseStore, fetchAuthorizer, pushAuthorizer) - - t.Run("Allowed", func(t *testing.T) { - pushAuthorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{nil}) - baseStore.EXPECT().Put(ctx, assetRef, assetData, digestFunction).Return(nil) - - err := aas.Put(ctx, assetRef, assetData, digestFunction) - require.NoError(t, err) - }) - - t.Run("Rejected", func(t *testing.T) { - pushAuthorizer.EXPECT().Authorize(ctx, instanceSlice).Return([]error{status.Error(codes.PermissionDenied, "None shall pass")}) - - err := aas.Put(ctx, assetRef, assetData, digestFunction) - require.Equal(t, status.Error(codes.PermissionDenied, "None shall pass"), err) - }) -}