[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] move ebiptables script out of /tmp



On Thu, Jun 17, 2010 at 09:37:33AM -0400, Stefan Berger wrote:
> On 06/17/2010 06:20 AM, Daniel P. Berrange wrote:
> >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.
> >   
> I had tried to get rid of the script creation a while ago, but couldn't 
> get rid of all of them, particularly those that create the 'anchors' for 
> the iptables are a bit more complicated where I run tests whether the 
> necessary rules already exists and only if they don't exist run a 
> certain set of iptables commands to create user-defined tables into 
> which libvirt then builds the rules pertaining to individual VMs.
> 
> Also, the whole XML filter tree is first translated into iptables rules 
> to detect errors there (references to filters that don't exist) and then 
> run, rather than executing the rules individually while translating a 
> single XML rule. Further, some of the rules may fail, particularly those 
> that do the tear-down, but don't cause the script to abort.

Yep, any replacement solution would have to allow for commands to
fail. The virCommand APIs[1] provide a convenient way to store
a command + set of args in memory. I'm thinking we'd introduce a
second object virCommandSet which stored

   struct virCommandEntry {
      virCommand cmd;
      bool stopOnError;
   };
   struct virCommandSet {
      size_t ncmds;
      virCommandEntry cmds;
   }


So the filter code would populate a virCommandSet object completely,
allowing for validation of the full XML filter tree. Then it would
invoke an execute operation against the virCommandSet to run the 
whole thing. If we wanted to be really clever, then we could make
the virCommandEntry contain 2 virCommand objects, one normal command
to run, and one 'rollback' command to undo its effects. If a
virCommandSet failed part way through, then we can just iterate
backwards over the entries executing any rollback commands.

This will have to wait till I finish the basic virCommand bits
though.

Regards,
Daniel

[1] https://www.redhat.com/archives/libvir-list/2010-May/msg00991.html
-- 
|: 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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]