On Wed, Nov 09, 2011 at 11:01:58AM -0700, Eric Blake wrote:
This series does not solve a more fundamental issue that has been on my mind - are we still sure that the best design for manipulating network filters involves the creation of a series of shell scripting commands, where we have to worry about proper quoting and so forth? Is it possible to refactor this code to make more direct use of virCommand for every call to iptables and friends (that is, doing the glue logic in C rather than using C to generate shell scripting commands where the glue logic is in generated sh)? Or perhaps to even refactor things into a well-defined file format that we can feed to a helper executable, which would allow finer-grained SELinux labelling (by isolating the direct execution of iptables into a well-defined helper executable, then SELinux can enforce that libvirtd cannot alter the host firewall except by going through the helper executable, which has been audited to make only known sets of iptables calls based on well-formed input)?
I think virCommand is a little too low level to do what nwfilter wants here, but we can definitely build a higher level API around it to replace the shell scripting. I has been a while since I looked at the generated scripts, IIUC, the shell scripts generated are basically defining groups of iptables commands to make "interesting" changes, and periodically a "rollback" command which is run if something fails. So I was thinking you could design a libvirt level API that would be used something like: virCommandScriptPtr script = virCommandScriptNew(); virCommandScriptBeginGroup(script); virCommandScriptAddCommandArgList(script, "/some/command", args.....); virCommandScriptAddRollbackArgList(script, "/some/command", args...); virCommandScriptBeginGroup(script); virCommandScriptAddCommandArgList(script, "/some/command", args.....); virCommandScriptAddCommandArgList(script, "/some/command", args.....); virCommandScriptAddCommandArgList(script, "/some/command", args.....); ...repeat... virCommandScriptAddRollbackArgList(script, "/some/command", args...); virCommandScriptRun(); If one of the commands following the 'BeginGroup' failed, then it would execute the Rollback command for that group, and for every preceeding group. Personally I would dearly love to kill off this use of shell scripting in libvirt, because I fear it is only a matter of time before we introduce some flaw wrt escaping. In general I'd like to be able to say that libvirt *never* runs shell commands, always uses execve() directly. 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 :|