Bug 638633

virsh shouldn't check for files existing, because it is executing in a different context to QEMU. libvirt ought not todo the checks either, since the check can succeed for libvirt, while the file is still not accessible to QEMU (eg due to selinux labelling).  QEMU needs to validate the script when a NIC is configured on the command line with -netdev, or netdev_add in the monitor. If QEMU reports an error in these scenarios libvirt will feed it back. I suspect QEMU isnt' checking for the script existing correctly.

Changed component to qemu-kvm per comment 3.

Simple testing with qemu works as expected:

 { "execute": "netdev_add", "arguments": { "type": "tap", "id": "net0", "script": "./qemu-if" } }
 {"error": {"class": "DeviceInitFailed", "desc": "Device 'tap' could not be initialized", "data": {"device": "tap"}}}

Also tested command-line and the human monitor. Either, there's something else going on (like, some additional option that would make netdev_add skip the check) or libvirt is not really checking if netdev_add succeeded.

I'm going to try the scenario described in the bz description, but would appreciate if anyone from libivrt took a look too.

I did:

 virsh # attach-interface f13-test bridge eth1 --script xyz
 Interface attached successfully

 virsh # 

 (Yes "eth1" is a bridge).

And logged the following command exchange between libvirt and qemu:

-> {"execute": "getfd", "arguments": {"fdname": "fd-net1"}}
<- {"return": {}}
-> {"execute": "netdev_add", "arguments": {"fd": "fd-net1", "type": "tap", "id": "hostnet1"}}
<- {"return": {}}
-> {"execute": "device_add", "arguments": {"bus": "pci.0", "netdev": "hostnet1", "addr": "0x5", "driver": "rtl8139", "mac": "52:54:00:83:24:eb", "id": "net1"}}
<- {"return": {}}

As we can see, the "script" argument is not passed to the netdev_add command, so there's nothing qemu can do about the not existing file.

I also tested with an existing script and could confirm that the script is not executed. I'm under the impression that libvirt is ignoring the --script argument altogether.

Changing component back to libvirt.

Thanks, Luiz

From examining the code, it looks like interface scripts for interfaces of type='bridge' have only ever worked for the xen hypervisor. For qemu domains, we only send the script parameter if the interface type is "ethernet" (see http://www.libvirt.org/formatdomain.html#elementsNICSEthernet). I'm not sure of the reasoning behind this, since the writing of that code precedes my involvement by "awhile".

Since libvirt ignores "extra unrecognized" xml attributes and elements, no error is produced if a script is specified when interface type='bridge', but nothing is done with it.

Do we want to start supporting connect-time scripts for interfaces of types other than "ethernet"? If so, which types of interfaces?

No, we don't want to support the 'script' argument for anything other than 'ethernet' type. Running external shell scripts is very undesirable from a security POV, and we block their ability todo anything useful with networking unless QEMU runs as root with full capabilities and no SELinux.

That's being the case, we need to explicitly reject the attempt to specify a script.

(In reply to comment #14)
> That's being the case, we need to explicitly reject the attempt to specify a
> script.

And what if someone gives:

   <interface type='network'>
      <script path='blah'/>
       ...
   </interface>

I imagine that should produce an error log too?

Then what about:

   <interface type='network'>
      <script name='blah'/>
       ...
   </interface>

(or similar, with "type='ethernet'"), or:

   <interface type='ethernet' script='blah'>
       ...
   </interface>

Should those both produce errors?

In that case, what about:

   <interface type='network'>
      <fiddle faddle='free'/>
       ...
   </interface>

?

It's not clear to me where the line should be drawn? (Sure, we could make a patch that would log an error in the specific case presented by this bug report, but that would likely be followed by another that had one of the variations given above (or some other permutation/extrapolation).

I've sent mail to libvir-list on this subject:

https://www.redhat.com/archives/libvir-list/2011-August/msg00660.html

Also, in terms of the virsh command - it should be a direct conduit to the API, so should be totally agnostic to --script arg vs. interface type vs. hypervisor.

Here's a description of one way to assure that improper usage of <script> is always reported:

https://www.redhat.com/archives/libvir-list/2011-August/msg00933.html

Reproduce this issue with:
libvirt-client-0.9.4-23.el6.x86_64
libvirt-0.9.4-23.el6.x86_64
libvirt-python-0.9.4-23.el6.x86_64
qemu-kvm-0.12.1.2-2.209.el6.x86_64

# virsh attach-interface test1 bridge virbr1 --script xyp
Interface attached successfully

#virsh dumpxml test1
...
  <interface type='bridge'>
      <mac address='52:54:00:7f:a6:67'/>
      <source bridge='virbr1'/>
      <script path='xyp'/>
      <target dev='vnet1'/>
      <alias name='net1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
    </interface>
...
# ll xyp
ls: cannot access xyp: No such file or directory

I posted a patch upstream to fix this problem. Now we'll see if the approach I used is accepted by the other upstream developers:

  https://www.redhat.com/archives/libvir-list/2012-January/msg00240.html

The above patch was ACKed and is now pushed upstream. It will be in the next rebase of libvrit for RHEL6.

# rpm -q libvirt
libvirt-0.9.10-1.el6.x86_64

# virsh attach-interface rhel6u2 --type bridge virbr0 --script xyz
error: Failed to attach interface
error: unsupported configuration: scripts are not supported on interfaces of type bridge

# virsh attach-interface rhel6u2 --type ethernet virbr0 --script xyz
error: No support for ethernet in command 'attach-interface'

The problem is fixed in this bug, but the script will never work for both type ethernet and bridge will not work on kvm.

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
libvirt and virsh would previously ignore any "script file" given in the specification for a network interface of a type that didn't actually use script files. To avoid confusion, it now explicitly prohibits this, logs an error, and fails the operation when any attempt is made to specify a script file for an interface type that doesn't support script files (for example, with Xen guests, it is legal to specify a script file for interfaces of type='bridge', but for qemu guests this is not allowed (mainly because it is not necessary, and allowing it would be an opportunity for a security hole)).

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2012-0748.html