Re: [PATCH] storage: Add support to set{uid,gid} and sticky bit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 21, 2020 at 02:36:59PM -0300, Julio Faracco wrote:
Hi Jano!

Em qui., 20 de fev. de 2020 às 12:57, Ján Tomko <jtomko@xxxxxxxxxx> escreveu:

On Wed, Feb 19, 2020 at 05:51:44PM -0300, Julio Faracco wrote:
>This commit add more features to storages that supports setuid, setgid
>and sticky bit. This extend some permission levels of volumes when you
>run an hypervisor using a specific user that can run but cannot delete
>volumes for instance.

I'm confused about the use case here - volumes should not be executable
and setuid/setgid only make sense on executable files.

>Additionally, when you create a directory without
>`pool-build` command, you cannot import those extra permissions.
>Example:
>
>    # mkdir /var/lib/libvirt/images/
>    # chmod 0755 /var/lib/libvirt/images/
>    # chmod u+s /var/lib/libvirt/images/
>    # pool-start default
>    # pool-dumpxml default
>
>No setuid from `<mode>0755</mode>`.
>Output should expect `<mode>4755</mode>`.
>

FYI I proposed a similar patch ~7.5 years ago (and still haven't
bothered to resend it O:-)):
https://www.redhat.com/archives/libvir-list/2012-August/msg00687.html
https://www.redhat.com/archives/libvir-list/2012-August/msg01004.html

The consensus seemed to be
* not wanting to touch the SGID/SUID bits
* reporting the perms should be OK

Sorry, I usually search if someone proposed a similar patch but
probably Google didn't find a 7 years patch hehe
What is the problem that I'm trying to solve?
Basically, I have some directories that already have pre-configured
permissions with those extra bits.

Glad to hear this can actually be useful for someone :)

If I run `build`, libvirt will overwrite all of them.
The same for reading and dumpxml.


Building a directory is handled in virStorageBackendBuildLocal,
a different code path than you're touching in src/storage.

It seems loosening up the parser checks is all it takes to get libvirt
to create directories with those permissions, which is why I suggested
checks in the callers of virStorageDefParsePerms to make sure we do it
because we want to allow it, not because we forgot to check it.

Same goes for every user of target.perms.mode

If someone has a better idea to solve this, I would appreciate a lot. :-)


For regular files, these bits seem to be useless for volumes, I think
we should reject them.
For directories, SGID and sticky might make sense


Jano

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux