[libvirt] [BUG,RFC] directory traversal vulnerability / qemu: name→uuid
Hello, I just tried the following command with libvirt-0.9.5git: # virsh snapshot-create "$VM" /dev/stdin <<<'<domainsnapshot><name>../../../../../../etc/passwd</name></domainsnapshot>' "Luckily" it adds a .xml suffix, but this still looks like a security problem to me, because you can overwrite any .xml-file with libvirt gibberish. Actually this was found by a user trying to create a snapshot with an embedded /, which didn't work, because the sub-directory didn't exist. I know SELinux can solve this, but I really would prefer the Qemu driver to reject such names. Another problem is, that I sometimes would like to rename a VM to a new name, because the old name doesn't describe the VM good enough. <description> is not an option, because 1) Xen doesn't store it, and 2) virsh list doesn't show it. Renaming a Qemu-VM is currently impossible, since the name of the VM is used for several files and directories and a undefine+define would loose state: /etc/libvirt/qemu/$VM.xml /var/lib/libvirt/qemu/$VM.monitor /var/lib/libvirt/qemu/save/$VM.save /var/lib/libvirt/qemu/snapshot/$VM/$SNAPSHOT.xml (Renaming outside of libvirtd can be done by hand, but requires a restart of libvirtd to get it to reload it's state.) Compared to Xen and VirtualBox (as far as I know) they both use the UUID to name their files and directroy, which looks a lot more sane to me than using the name of the VM. Would it be possible and feasible to convert the Qemu driver to use the UUID instead for file and directory naming? Sincerely Philipp -- Philipp Hahn Open Source Software Engineer hahn@univention.de Univention GmbH Linux for Your Business fon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/ ---------------------------------------------------------------------------- Treffen Sie Univention auf der IT&Business vom 20. bis 22. September 2011 auf dem Gemeinschaftsstand der Open Source Business Alliance in Stuttgart in Halle 3 Stand 3D27-7.
On 09/07/2011 11:12 AM, Philipp Hahn wrote:
Hello,
I just tried the following command with libvirt-0.9.5git: # virsh snapshot-create "$VM" /dev/stdin <<<'<domainsnapshot><name>../../../../../../etc/passwd</name></domainsnapshot>'
"Luckily" it adds a .xml suffix, but this still looks like a security problem to me, because you can overwrite any .xml-file with libvirt gibberish. Actually this was found by a user trying to create a snapshot with an embedded /, which didn't work, because the sub-directory didn't exist. I know SELinux can solve this, but I really would prefer the Qemu driver to reject such names.
Qemu won't reject names with /, but I agree with your thought that libvirt needs to prevent such names, particularly since it creates several other file names (such as log files, managed save, snapshots, and even the monitor file) all based on the domain name.
Another problem is, that I sometimes would like to rename a VM to a new name, because the old name doesn't describe the VM good enough.<description> is not an option, because 1) Xen doesn't store it, and 2) virsh list doesn't show it.
Adding a virDomainRename command would indeed be a nice API addition, but it certainly involves quite a bit of work.
Renaming a Qemu-VM is currently impossible, since the name of the VM is used for several files and directories and a undefine+define would loose state: /etc/libvirt/qemu/$VM.xml /var/lib/libvirt/qemu/$VM.monitor /var/lib/libvirt/qemu/save/$VM.save /var/lib/libvirt/qemu/snapshot/$VM/$SNAPSHOT.xml
All of these files would have to be edited as part of a virDomainRename. You are also missing: /var/log/libvirt/qemu/$VM.log
Would it be possible and feasible to convert the Qemu driver to use the UUID instead for file and directory naming?
Maybe, but I prefer seeing files by name rather than by UUID when browsing through the libvirt internal directories. If we supported renaming, and properly altered the name of all affected files, then I see no reason to keep the files by name instead of uuid. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Hello Eric, On Wednesday 07 September 2011 16:02:51 Eric Blake wrote:
On 09/07/2011 11:12 AM, Philipp Hahn wrote:
I just tried the following command with libvirt-0.9.5git: # virsh snapshot-create "$VM" /dev/stdin <<<'<domainsnapshot><name>../../../../../../etc/passwd</name></domainsnap shot>'
"Luckily" it adds a .xml suffix, but this still looks like a security problem to me, because you can overwrite any .xml-file with libvirt gibberish. Actually this was found by a user trying to create a snapshot with an embedded /, which didn't work, because the sub-directory didn't exist. I know SELinux can solve this, but I really would prefer the Qemu driver to reject such names.
Qemu won't reject names with /, but I agree with your thought that libvirt needs to prevent such names, particularly since it creates several other file names (such as log files, managed save, snapshots, and even the monitor file) all based on the domain name.
For Qemu the name is just a C-string, but libvirt make the error to use those bits as something else, namely a UNIX/Windows/whatever path name, which has additional constraints. So if libvirt wants to use the name as a path, it must add an additional constraint on the naming to make it safe, or at lease use some escaping when translating the name to a path name.
You are also missing: /var/log/libvirt/qemu/$VM.log
Yes, which is compilcated by logrotate replacing and renaming those files.
Would it be possible and feasible to convert the Qemu driver to use the UUID instead for file and directory naming?
Maybe, but I prefer seeing files by name rather than by UUID when browsing through the libvirt internal directories. If we supported renaming, and properly altered the name of all affected files, then I see no reason to keep the files by name instead of uuid.
Yes, names are definitly nicer than UUIDs, but they make renaming harder (I hope nobody want's to change the UUID) and have the meta-character problem. With UUID we are sure, that they always consists of safe characters and have a finit length. Sincerely Philipp -- Philipp Hahn Open Source Software Engineer hahn@univention.de Univention GmbH Linux for Your Business fon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/ ---------------------------------------------------------------------------- Treffen Sie Univention auf der IT&Business vom 20. bis 22. September 2011 auf dem Gemeinschaftsstand der Open Source Business Alliance in Stuttgart in Halle 3 Stand 3D27-7.
On Wed, Sep 07, 2011 at 03:02:51PM +0100, Eric Blake wrote:
On 09/07/2011 11:12 AM, Philipp Hahn wrote:
Hello,
I just tried the following command with libvirt-0.9.5git: # virsh snapshot-create "$VM" /dev/stdin <<<'<domainsnapshot><name>../../../../../../etc/passwd</name></domainsnapshot>'
"Luckily" it adds a .xml suffix, but this still looks like a security problem to me, because you can overwrite any .xml-file with libvirt gibberish. Actually this was found by a user trying to create a snapshot with an embedded /, which didn't work, because the sub-directory didn't exist. I know SELinux can solve this, but I really would prefer the Qemu driver to reject such names.
Qemu won't reject names with /, but I agree with your thought that libvirt needs to prevent such names, particularly since it creates several other file names (such as log files, managed save, snapshots, and even the monitor file) all based on the domain name.
Another problem is, that I sometimes would like to rename a VM to a new name, because the old name doesn't describe the VM good enough.<description> is not an option, because 1) Xen doesn't store it, and 2) virsh list doesn't show it.
Adding a virDomainRename command would indeed be a nice API addition, but it certainly involves quite a bit of work.
Renaming a Qemu-VM is currently impossible, since the name of the VM is used for several files and directories and a undefine+define would loose state: /etc/libvirt/qemu/$VM.xml /var/lib/libvirt/qemu/$VM.monitor /var/lib/libvirt/qemu/save/$VM.save /var/lib/libvirt/qemu/snapshot/$VM/$SNAPSHOT.xml
All of these files would have to be edited as part of a virDomainRename. You are also missing:
/var/log/libvirt/qemu/$VM.log
Would it be possible and feasible to convert the Qemu driver to use the UUID instead for file and directory naming?
Maybe, but I prefer seeing files by name rather than by UUID when browsing through the libvirt internal directories. If we supported renaming, and properly altered the name of all affected files, then I see no reason to keep the files by name instead of uuid.
I really don't like the idea of using UUID for files we store on disk because it makes it really unpleasant when debugging / troubleshooting. Clearly we should forbid '/' in any guest name though. In addition libvirt code should not be using the shell when running commands, so we avoid the shell meta-charcter problem already. Note, that this isn't a serious security flaw at this time, since access to a privileged libvirtd daemon is already effectively equivalent to having a root shell. Only once we get RBAC controls would this kind of thing be able to be used for privilege escalation / DOS. 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 :|
On 12 September 2011 23:10, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Sep 07, 2011 at 03:02:51PM +0100, Eric Blake wrote:
On 09/07/2011 11:12 AM, Philipp Hahn wrote:
Hello,
I just tried the following command with libvirt-0.9.5git: # virsh snapshot-create "$VM" /dev/stdin <<<'<domainsnapshot><name>../../../../../../etc/passwd</name></domainsnapshot>'
"Luckily" it adds a .xml suffix, but this still looks like a security problem to me, because you can overwrite any .xml-file with libvirt gibberish. Actually this was found by a user trying to create a snapshot with an embedded /, which didn't work, because the sub-directory didn't exist. I know SELinux can solve this, but I really would prefer the Qemu driver to reject such names.
Qemu won't reject names with /, but I agree with your thought that libvirt needs to prevent such names, particularly since it creates several other file names (such as log files, managed save, snapshots, and even the monitor file) all based on the domain name.
Another problem is, that I sometimes would like to rename a VM to a new name, because the old name doesn't describe the VM good enough.<description> is not an option, because 1) Xen doesn't store it, and 2) virsh list doesn't show it.
Adding a virDomainRename command would indeed be a nice API addition, but it certainly involves quite a bit of work.
Renaming a Qemu-VM is currently impossible, since the name of the VM is used for several files and directories and a undefine+define would loose state: /etc/libvirt/qemu/$VM.xml /var/lib/libvirt/qemu/$VM.monitor /var/lib/libvirt/qemu/save/$VM.save /var/lib/libvirt/qemu/snapshot/$VM/$SNAPSHOT.xml
All of these files would have to be edited as part of a virDomainRename. You are also missing:
/var/log/libvirt/qemu/$VM.log
Would it be possible and feasible to convert the Qemu driver to use the UUID instead for file and directory naming?
Maybe, but I prefer seeing files by name rather than by UUID when browsing through the libvirt internal directories. If we supported renaming, and properly altered the name of all affected files, then I see no reason to keep the files by name instead of uuid.
I really don't like the idea of using UUID for files we store on disk because it makes it really unpleasant when debugging / troubleshooting.
Clearly we should forbid '/' in any guest name though. In addition libvirt code should not be using the shell when running commands, so we avoid the shell meta-charcter problem already.
Note, that this isn't a serious security flaw at this time, since access to a privileged libvirtd daemon is already effectively equivalent to having a root shell. Only once we get RBAC controls would this kind of thing be able to be used for privilege escalation / DOS.
"Access to a privileged libvirtd daemon is already effectively equivalent to having a root shell" --> why is this?
On Tue, Sep 13, 2011 at 12:43:07AM +1000, dave bl wrote:
On 12 September 2011 23:10, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Sep 07, 2011 at 03:02:51PM +0100, Eric Blake wrote:
On 09/07/2011 11:12 AM, Philipp Hahn wrote:
Hello,
I just tried the following command with libvirt-0.9.5git: # virsh snapshot-create "$VM" /dev/stdin <<<'<domainsnapshot><name>../../../../../../etc/passwd</name></domainsnapshot>'
"Luckily" it adds a .xml suffix, but this still looks like a security problem to me, because you can overwrite any .xml-file with libvirt gibberish. Actually this was found by a user trying to create a snapshot with an embedded /, which didn't work, because the sub-directory didn't exist. I know SELinux can solve this, but I really would prefer the Qemu driver to reject such names.
Qemu won't reject names with /, but I agree with your thought that libvirt needs to prevent such names, particularly since it creates several other file names (such as log files, managed save, snapshots, and even the monitor file) all based on the domain name.
Another problem is, that I sometimes would like to rename a VM to a new name, because the old name doesn't describe the VM good enough.<description> is not an option, because 1) Xen doesn't store it, and 2) virsh list doesn't show it.
Adding a virDomainRename command would indeed be a nice API addition, but it certainly involves quite a bit of work.
Renaming a Qemu-VM is currently impossible, since the name of the VM is used for several files and directories and a undefine+define would loose state: /etc/libvirt/qemu/$VM.xml /var/lib/libvirt/qemu/$VM.monitor /var/lib/libvirt/qemu/save/$VM.save /var/lib/libvirt/qemu/snapshot/$VM/$SNAPSHOT.xml
All of these files would have to be edited as part of a virDomainRename. You are also missing:
/var/log/libvirt/qemu/$VM.log
Would it be possible and feasible to convert the Qemu driver to use the UUID instead for file and directory naming?
Maybe, but I prefer seeing files by name rather than by UUID when browsing through the libvirt internal directories. If we supported renaming, and properly altered the name of all affected files, then I see no reason to keep the files by name instead of uuid.
I really don't like the idea of using UUID for files we store on disk because it makes it really unpleasant when debugging / troubleshooting.
Clearly we should forbid '/' in any guest name though. In addition libvirt code should not be using the shell when running commands, so we avoid the shell meta-charcter problem already.
Note, that this isn't a serious security flaw at this time, since access to a privileged libvirtd daemon is already effectively equivalent to having a root shell. Only once we get RBAC controls would this kind of thing be able to be used for privilege escalation / DOS.
"Access to a privileged libvirtd daemon is already effectively equivalent to having a root shell" --> why is this?
Consider your host OS root filesystem is on a disk /dev/hda. You can start a guest, adding in an extra disk <disk type='block'> <source dev='/dev/hda'/> <target dev='/dev/vdb'/> </disk> And now when booted your guest will see the host's /dev/hda block device appearing as /dev/vdb in the guest, at which point it can mess with the host's filesytem. The current libvirt security model is *not* attempting to protect the host OS [1], from a malicious host OS administrator. It only attempts to protect the host OS from a malicious guest OS. The RBAC stuff I mention above is a plan to add protection against malicious host OS admins. https://www.redhat.com/archives/libvir-list/2011-June/msg00244.html Regards, Daniel [1] Technically this isn't quite true. If you run libvirt on a host with the SELinux MLS policy, then there is some level of protection, but no one really uses that in the real world, except for perhaps some super paranoid government agencies -- |: 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 (4)
-
Daniel P. Berrange -
dave bl -
Eric Blake -
Philipp Hahn