Code review comment for ~jslarraz/ubuntu-qa-tools:fix-acl

On Mon, Apr 22, 2024 at 12:11:21PM -0000, Jorge Sancho Larraz wrote:
> When using acl, the `mask` attribute sets the maximum permissions a non-owner user may have. Thus, this field also needs to be check to ensure the `libvirt-qemu` has effective search permissions on the required directories
> --
> You are subscribed to branch ubuntu-qa-tools:master.

> diff --git a/vm-tools/uvt b/vm-tools/uvt
> index 2f6eca3..36a3c31 100755
> --- a/vm-tools/uvt
> +++ b/vm-tools/uvt
> @@ -3626,7 +3626,9 @@ def load_uvt_config():
> path = config[d]
> while path != "/":
> rc, out = runcmd(["getfacl", "-e", path])
> - if (not os.stat(path).st_mode & 0o001) and (re.search("user:libvirt-qemu:..x", out) is None):
> + if (not os.stat(path).st_mode & 0o001) and \
> + ((re.search("user:libvirt-qemu:..x", out) is None) or
> + (re.search("mask::..x", out) is None)):

Oh, I hadn't noticed we'd started using ACLs here as a fallback, yay!

This has already been merged, so treat this as a possible suggestion
for a future improvement, but rather than trying to parse the
output of the getfacl command (and making sure that things like the
LANG environment variable are set), would it make sense to use the
python3-pylibacl library to query acl status directly instead? It's
available in all the currently supported Ubuntu releases.

That said, it's not exactly the easiest interface to use, being a python
wrapper around libacl:

>>> import posix1e
>>> import pwd
>>> acls = posix1e.ACL(file="testimage-focal-amd64.qcow2")
>>> for acl in acls:
... print(acl)
... if acl.tag_type == posix1e.ACL_USER:
... print(f"--> username = {pwd.getpwuid(acl.qualifier).pw_name}")
... if acl.tag_type in (posix1e.ACL_MASK, posix1e.ACL_USER):
... print(f"--> read = {acl.permset.read}")
... print(f"--> write = {acl.permset.write}")
... print(f"--> execute = {acl.permset.execute}")
...
ACL entry for the owner
ACL entry for user with uid 64055
--> username = libvirt-qemu
--> read = True
--> write = False
--> execute = True
ACL entry for the group
ACL entry for the mask
--> read = True
--> write = False
--> execute = True
ACL entry for the others

--
Steve Beattie
<email address hidden>