On Wed, Jun 16, 2010 at 03:57:50PM -0500, Jamie Strandboge wrote:
On Wed, 2010-06-16 at 14:04 -0600, Eric Blake wrote:
> I'm with Stefan - the whole point of mkstemp is that the created file
> has 0600 permissions, and /tmp is restricted-deletion, so no other user
> can either overwrite the file contents or unlink it and replace it with
> an alternate file. Then again, gnulib documents that glibc 2.0.7 or
> older would create a file with group/other permissions if the umask
> wasn't set prior to the mkstemp() call, and gnulib's mkstemp() does not
> work around this issue; but that's a rather old version of glibc to be
> worrying about.
This has nothing to do with mkstemp(). As I said, libvirt's use of it is
fine and there is no symlink race or security vulnerability by itself.
The issue is that use of /tmp is not required *and* it becomes difficult
to properly confine libvirtd via a MAC if you must allow execution of
files in /tmp. See my answer to Stefan's question for an example
scenario.
ACK to your patch but really this whole set of code needs to be shot.
We simply should not be writing shell scripts and then executing them,
when we have perfectly capable APIs for directly executing commands.
Using the shell adds no value here IMHO. I will be rewriting this code
when I finish the virCommandPtr code that I proposed a few weeks back
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://deltacloud.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|