[Libvir] PATCH: Allow control over UNIX socket perms & group in libvirtd

When I submitted the patches for PolicyKit[1] support a few weeks back Rich suggested that we should have the ability to set UNIX socket permissions and group ownership regardless. So this patch adds that ability. The default setting is still, group=root, and mode=0700 for R/W socket and mode=0777 for the R/O socket. It is possible to override this via the config file eg, Don't allow R/O monitoring unix_sock_ro_perms="0700" eg, Allow any user in 'admin' group to manage VMs unix_sock_group="admin" unix_sock_rw_perms="0770" eg, Allow anyone todo anything unix_sock_rw_perms="0777" NB, the fchgrp, and fchown syscalls don't have any effect on sockets, so to set the group ownership & desired mode, I have to play games with the setgid() and umask() calls prior to bind(), and then restore them to their original values. NB, the virConf apis don't seem to recognise Octal numbers when parsing the config file, so I've used strings for the permissions. Not a big deal really unless someone desperately wants to fix the config file parser... Dan. [1] A new set of patches will be forthcoming soon... -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On Tue, Sep 18, 2007 at 05:38:09AM +0100, Daniel P. Berrange wrote:
When I submitted the patches for PolicyKit[1] support a few weeks back Rich suggested that we should have the ability to set UNIX socket permissions and group ownership regardless. So this patch adds that ability. The default setting is still, group=root, and mode=0700 for R/W socket and mode=0777 for the R/O socket.
It is possible to override this via the config file
eg, Don't allow R/O monitoring
unix_sock_ro_perms="0700"
eg, Allow any user in 'admin' group to manage VMs
unix_sock_group="admin" unix_sock_rw_perms="0770"
eg, Allow anyone todo anything
unix_sock_rw_perms="0777"
NB, the fchgrp, and fchown syscalls don't have any effect on sockets, so to set the group ownership & desired mode, I have to play games with the setgid() and umask() calls prior to bind(), and then restore them to their original values.
NB, the virConf apis don't seem to recognise Octal numbers when parsing the config file, so I've used strings for the permissions. Not a big deal really unless someone desperately wants to fix the config file parser... [...] +static gid_t unix_sock_gid = 0; /* Only root by default */ +static int unix_sock_rw_perms = 0700; /* Allow user only */ +static int unix_sock_ro_perms = 0777; /* Allow world */ [...] - if (readonly) - oldmask = umask(~(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)); - else - oldmask = umask(~(S_IRUSR | S_IWUSR)); + oldgrp = getgid(); + oldmask = umask(readonly ? ~unix_sock_ro_perms : ~unix_sock_rw_perms); + if (getuid() == 0) + setgid(unix_sock_gid); +
Looks fine but we went from the full macros definition to the pre digested octal value. But I'm not old enough to really care :-) +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Tue, Sep 18, 2007 at 04:17:32AM -0400, Daniel Veillard wrote:
On Tue, Sep 18, 2007 at 05:38:09AM +0100, Daniel P. Berrange wrote:
NB, the virConf apis don't seem to recognise Octal numbers when parsing the config file, so I've used strings for the permissions. Not a big deal really unless someone desperately wants to fix the config file parser... [...] +static gid_t unix_sock_gid = 0; /* Only root by default */ +static int unix_sock_rw_perms = 0700; /* Allow user only */ +static int unix_sock_ro_perms = 0777; /* Allow world */ [...] - if (readonly) - oldmask = umask(~(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)); - else - oldmask = umask(~(S_IRUSR | S_IWUSR)); + oldgrp = getgid(); + oldmask = umask(readonly ? ~unix_sock_ro_perms : ~unix_sock_rw_perms); + if (getuid() == 0) + setgid(unix_sock_gid); +
Looks fine but we went from the full macros definition to the pre digested octal value. But I'm not old enough to really care :-)
The umask was wanting a mask, while the configuration file (for end-user sanity) wants a mode instead. So I figured it was best to simply stick with mode throughout the code, and simply invert it when passing into umask at time of use. I added this to CVS & along with the updated docs on the 3 new configuration parameters for the daemon. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On Wed, Sep 19, 2007 at 03:35:50AM +0100, Daniel P. Berrange wrote:
On Tue, Sep 18, 2007 at 04:17:32AM -0400, Daniel Veillard wrote:
On Tue, Sep 18, 2007 at 05:38:09AM +0100, Daniel P. Berrange wrote:
NB, the virConf apis don't seem to recognise Octal numbers when parsing the config file, so I've used strings for the permissions. Not a big deal really unless someone desperately wants to fix the config file parser... [...] +static gid_t unix_sock_gid = 0; /* Only root by default */ +static int unix_sock_rw_perms = 0700; /* Allow user only */ +static int unix_sock_ro_perms = 0777; /* Allow world */ [...] - if (readonly) - oldmask = umask(~(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)); - else - oldmask = umask(~(S_IRUSR | S_IWUSR)); + oldgrp = getgid(); + oldmask = umask(readonly ? ~unix_sock_ro_perms : ~unix_sock_rw_perms); + if (getuid() == 0) + setgid(unix_sock_gid); +
Looks fine but we went from the full macros definition to the pre digested octal value. But I'm not old enough to really care :-)
The umask was wanting a mask, while the configuration file (for end-user sanity) wants a mode instead. So I figured it was best to simply stick with mode throughout the code, and simply invert it when passing into umask at time of use.
yeah, it's actually simpler to read and deal with
I added this to CVS & along with the updated docs on the 3 new configuration parameters for the daemon.
Cool, thanks Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (2)
-
Daniel P. Berrange
-
Daniel Veillard