Re: [libvirt] [BUG,RFC] directory traversal vulnerability / qemu: name→uuid

On 12 September 2011 23:10, Daniel P. Berrange <berrange(a)redhat.com&gt; 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?