[pve-devel] [PATCH cluster v5] implement chown and chmod for user root group www-data and perm 0640

[Wolfgang Bumiller]

On Tue, Mar 28, 2017 at 03:10:24PM +0200, Thomas Lamprecht wrote:
> From: Stefan Priebe <s.priebe at profihost.ag>
> 
> This allows us to use management software for files inside of /etc/pve.
> e.g. saltstack which rely on being able to set uid,gid and chmod
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> Signed-off-by: Stefan Priebe <s.priebe at profihost.ag>
> ---
> 
> changes since Stefans V4:
> * refactored cfs_fuse_chmod to reduce if nesting level
> 
>  data/src/pmxcfs.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/data/src/pmxcfs.c b/data/src/pmxcfs.c
> index 1b6cbcc..6afe292 100644
> --- a/data/src/pmxcfs.c
> +++ b/data/src/pmxcfs.c
> @@ -186,6 +186,40 @@ ret:
>  	return ret;
>  }
>  
> +static int cfs_fuse_chmod(const char *path, mode_t mode)
> +{
> +	int ret = -EACCES;
> +
> +	cfs_debug("enter cfs_fuse_chmod %s", path);
> +
> +	mode_t allowed_mode = (S_IRUSR | S_IWUSR);
> +	if (!path_is_private(path))
> +		allowed_mode |= (S_IRGRP);
> +
> +	// ACCESSPERMS masks out UID and GID for the check, some programs need to set them
> +	if ((mode & ACCESSPERMS) == allowed_mode)
> +		ret = 0;

If you really only check ACCESSPERMS you also return success when
someone tries to set unsupported bits such as the sticky bit or
setuid/setgid. Since these are not supported it makes more sense to
return -ENOTSUPP in those cases. Iow. use something like:

    if (mode & ~ACCESSPERMS)
        ret = -ENOTSUPP;
    else if ((mode & ACCESSPERMS) == allowed_mode)
        ret = 0;

> +
> +	cfs_debug("leave cfs_fuse_chmod %s (%d) mode: %o", path, ret, (int)mode);
> +
> +	return ret;
> +}
> +
> +static int cfs_fuse_chown(const char *path, uid_t user, gid_t group)
> +{
> +	int ret = -EACCES;
> +
> +	cfs_debug("enter cfs_fuse_chown %s", path);
> +
> +	// we get -1 if no change should be made
> +	if ((user == 0 || user == -1) && (group == cfs.gid || group == -1))
> +		ret = 0;
> +
> +	cfs_debug("leave cfs_fuse_chown %s (%d) (uid: %d; gid: %d)", path, ret, user, group);
> +
> +	return ret;
> +}
> +
>  static int cfs_fuse_mkdir(const char *path, mode_t mode)
>  {
>  	cfs_debug("enter cfs_fuse_mkdir %s", path);
> @@ -488,7 +522,9 @@ static struct fuse_operations fuse_ops = {
>  	.readlink = cfs_fuse_readlink,
>  	.utimens = cfs_fuse_utimens,
>  	.statfs = cfs_fuse_statfs,
> -	.init = cfs_fuse_init
> +	.init = cfs_fuse_init,
> +	.chown = cfs_fuse_chown,
> +	.chmod = cfs_fuse_chmod
>  };
>  
>  static char *
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel