[libvirt] [PATCH] libvirtd: Enable private /tmp under systemd.
The last intentional use of /tmp by libvirt was patched in commit bd6083c9b; we can add an extra measure of security by explicitly requesting that libvirtd's /tmp is not visible to arbitrary users. See https://bugzilla.redhat.com/782474 * daemon/libvirtd.service.in (Service): Enable PrivateTmp. --- daemon/libvirtd.service.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 8f2458a..cf68440 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -17,6 +17,7 @@ ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID # Override the maximum number of opened files #LimitNOFILE=2048 +PrivateTmp=true [Install] WantedBy=multi-user.target -- 1.7.7.6
On 02/06/2012 02:15 PM, Eric Blake wrote:
The last intentional use of /tmp by libvirt was patched in commit bd6083c9b; we can add an extra measure of security by explicitly requesting that libvirtd's /tmp is not visible to arbitrary users. See https://bugzilla.redhat.com/782474
* daemon/libvirtd.service.in (Service): Enable PrivateTmp. --- daemon/libvirtd.service.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 8f2458a..cf68440 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -17,6 +17,7 @@ ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID # Override the maximum number of opened files #LimitNOFILE=2048 +PrivateTmp=true
Before acking this patch, you should bear in mind that some users (for better or worse) _like_ to have libvirtd interact with /tmp. For example, 'virsh screenshot domain /tmp/sreen.ppm' would store into the user's /tmp (the API uses a stream, with the client side piping the stream into the client's file system), while 'virsh dump domain /tmp/dom.dump' would store into libvirtd's /tmp. If the two are not the same /tmp, that could break particular use cases. I think that argues in part that we should be adding new API counterparts to anything that currently takes a filename, to give a stream alternative that works regardless of how libvirtd is mounted differently than the calling user (also good for remote connections, as 'virsh dump' over a remote connection is already quite sensitive to differences in file system layout between client and host). But maybe it means we should NAK this patch. Thoughts? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Feb 06, 2012 at 02:31:55PM -0700, Eric Blake wrote:
On 02/06/2012 02:15 PM, Eric Blake wrote:
The last intentional use of /tmp by libvirt was patched in commit bd6083c9b; we can add an extra measure of security by explicitly requesting that libvirtd's /tmp is not visible to arbitrary users. See https://bugzilla.redhat.com/782474
* daemon/libvirtd.service.in (Service): Enable PrivateTmp. --- daemon/libvirtd.service.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 8f2458a..cf68440 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -17,6 +17,7 @@ ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID # Override the maximum number of opened files #LimitNOFILE=2048 +PrivateTmp=true
Before acking this patch, you should bear in mind that some users (for better or worse) _like_ to have libvirtd interact with /tmp. For example, 'virsh screenshot domain /tmp/sreen.ppm' would store into the user's /tmp (the API uses a stream, with the client side piping the stream into the client's file system), while 'virsh dump domain /tmp/dom.dump' would store into libvirtd's /tmp. If the two are not the same /tmp, that could break particular use cases.
Actually the screenshot command is one of the well designed ones that would *not* have a problem. It uses streams, so the PATH is interpreted client side, not server side. The problem APIs are virDomainSave/Restore/CoreDump.
I think that argues in part that we should be adding new API counterparts to anything that currently takes a filename, to give a stream alternative that works regardless of how libvirtd is mounted differently than the calling user (also good for remote connections, as 'virsh dump' over a remote connection is already quite sensitive to differences in file system layout between client and host). But maybe it means we should NAK this patch.
We have the managed save APIs and snapshot APIs which avoid the problem of file paths that virDomainSave/Restore have, so I'm not too concerned about those. The virDomainCoreDump AP is the main one without a good solution. I think it would be desirable to have some "managed core dump" API which always saved dumps into a fixed location under /var/lib/libvirt/dumps (as our automatic core dump code already does). Then perhaps have some APIs to list/delete core dumps, and to download their contents. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Eric Blake