Bug 985269 - Can't set acl value for a specified user with 'acl-set-file'
Can't set acl value for a specified user with 'acl-set-file'
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libguestfs (Show other bugs)
6.5
Unspecified Unspecified
medium Severity medium
: rc
: ---
Assigned To: Richard W.M. Jones
Virtualization Bugs
:
Depends On:
Blocks: 985856
  Show dependency treegraph
 
Reported: 2013-07-17 04:26 EDT by bfan
Modified: 2013-11-20 23:46 EST (History)
2 users (show)

See Also:
Fixed In Version: libguestfs-1.20.10-1.el6
Doc Type: Bug Fix
Doc Text:
Cause: Setting file ACLs via libguestfs. Consequence: These would fail because the syntax for ACLs is very obscure. Fix: No fix, we have just documented what the correct syntax should be in the manual.
Story Points: ---
Clone Of:
: 985856 (view as bug list)
Environment:
Last Closed: 2013-11-20 23:46:01 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description bfan 2013-07-17 04:26:06 EDT
Description of problem:
Command 'acl-set-file' failed for a specified user


Version-Release number of selected component (if applicable):
libguestfs-1.20.9-6.el6.x86_64


How reproducible:
always


Steps to reproduce:
1. Prepare a linux guest(rhel6.4), and add a user named "acltest" in guest
2. Create a file and set acl value for user 'acltest'
    # setfacl -m u:acltest:r-x /home/test

3. lauch guestfish to check
><fs> guestfish -a RHEL-Server-6.4-64-hvm.raw
><fs> run
><fs> mount-options acl /dev/VolGroup/lv_root /
><fs> acl-get-file /home/test access
user::rwx
user:500:r-x        # we can see it uses UID here
group::rwx
mask::rwx
other::rwx

><fs> cat /etc/passwd
...
acltest:500:500::/home/acltest:/bin/bash

# set acl value with user uid
><fs> acl-set-file /home/test access 'u:500:rwx,g::rwx,o::rwx'
libguestfs: error: acl_set_file: /home/test: Invalid argument

# set acl value with user name
><fs> acl-set-file /home/test access 'u:acltest:rwx,g::rwx,o::rwx'
libguestfs: error: acl_set_file: could not parse acl string: u:acltest:rwx,g::rwx,o::rwx: acl_from_text: Success


Actual results:
set acl value for a specified user failed


Expected results:
can set acl value for a specified user


Additional info:
Same issue is in rhel7
Comment 1 Richard W.M. Jones 2013-07-17 04:59:39 EDT
We don't resolve UIDs against the guest database.  The caller
has to resolve usernames to UIDs (eg. using Augeas).  Compare
it to the 'chown' call which uses UIDs, not user names.

> ><fs> acl-set-file /home/test access 'u:500:rwx,g::rwx,o::rwx'
> libguestfs: error: acl_set_file: /home/test: Invalid argument

This failed presumably because you used the wrong path?

> ><fs> acl-set-file /home/test access 'u:acltest:rwx,g::rwx,o::rwx'
> libguestfs: error: acl_set_file: could not parse acl string: u:acltest:rwx,g::rwx,o::rwx: acl_from_text: Success

It shouldn't say "Success" here.  It looks like there's
a bug in acl_from_text -- it doesn't set errno.  According
to the documentation it should set errno.
Comment 2 Richard W.M. Jones 2013-07-17 05:05:00 EDT
It's actually a bug in how acl_from_text uses getpwnam:

/* libacl/acl_from_text.c */

static int
get_uid(const char *token, uid_t *uid_p)
{
        struct passwd *passwd;

        if (get_id(token, uid_p) == 0)
                return 0;
        passwd = getpwnam(token);
        if (passwd) {
                *uid_p = passwd->pw_uid;
                return 0;
        }
        return -1;
}

It expects that errno is always set, but getpwnam only sets
errno if there was an error.  It can return NULL if the user
is not found, in which case errno is not touched.

This code has other failures too.
Comment 3 bfan 2013-07-17 06:07:21 EDT
(In reply to Richard W.M. Jones from comment #1)
> We don't resolve UIDs against the guest database.  The caller
> has to resolve usernames to UIDs (eg. using Augeas).  Compare
> it to the 'chown' call which uses UIDs, not user names.
> 
> > ><fs> acl-set-file /home/test access 'u:500:rwx,g::rwx,o::rwx'
> > libguestfs: error: acl_set_file: /home/test: Invalid argument
> 
> This failed presumably because you used the wrong path?
> 
No, it's a correct path.

> > ><fs> acl-set-file /home/test access 'u:acltest:rwx,g::rwx,o::rwx'
> > libguestfs: error: acl_set_file: could not parse acl string: u:acltest:rwx,g::rwx,o::rwx: acl_from_text: Success
> 
> It shouldn't say "Success" here.  It looks like there's
> a bug in acl_from_text -- it doesn't set errno.  According
> to the documentation it should set errno.

<fs> acl-set-file /home/test access 'u:root:rwx,g::rwx,o::rwx'
libguestfs: error: acl_set_file: /home/test: Invalid argument

if user is root, it will give this error message

Does this command can set independent access rights for a specified user, if not, what it can do?
Comment 4 Richard W.M. Jones 2013-07-17 07:01:45 EDT
Here's my self-contained test script:

guestfish -x -N fs:ext4 -m /dev/sda1:/:acl <<EOF                                
mkdir /test                                                                     
-acl-set-file /test access u:500:rwx,g::rwx,o::rwx                              
-acl-set-file /test access u:root:rwx,g::rwx,o::rwx                             
-acl-set-file /test access u:notauser:rwx,g::rwx,o::rwx                         
EOF                                                                             

This does appear to fail.  Here is the output:

libguestfs: trace: mkdir "/test"
libguestfs: trace: mkdir = 0
libguestfs: trace: acl_set_file "/test" "access" "u:500:rwx,g::rwx,o::rwx"
libguestfs: trace: acl_set_file = -1 (error)
*stdin*:4: libguestfs: error: acl_set_file: /test: Invalid argument
libguestfs: trace: acl_set_file "/test" "access" "u:root:rwx,g::rwx,o::rwx"
libguestfs: trace: acl_set_file = -1 (error)
*stdin*:6: libguestfs: error: acl_set_file: /test: Invalid argument
libguestfs: trace: acl_set_file "/test" "access" "u:notauser:rwx,g::rwx,o::rwx"
libguestfs: trace: acl_set_file = -1 (error)
*stdin*:8: libguestfs: error: acl_set_file: could not parse acl string: u:notauser:rwx,g::rwx,o::rwx: acl_from_text: Success
Comment 5 Richard W.M. Jones 2013-07-17 07:23:49 EDT
(In reply to Richard W.M. Jones from comment #4)
> Here's my self-contained test script:

Actually that test is incorrect.  You have to set regular
permissions before setting extra ACLs.  A corrected test is:

guestfish -x -N fs:ext4 -m /dev/sda1:/:acl <<EOF                                
mkdir /testdir                                                                  
touch /testfile                                                                 
                                                                                
acl-set-file /testdir access u::rwx,g::rwx,o::rwx,u:500:rwx                     
acl-set-file /testfile access u::rwx,g::rwx,o::rwx,u:500:rwx                    
                                                                                
acl-set-file /testdir access u::rwx,g::rwx,o::rwx,u:root:rwx                    
acl-set-file /testfile access u::rwx,g::rwx,o::rwx,u:root:rwx                   
                                                                                
# notauser doesn't exist, so we expect a real error.                            
-acl-set-file /testdir access u:notauser:rwx,g::rwx,o::rwx                      
EOF                                                                             

Note the corrected test is still failing, so it does seem
as if there's a real error here.  (The equivalent setfacl
command does not fail).
Comment 6 Richard W.M. Jones 2013-07-17 07:41:55 EDT
Apparently you have to set the 'mask' entry.  The setfacl
program does some trickery behind the scenes, automatically
setting mask, so it's not clear that you have to do this
if you use setfacl.

Here's a corrected, and working program:

guestfish -x -N fs:ext4 -m /dev/sda1:/:acl <<EOF
mkdir /testdir
touch /testfile

acl-set-file /testdir access u::rwx,g::rwx,o::rwx,m::rwx,u:500:rwx
acl-set-file /testfile access u::rwx,g::rwx,o::rwx,m::rwx,u:500:rwx

acl-set-file /testdir access u::rwx,g::rwx,o::rwx,m::rwx,u:root:rwx
acl-set-file /testfile access u::rwx,g::rwx,o::rwx,m::rwx,u:root:rwx

# notauser doesn't exist, so we expect a real error.
-acl-set-file /testdir access u:notauser:rwx,g::rwx,o::rwx
EOF

Output:

libguestfs: trace: mkdir "/testdir"
libguestfs: trace: mkdir = 0
libguestfs: trace: touch "/testfile"
libguestfs: trace: touch = 0
libguestfs: trace: acl_set_file "/testdir" "access" "u::rwx,g::rwx,o::rwx,m::rwx,u:500:rwx"
libguestfs: trace: acl_set_file = 0
libguestfs: trace: acl_set_file "/testfile" "access" "u::rwx,g::rwx,o::rwx,m::rwx,u:500:rwx"
libguestfs: trace: acl_set_file = 0
libguestfs: trace: acl_set_file "/testdir" "access" "u::rwx,g::rwx,o::rwx,m::rwx,u:root:rwx"
libguestfs: trace: acl_set_file = 0
libguestfs: trace: acl_set_file "/testfile" "access" "u::rwx,g::rwx,o::rwx,m::rwx,u:root:rwx"
libguestfs: trace: acl_set_file = 0
# the following error is expected:
libguestfs: trace: acl_set_file "/testdir" "access" "u:notauser:rwx,g::rwx,o::rwx"
libguestfs: trace: acl_set_file = -1 (error)
*stdin*:11: libguestfs: error: acl_set_file: could not parse acl string: u:notauser:rwx,g::rwx,o::rwx: acl_from_text: Success
Comment 7 bfan 2013-07-17 22:43:34 EDT
Jones,
Thanks for your help, it worked with your approach. I think we'd better to update helpout and gives the usage for customer.

What do you think?
Comment 8 Richard W.M. Jones 2013-07-18 05:04:57 EDT
Yup, this should be documented since it's very obscure.

Added upstream:

https://github.com/libguestfs/libguestfs/commit/758a2262f5f4033de3400bbe00fac2f85310b4ff
Comment 10 bfan 2013-08-06 03:18:37 EDT
Verified with libguestfs-1.20.10-2.el6.x86_64

><fs> help acl-set-file
NAME
    acl-set-file - set the POSIX ACL attached to a file

SYNOPSIS
     acl-set-file path acltype acl

DESCRIPTION
    This function sets the POSIX Access Control List (ACL) attached to
    "path".

    The "acltype" parameter may be:

    "access"
        Set the ordinary (access) ACL for any file, directory or other
        filesystem object.

    "default"
        Set the default ACL. Normally this only makes sense if "path" is a
        directory.

    The "acl" parameter is the new ACL in either "long text form" or "short
    text form" (see acl(5)). The new ACL completely replaces any previous
    ACL on the file. The ACL must contain the full Unix permissions (eg.
    "u::rwx,g::rx,o::rx").

    If you are specifying individual users or groups, then the mask field is
    also required (eg. "m::rwx"), followed by the "u:*ID*:..." and/or
    "g:*ID*:..." field(s). A full ACL string might therefore look like this:

     u::rwx,g::rwx,o::rwx,m::rwx,u:500:rwx,g:500:rwx
     \ Unix permissions / \mask/ \      ACL        /

    You should use numeric UIDs and GIDs. To map usernames and groupnames to
    the correct numeric ID in the context of the guest, use the Augeas
    functions (see "aug_init").

Add detailed helpout for this command, so change the status to verified
Comment 12 errata-xmlrpc 2013-11-20 23:46:01 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2013-1536.html

Note You need to log in before you can comment on or make changes to this bug.