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

Stefan Berger stefanb at linux.vnet.ibm.com
Thu Jun 17 13:37:33 UTC 2010


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. Others may 
not fail and would cause the abort and the cleanup of the rules 
afterwards. So, what previously was a construct like this here that 
checked the return code

cmd='iptables ...'
${cmd}
if [ $? -ne 0 ]; then
     echo "Error: command ${cmd} failed."
     exit 1;
fi

became something like this here:

iptables ...
STOPONERROR

where STOPONERROR was basically emulating the check of the iptables 
status code above. So it was processing a command like iptables, split 
its parameters into an array for passing to the libvirt api, then 
checked whether the next command was a STOPONERROR and took appropriate 
action if the status code was != 0.

The patch from a while ago is attached.

    Stefan

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: less_batch_script.diff
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100617/ae5b52ba/attachment-0001.ksh>


More information about the libvir-list mailing list