Bug 638633 - [RHEL6-Beta] 'virsh attach-interface' succeeds even if a nonexistent script file is specified to the option --script.
Summary: [RHEL6-Beta] 'virsh attach-interface' succeeds even if a nonexistent script f...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt
Version: 6.0
Hardware: All
OS: Linux
low
medium
Target Milestone: rc
: ---
Assignee: Laine Stump
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks: GSS_6_2_PROPOSED 747123 727267 747667 756082
TreeView+ depends on / blocked
 
Reported: 2010-09-29 14:37 UTC by Sadique Puthen
Modified: 2018-11-27 21:56 UTC (History)
15 users (show)

Fixed In Version: libvirt-0.9.10-1.el6
Doc Type: Bug Fix
Doc Text:
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)).
Clone Of:
Environment:
Last Closed: 2012-06-20 06:25:17 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2012:0748 normal SHIPPED_LIVE Low: libvirt security, bug fix, and enhancement update 2012-06-19 19:31:38 UTC

Description Sadique Puthen 2010-09-29 14:37:56 UTC
Description of problem:

"virsh attach-interface" succeeds even if a nonexistent script file (e.g. xyz) is specified to the option --script.

# virsh attach-interface vm1 bridge br0 --script xyz
Interface attached successfully

It should report an error message like "script not found" then. 

Version-Release number of selected component (if applicable):

Red Hat Enterprise Linux Version Number: 6
Release Number: snapshot13
Architecture: x86_64
Kernel Version: kernel-2.6.32-70.el6.x86_64
Related Package Version: libvirt-client-0.8.1-27.el6.x86_64 

How reproducible:

Always

Steps to Reproduce:

 1. Just run the following command.
# virsh attach-interface <domain-name> bridge <source-device> --script xyz
<Note the script "xyz" doesn't exist.> 

virsh dumpxml shows the interface has been added.

   <interface type='bridge'>
      <mac address='52:54:00:18:d6:66'/>
      <source bridge='br0'/>
      <script path='xyz'/>
      <target dev='vnet1'/>
      <alias name='net1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
    </interface>
 
Actual results:

If a non-existent script path is given, virsh is able to attach the nic.

Expected results:

If a non-existent script path is given, virsh should fail to attach the nic.

Additional info:

Comment 3 Daniel Berrangé 2011-02-17 16:38:00 UTC
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.

Comment 6 Dave Allan 2011-04-19 14:52:18 UTC
Changed component to qemu-kvm per comment 3.

Comment 8 Luiz Capitulino 2011-06-17 14:38:33 UTC
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.

Comment 9 Luiz Capitulino 2011-06-17 17:30:44 UTC
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.

Comment 10 Dave Allan 2011-06-20 18:31:12 UTC
Thanks, Luiz

Comment 12 Laine Stump 2011-08-15 07:22:51 UTC
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?

Comment 13 Daniel Berrangé 2011-08-15 16:10:12 UTC
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.

Comment 14 Dave Allan 2011-08-15 18:27:57 UTC
That's being the case, we need to explicitly reject the attempt to specify a script.

Comment 15 Laine Stump 2011-08-16 08:47:12 UTC
(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

Comment 16 Laine Stump 2011-08-16 08:50:35 UTC
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.

Comment 17 Laine Stump 2011-08-22 09:58:06 UTC
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

Comment 22 yuping zhang 2011-11-29 07:49:36 UTC
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

Comment 27 Laine Stump 2012-01-06 20:55:33 UTC
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

Comment 28 Laine Stump 2012-01-08 16:13:22 UTC
The above patch was ACKed and is now pushed upstream. It will be in the next rebase of libvrit for RHEL6.

Comment 33 Wayne Sun 2012-02-15 03:27:39 UTC
# 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.

Comment 34 Laine Stump 2012-05-08 19:21:49 UTC
    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)).

Comment 36 errata-xmlrpc 2012-06-20 06:25:17 UTC
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


Note You need to log in before you can comment on or make changes to this bug.