diff --git a/changelog/unreleased/fix-share-jail-perms b/changelog/unreleased/fix-share-jail-perms new file mode 100644 index 00000000000..44279fcd1d6 --- /dev/null +++ b/changelog/unreleased/fix-share-jail-perms @@ -0,0 +1,5 @@ +Bugfix: fix the share jail permissions in the decomposedfs + +The share jail should be not writable + +https://github.com/cs3org/reva/pull/1939 diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index ccce2b4fce7..042de3052ba 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -53,7 +53,7 @@ import ( // PermissionsChecker defines an interface for checking permissions on a Node type PermissionsChecker interface { - AssemblePermissions(ctx context.Context, n *node.Node) (ap *provider.ResourcePermissions, err error) + AssemblePermissions(ctx context.Context, n *node.Node) (ap provider.ResourcePermissions, err error) HasPermission(ctx context.Context, n *node.Node, check func(*provider.ResourcePermissions) bool) (can bool, err error) } @@ -148,7 +148,7 @@ func (fs *Decomposedfs) GetQuota(ctx context.Context) (total uint64, inUse uint6 return 0, 0, errtypes.PermissionDenied(n.ID) } - ri, err := n.AsResourceInfo(ctx, rp, []string{"treesize", "quota"}) + ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}) if err != nil { return 0, 0, err } @@ -399,7 +399,7 @@ func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKe return nil, errtypes.PermissionDenied(node.ID) } - return node.AsResourceInfo(ctx, rp, mdKeys) + return node.AsResourceInfo(ctx, &rp, mdKeys) } // ListFolder returns a list of resources in the specified folder @@ -431,8 +431,9 @@ func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, for i := range children { np := rp // add this childs permissions - node.AddPermissions(np, n.PermissionSet(ctx)) - if ri, err := children[i].AsResourceInfo(ctx, np, mdKeys); err == nil { + pset := n.PermissionSet(ctx) + node.AddPermissions(&np, &pset) + if ri, err := children[i].AsResourceInfo(ctx, &np, mdKeys); err == nil { finfos = append(finfos, ri) } } diff --git a/pkg/storage/utils/decomposedfs/lookup.go b/pkg/storage/utils/decomposedfs/lookup.go index fe1fc315bca..ade953423ac 100644 --- a/pkg/storage/utils/decomposedfs/lookup.go +++ b/pkg/storage/utils/decomposedfs/lookup.go @@ -168,3 +168,8 @@ func (lu *Lookup) mustGetUserLayout(ctx context.Context) string { u := user.ContextMustGetUser(ctx) return templates.WithUser(u, lu.Options.UserLayout) } + +// ShareFolder returns the internal storage root directory +func (lu *Lookup) ShareFolder() string { + return lu.Options.ShareFolder +} diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 4b8a1f9f18d..afb0bc757d8 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -84,6 +84,7 @@ type PathLookup interface { InternalRoot() string InternalPath(ID string) string Path(ctx context.Context, n *Node) (path string, err error) + ShareFolder() string } // New returns a new instance of Node @@ -313,20 +314,20 @@ func (n *Node) Owner() (o *userpb.UserId, err error) { // PermissionSet returns the permission set for the current user // the parent nodes are not taken into account -func (n *Node) PermissionSet(ctx context.Context) *provider.ResourcePermissions { +func (n *Node) PermissionSet(ctx context.Context) provider.ResourcePermissions { u, ok := user.ContextGetUser(ctx) if !ok { appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("no user in context, returning default permissions") - return NoPermissions + return NoPermissions() } if o, _ := n.Owner(); isSameUserID(u.Id, o) { - return OwnerPermissions + return OwnerPermissions() } // read the permissions for the current user from the acls of the current node if np, err := n.ReadUserPermissions(ctx, u); err == nil { return np } - return NoPermissions + return NoPermissions() } // InternalPath returns the internal path of the Node @@ -733,25 +734,25 @@ func (n *Node) UnsetTempEtag() (err error) { } // ReadUserPermissions will assemble the permissions for the current user on the given node without parent nodes -func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap *provider.ResourcePermissions, err error) { +func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap provider.ResourcePermissions, err error) { // check if the current user is the owner o, err := n.Owner() if err != nil { // TODO check if a parent folder has the owner set? appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not determine owner, returning default permissions") - return NoPermissions, err + return NoPermissions(), err } if o.OpaqueId == "" { // this happens for root nodes in the storage. the extended attributes are set to emptystring to indicate: no owner // TODO what if no owner is set but grants are present? - return NoOwnerPermissions, nil + return NoOwnerPermissions(), nil } if isSameUserID(u.Id, o) { appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("user is owner, returning owner permissions") - return OwnerPermissions, nil + return OwnerPermissions(), nil } - ap = &provider.ResourcePermissions{} + ap = provider.ResourcePermissions{} // for an efficient group lookup convert the list of groups to a map // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! @@ -766,7 +767,7 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap *pro var grantees []string if grantees, err = n.ListGrantees(ctx); err != nil { appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("error listing grantees") - return nil, err + return NoPermissions(), err } // instead of making n getxattr syscalls we are going to list the acls and filter them here @@ -796,7 +797,7 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap *pro switch { case err == nil: - AddPermissions(ap, g.GetPermissions()) + AddPermissions(&ap, g.GetPermissions()) case isNoData(err): err = nil appctx.GetLogger(ctx).Error().Interface("node", n).Str("grant", grantees[i]).Interface("grantees", grantees).Msg("grant vanished from node after listing") diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index ea3e5cae9dc..49ebd922142 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -32,35 +32,54 @@ import ( "github.com/pkg/xattr" ) -// NoPermissions represents an empty set of permssions -var NoPermissions *provider.ResourcePermissions = &provider.ResourcePermissions{} +// NoPermissions represents an empty set of permissions +func NoPermissions() provider.ResourcePermissions { + return provider.ResourcePermissions{} +} // NoOwnerPermissions defines permissions for nodes that don't have an owner set, eg the root node -var NoOwnerPermissions *provider.ResourcePermissions = &provider.ResourcePermissions{ - Stat: true, +func NoOwnerPermissions() provider.ResourcePermissions { + return provider.ResourcePermissions{ + Stat: true, + } +} + +// ShareFolderPermissions defines permissions for the shared jail +func ShareFolderPermissions() provider.ResourcePermissions { + return provider.ResourcePermissions{ + // read permissions + ListContainer: true, + Stat: true, + InitiateFileDownload: true, + GetPath: true, + GetQuota: true, + ListFileVersions: true, + } } // OwnerPermissions defines permissions for nodes owned by the user -var OwnerPermissions *provider.ResourcePermissions = &provider.ResourcePermissions{ - // all permissions - AddGrant: true, - CreateContainer: true, - Delete: true, - GetPath: true, - GetQuota: true, - InitiateFileDownload: true, - InitiateFileUpload: true, - ListContainer: true, - ListFileVersions: true, - ListGrants: true, - ListRecycle: true, - Move: true, - PurgeRecycle: true, - RemoveGrant: true, - RestoreFileVersion: true, - RestoreRecycleItem: true, - Stat: true, - UpdateGrant: true, +func OwnerPermissions() provider.ResourcePermissions { + return provider.ResourcePermissions{ + // all permissions + AddGrant: true, + CreateContainer: true, + Delete: true, + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + ListContainer: true, + ListFileVersions: true, + ListGrants: true, + ListRecycle: true, + Move: true, + PurgeRecycle: true, + RemoveGrant: true, + RestoreFileVersion: true, + RestoreRecycleItem: true, + Stat: true, + UpdateGrant: true, + } } // Permissions implements permission checks @@ -76,38 +95,41 @@ func NewPermissions(lu PathLookup) *Permissions { } // AssemblePermissions will assemble the permissions for the current user on the given node, taking into account all parent nodes -func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap *provider.ResourcePermissions, err error) { +func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap provider.ResourcePermissions, err error) { u, ok := user.ContextGetUser(ctx) if !ok { appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("no user in context, returning default permissions") - return NoPermissions, nil + return NoPermissions(), nil } // check if the current user is the owner o, err := n.Owner() if err != nil { // TODO check if a parent folder has the owner set? appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not determine owner, returning default permissions") - return NoPermissions, err + return NoPermissions(), err } if o.OpaqueId == "" { // this happens for root nodes in the storage. the extended attributes are set to emptystring to indicate: no owner // TODO what if no owner is set but grants are present? - return NoOwnerPermissions, nil + return NoOwnerPermissions(), nil } if isSameUserID(u.Id, o) { + lp, err := n.lu.Path(ctx, n) + if err == nil && lp == n.lu.ShareFolder() { + return ShareFolderPermissions(), nil + } appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("user is owner, returning owner permissions") - return OwnerPermissions, nil + return OwnerPermissions(), nil } - // determine root var rn *Node if rn, err = p.lu.RootNode(ctx); err != nil { - return nil, err + return NoPermissions(), err } cn := n - ap = &provider.ResourcePermissions{} + ap = provider.ResourcePermissions{} // for an efficient group lookup convert the list of groups to a map // groups are just strings ... groupnames ... or group ids ??? AAARGH !!! @@ -118,14 +140,12 @@ func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap *pro // for all segments, starting at the leaf for cn.ID != rn.ID { - if np, err := cn.ReadUserPermissions(ctx, u); err == nil { - AddPermissions(ap, np) + AddPermissions(&ap, &np) } else { appctx.GetLogger(ctx).Error().Err(err).Interface("node", cn).Msg("error reading permissions") // continue with next segment } - if cn, err = cn.Parent(); err != nil { return ap, errors.Wrap(err, "Decomposedfs: error getting parent "+cn.ParentID) } @@ -232,7 +252,7 @@ func (p *Permissions) HasPermission(ctx context.Context, n *Node, check func(*pr } } - appctx.GetLogger(ctx).Debug().Interface("permissions", NoPermissions).Interface("node", n).Interface("user", u).Msg("no grant found, returning default permissions") + appctx.GetLogger(ctx).Debug().Interface("permissions", NoPermissions()).Interface("node", n).Interface("user", u).Msg("no grant found, returning default permissions") return false, nil } @@ -240,22 +260,26 @@ func (p *Permissions) getUserAndPermissions(ctx context.Context, n *Node) (*user u, ok := user.ContextGetUser(ctx) if !ok { appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("no user in context, returning default permissions") - return nil, NoPermissions + perms := NoPermissions() + return nil, &perms } // check if the current user is the owner o, err := n.Owner() if err != nil { appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not determine owner, returning default permissions") - return nil, NoPermissions + perms := NoPermissions() + return nil, &perms } if o.OpaqueId == "" { // this happens for root nodes in the storage. the extended attributes are set to emptystring to indicate: no owner // TODO what if no owner is set but grants are present? - return nil, NoOwnerPermissions + perms := NoOwnerPermissions() + return nil, &perms } if isSameUserID(u.Id, o) { appctx.GetLogger(ctx).Debug().Interface("node", n).Msg("user is owner, returning owner permissions") - return u, OwnerPermissions + perms := OwnerPermissions() + return u, &perms } return u, nil } diff --git a/pkg/storage/utils/decomposedfs/recycle.go b/pkg/storage/utils/decomposedfs/recycle.go index d3dcebe24af..cd45269d14d 100644 --- a/pkg/storage/utils/decomposedfs/recycle.go +++ b/pkg/storage/utils/decomposedfs/recycle.go @@ -54,12 +54,12 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, key, path string) ([]*p // TODO how do we check if the storage allows listing the recycle for the current user? check owner of the root of the storage? // use permissions ReadUserPermissions? if fs.o.EnableHome { - if !node.OwnerPermissions.ListContainer { + if !node.OwnerPermissions().ListContainer { log.Debug().Msg("owner not allowed to list trash") return items, errtypes.PermissionDenied("owner not allowed to list trash") } } else { - if !node.NoPermissions.ListContainer { + if !node.NoPermissions().ListContainer { log.Debug().Msg("default permissions prevent listing trash") return items, errtypes.PermissionDenied("default permissions prevent listing trash") } diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 934799f9e19..c7211fc882c 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -61,6 +61,7 @@ type PathLookup interface { InternalRoot() string InternalPath(ID string) string Path(ctx context.Context, n *node.Node) (path string, err error) + ShareFolder() string } // Tree manages a hierarchical tree