Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions changelog/unreleased/fix-vault-share-grantee-permission.md
Original file line number Diff line number Diff line change
@@ -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
60 changes: 60 additions & 0 deletions internal/grpc/services/gateway/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -48,13 +50,71 @@ 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)
}
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) {
Expand Down
64 changes: 64 additions & 0 deletions internal/grpc/services/gateway/usershareprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
})
}
}
2 changes: 2 additions & 0 deletions pkg/permission/permission.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down