fix permissions for share jail (#1939)

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

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)
 		}
 	}

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
+}

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")

pkg/storage/utils/decomposedfs/node/node_test.go

@@ -171,20 +171,22 @@ var _ = Describe("Node", func() {
 
 		Describe("the Etag field", func() {
 			It("is set", func() {
-				ri, err := n.AsResourceInfo(env.Ctx, node.OwnerPermissions, []string{})
+				perms := node.OwnerPermissions()
+				ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(len(ri.Etag)).To(Equal(34))
 			})
 
 			It("changes when the tmtime is set", func() {
-				ri, err := n.AsResourceInfo(env.Ctx, node.OwnerPermissions, []string{})
+				perms := node.OwnerPermissions()
+				ri, err := n.AsResourceInfo(env.Ctx, &perms, []string{})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(len(ri.Etag)).To(Equal(34))
 				before := ri.Etag
 
 				Expect(n.SetTMTime(time.Now().UTC())).To(Succeed())
 
-				ri, err = n.AsResourceInfo(env.Ctx, node.OwnerPermissions, []string{})
+				ri, err = n.AsResourceInfo(env.Ctx, &perms, []string{})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(len(ri.Etag)).To(Equal(34))
 				Expect(ri.Etag).ToNot(Equal(before))

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,30 +252,34 @@ 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
 }
 
 func (p *Permissions) getUserAndPermissions(ctx context.Context, n *Node) (*userv1beta1.User, *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 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
 }

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")
 		}

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