Bug 782474
Summary: | Consider using PrivateTmp=true in service file for libvirt | ||
---|---|---|---|
Product: | [Community] Virtualization Tools | Reporter: | Daniel Walsh <dwalsh> |
Component: | libvirt | Assignee: | Libvirt Maintainers <libvirt-maint> |
Status: | CLOSED WONTFIX | QA Contact: | |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | unspecified | CC: | berrange, clalancette, crobinso, dougsland, eblake, itamar, jforbes, laine, libvirt-maint, rbalakri, veillard, virt-maint |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2016-04-14 20:18:10 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 782466 |
Description
Daniel Walsh
2012-01-17 15:12:11 UTC
Any change on this bug. We are coming up to Feature Freeze, and would like some comment on this bug. If you do not believe this application uses /tmp than please comment on this and close the bug. If you believe this application needs to use /tmp to communicate with other applications or users then you can close this bug with that comment. If your app does not use systemd, then close this bug with that comment. If you have no idea, then please add a comment, and change the bug to assigned. I need to update the status on this feature. Thanks for your help. Libvirt should not be explicitly using /tmp for anything needed for communication with the user. The last known instance of using /tmp was patched in libvirt 0.9.8 by: commit bd6083c9baf1a8248b3b9fcde0a2c3f44e8e61ad Author: Eric Blake <eblake> Date: Wed Nov 9 10:23:49 2011 -0700 nwfilter: avoid failure with noexec /tmp If /tmp is mounted with the noexec flag (common on security-conscious systems), then nwfilter will fail to initialize, because we cannot run any temporary script via virRun("/tmp/script"); but we _can_ use "/bin/sh /tmp/script". For that matter, using /tmp risks collisions with other unrelated programs; we already have /var/run/libvirt as a dedicated temporary directory for use by libvirt. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesWriteToTempFile): Use internal directory, not /tmp; drop attempts to make script executable; and detect close error. (ebiptablesExecCLI): Switch to virCommand, and invoke the shell to read the script, rather than requiring an executable script. Supposing we want private temp enabled, what would the patch to the systemd service file look like? Eric, I think all the patch would be is adding PrivateTmp=true to the unit file. That said, I think enabling this feature would cause us pain, because I guarantee there are users out there storing disk images or serial logs or whatever in /tmp (for better or worse), and I'm pretty sure this feature would cause their guests to now fail to start. If we are gonna enable it, finding some way to indicate we have private tmp in an error message, or at least documenting the crap out of it would be important. (In reply to comment #3) > Eric, I think all the patch would be is adding PrivateTmp=true to the unit > file. > > That said, I think enabling this feature would cause us pain, because I > guarantee there are users out there storing disk images or serial logs or > whatever in /tmp (for better or worse), and I'm pretty sure this feature would > cause their guests to now fail to start. Indeed, commands such as 'virsh screenshot domain /tmp/file.bmp' would fail if the /tmp/file.bmp seen by libvirtd is different than the /tmp/file.bmp seen by the user issuing the virsh command. > > If we are gonna enable it, finding some way to indicate we have private tmp in > an error message, or at least documenting the crap out of it would be > important. Then let's make sure we discuss the ramifications on the upstream patch; if it gets rejected for reasons like you have mentioned, then we can reject this bug for libvirtd. (In reply to comment #5) > Indeed, commands such as 'virsh screenshot domain /tmp/file.bmp' would fail if > the /tmp/file.bmp seen by libvirtd is different than the /tmp/file.bmp seen by > the user issuing the virsh command. Actually, 'virsh screenshot' is a bad example - that sets up a stream, where the end file is created on the user side (virsh), not the server side (libvirtd). But we _do_ have libvirt API that pass filenames over the RPC call for libvirtd to open. Any API that can also pass streams instead of filenames is safe, but we don't have all of our APIs converted to streams yet (for example, virDomainCoreDump has no stream counterpart). I would say if this seems at all dangerous then put it off until you guys are ready. Yeah I thought that this would affect a qemu guest trying use a disk in /tmp but I guess I was wrong. In which case private tmp probably won't have too many realistic effects. While this wasn't outright rejected, I think there's work to be done in libvirt before we could enable this. Moving this to the upstream tracker I just tried this. This basically breaks any usage of disk images in /tmp, since libvirt wants to inspect them to determine their disk formats. While putting disk images in tmp might not be a recommended setup, I'm sure someone is doing it somewhere, and PrivateTmp would make it fail in a very non-obvious way. So this isn't something we could unconditionally enable IMO |