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

[Stefan Priebe - Profihost AG]

thanks for review. V4 sent.

Stefan

Am 10.03.2017 um 10:20 schrieb Thomas Lamprecht:
> small comment inline,
> 
> On 03/09/2017 08:17 PM, Stefan Priebe wrote:
>> This allows us to use management software for files inside of /etc/pve.
>> f.e. saltstack which rely on being able to set uid,gid and chmod
>>
>> Signed-off-by: Stefan Priebe <s.priebe at profihost.ag>
>> ---
>>   data/src/pmxcfs.c | 33 ++++++++++++++++++++++++++++++++-
>>   1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/data/src/pmxcfs.c b/data/src/pmxcfs.c
>> index 1b6cbcc..1204331 100644
>> --- a/data/src/pmxcfs.c
>> +++ b/data/src/pmxcfs.c
>> @@ -186,6 +186,35 @@ 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);
>> +
>> +    // asserts 0640, but allows setting UID and GID - some programs
>> need that
>> +    if ((mode & ACCESSPERMS) == (S_IRUSR | S_IWUSR | S_IRGRP))
>> +        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);
> 
> Can we add the uid and gid to the debug message, I already needed them
> to review the patch:
> 
> cfs_debug("enter cfs_fuse_chown %s (uid: %d; gid: %d)", path, user, group);
> 
>> +
>> +    if (user == 0 && group == cfs.gid)
> 
> If we do not change either group or user chmod uses `-1` for that
> parameter.
> So something like this:
> 
>         // we get -1 if no change should be made
>         if ((user == 0 || user == -1) && (group == cfs.gid || group == -1))
> 
> should be done here, this allows also:
> 
> chmod root /etc/pve/...
> chmod :www-data /etc/pve/...
> 
> else only
> chmod root:www-data /etc/pve/...
> 
> was allowed (all three are valid for us).
> 
> The rest looks good for me as is!
> 
> With the above changes made you may pickup my reviewed tag - if wanted:
> Reviewed-by Thomas Lamprecht <t.lamprecht at proxmox.com>
> 
>> +        ret = 0;
>> +
>> +    cfs_debug("leave cfs_fuse_chown %s (%d)", path, ret);
>> +
>> +    return ret;
>> +}
>> +
>>   static int cfs_fuse_mkdir(const char *path, mode_t mode)
>>   {
>>       cfs_debug("enter cfs_fuse_mkdir %s", path);
>> @@ -488,7 +517,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 *
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel