diff --git a/changelog/unreleased/fix-vault-share-grantee-permission.md b/changelog/unreleased/fix-vault-share-grantee-permission.md new file mode 100644 index 00000000000..1160b9e86e9 --- /dev/null +++ b/changelog/unreleased/fix-vault-share-grantee-permission.md @@ -0,0 +1,15 @@ +Bugfix: Only share vault resources with grantees allowed to use vault mode + +Vault ("Safe") resources could be shared with any grantee, including users that lack +the `VaultMode.ReadWriteEnabled` permission and can therefore never enter vault mode. +Such a grant was useless to the grantee but still produced a share notification that +disclosed the resource name to them. + +Share creation on a vault resource now verifies that the grantee holds the vault mode +permission and is denied otherwise. The check is applied in the gateway ahead of the +space root branch, so it covers plain shares as well as space memberships, and it fails +closed if the permission cannot be determined. Group grantees are rejected, because +settings role assignments are held by individual accounts and group membership can +change after the share has been created. + +https://github.com/owncloud/reva/pull/727 diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 7d0980f71ca..5a7b07fc5c7 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -23,6 +23,7 @@ import ( "slices" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -31,6 +32,7 @@ import ( "github.com/owncloud/reva/v2/pkg/conversions" ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" "github.com/owncloud/reva/v2/pkg/errtypes" + "github.com/owncloud/reva/v2/pkg/permission" "github.com/owncloud/reva/v2/pkg/rgrpc/status" "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/storage/utils/grants" @@ -48,6 +50,24 @@ func hasGrantManagementBits(p *provider.ResourcePermissions) bool { // TODO(labkode): add multi-phase commit logic when commit share or commit ref is enabled. func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) { + // Vault resources must only be shared with grantees that may use vault mode. + if req.GetResourceInfo().GetId().GetStorageId() == utils.VaultStorageProviderID { + allowed, err := s.granteeMayAccessVault(ctx, req.GetGrant().GetGrantee()) + if err != nil { + appctx.GetLogger(ctx).Error().Err(err). + Interface("grantee", req.GetGrant().GetGrantee()). + Msg("could not determine whether the grantee may access vault resources") + return &collaboration.CreateShareResponse{ + Status: status.NewInternal(ctx, "error checking grantee vault permission"), + }, nil + } + if !allowed { + return &collaboration.CreateShareResponse{ + Status: status.NewPermissionDenied(ctx, nil, "grantee is not allowed to access vault resources"), + }, nil + } + } + // Don't use the share manager when sharing a space root if !s.c.UseCommonSpaceRootShareLogic && refIsSpaceRoot(req.ResourceInfo.Id) { return s.addSpaceShare(ctx, req) @@ -55,6 +75,46 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq return s.addShare(ctx, req) } +// granteeMayAccessVault reports whether the grantee may be granted access to a vault resource. +// A non-nil error means undetermined, never denied, so callers must fail closed. +func (s *svc) granteeMayAccessVault(ctx context.Context, g *provider.Grantee) (bool, error) { + ref := vaultPermissionSubject(g) + if ref == nil { + return false, nil + } + + res, err := s.CheckPermission(ctx, &permissions.CheckPermissionRequest{ + SubjectRef: ref, + Permission: permission.VaultMode, + }) + if err != nil { + return false, err + } + return vaultPermissionGranted(res.GetStatus()) +} + +// vaultPermissionSubject returns the subject to check VaultMode for, or nil for grantees that can never hold it, like groups. +func vaultPermissionSubject(g *provider.Grantee) *permissions.SubjectReference { + if g.GetType() != provider.GranteeType_GRANTEE_TYPE_USER { + return nil + } + return &permissions.SubjectReference{ + Spec: &permissions.SubjectReference_UserId{UserId: g.GetUserId()}, + } +} + +// vaultPermissionGranted interprets the status of a VaultMode check, erring on anything but a definitive answer. +func vaultPermissionGranted(st *rpc.Status) (bool, error) { + switch code := st.GetCode(); code { + case rpc.Code_CODE_OK: + return true, nil + case rpc.Code_CODE_PERMISSION_DENIED: + return false, nil + default: + return false, errors.Errorf("vault permission check returned non-authoritative status %q: %s", code, st.GetMessage()) + } +} + func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareRequest) (*collaboration.RemoveShareResponse, error) { key := req.GetRef().GetKey() if !s.c.UseCommonSpaceRootShareLogic && shareIsSpaceRoot(key) { diff --git a/internal/grpc/services/gateway/usershareprovider_test.go b/internal/grpc/services/gateway/usershareprovider_test.go index 3735b97a9e7..3462a2db2bb 100644 --- a/internal/grpc/services/gateway/usershareprovider_test.go +++ b/internal/grpc/services/gateway/usershareprovider_test.go @@ -21,7 +21,9 @@ package gateway import ( "testing" + grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/owncloud/reva/v2/pkg/conversions" @@ -132,3 +134,65 @@ func TestSpaceShareRejectsPartialGrantManagement(t *testing.T) { t.Error("space editor grant should be accepted on a space root") } } + +func groupGrantee(id string) *provider.Grantee { + return &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_GROUP, + Id: &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{OpaqueId: id}}, + } +} + +// TestVaultPermissionSubject covers which grantees are eligible for a VaultMode check at all. +func TestVaultPermissionSubject(t *testing.T) { + tests := []struct { + name string + grantee *provider.Grantee + wantSubject bool + }{ + {"user grantee is checked", userGrantee("alice"), true}, + {"group grantee is denied outright", groupGrantee("students"), false}, + {"invalid grantee is denied outright", &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_INVALID}, false}, + {"nil grantee is denied outright", nil, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := vaultPermissionSubject(tt.grantee) + if (got != nil) != tt.wantSubject { + t.Fatalf("vaultPermissionSubject() subject = %v, want subject %v", got, tt.wantSubject) + } + if tt.wantSubject && got.GetUserId().GetOpaqueId() != tt.grantee.GetUserId().GetOpaqueId() { + t.Errorf("vaultPermissionSubject() checked %q, want the grantee %q", + got.GetUserId().GetOpaqueId(), tt.grantee.GetUserId().GetOpaqueId()) + } + }) + } +} + +// TestVaultPermissionGranted covers the fail-closed contract: only a definitive OK grants access. +func TestVaultPermissionGranted(t *testing.T) { + tests := []struct { + name string + status *rpc.Status + want bool + wantErr bool + }{ + {"ok grants", &rpc.Status{Code: rpc.Code_CODE_OK}, true, false}, + {"permission denied denies", &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}, false, false}, + {"internal error fails closed", &rpc.Status{Code: rpc.Code_CODE_INTERNAL}, false, true}, + {"not found fails closed", &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}, false, true}, + {"unset status fails closed", nil, false, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := vaultPermissionGranted(tt.status) + if (err != nil) != tt.wantErr { + t.Fatalf("vaultPermissionGranted() error = %v, wantErr %v", err, tt.wantErr) + } + if got != tt.want { + t.Errorf("vaultPermissionGranted() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/permission/permission.go b/pkg/permission/permission.go index fb82add4439..607198931b5 100644 --- a/pkg/permission/permission.go +++ b/pkg/permission/permission.go @@ -37,6 +37,8 @@ const ( WriteFavorites string = "Favorites.Write" // DeleteReadOnlyPassword is the hardcoded name for the ReadOnlyPublicLinkPassword.Delete permission DeleteReadOnlyPassword string = "ReadOnlyPublicLinkPassword.Delete" + // VaultMode is the hardcoded name for the VaultMode.ReadWriteEnabled permission + VaultMode string = "VaultMode.ReadWriteEnabled" ) // Manager defines the interface for the permission service driver