On Thu, Apr 15, 2010 at 02:35:41PM +0200, Gerhard Stenzel wrote: > The following patch mainly adds a set of test case to verify that > several spoofing attacks are prevented by the nwfilter subsystem. > > In order to have a well defined test machine, the patch also includes > test scripts to network install a virtual disk from scratch, to boot the > virtual test machine prior to running the actual test scripts and to > shut it down afterwards. > > While I have tried to remove as much dependency on my local setup as > possible there is still some left, so I am currently more interested in > feedback about the general approach, not necessarily actual inclusion > into the libvirt-TCK git. Your actual test cases look good, so I'll just put comments about the setup/teardown stuff inline. > > For example, I am currently trying to find a suitable location for the > kickstart file, and also a suitable place for the common_functions.pl. The 'lib' directory contains modules which provide common functions & code for the test scripts. In this case I'd suggest creating a file lib/Sys/Virt/TCK/NetworkHelpers.pm (use Sys::Virt::TCK::NetworkHelpers) > Index: libvirt-tck/scripts/network/README > =================================================================== > --- /dev/null > +++ libvirt-tck/scripts/network/README > @@ -0,0 +1,14 @@ > + > +Test cases: > + > +000-install-image.t creates and install a 2GB fedora virtual disk via > kickstart file from the network > +001-boot-image.t defines and boots a VM which uses the fedora virtual > disk > +100-ping-still-working.t verifies the VM is pingable > +210-no-mac-spoofing.t verifies mac spoofing is prevented > +220-no-ip-spoofing.t verifies ip spoofing is prevented > +230-no-mac-broadcast.t verifies mac broadcasting is prevented > +240-no-arp-spoofing.t verifies arp spoofing is prevented > +999-shutdown-image.t shuts the VM down One thing about the TCK test cases is that each one should be self-contained, doing all setup & teardown it requires, not reliant on any of the other tests cases or ordering of tests. So instead of having the 000-install-image.t & 0001-boot-image.t scripts that do setup, you'd want to create some library code that can be used to install + boot the guest, and just call that from each test case. > Index: libvirt-tck/scripts/network/000-install-image.t > =================================================================== > --- /dev/null > +++ libvirt-tck/scripts/network/000-install-image.t > @@ -0,0 +1,181 @@ > +# -*- perl -*- > +# > +# Copyright (C) 2010 IBM Corp. > +# > +# This program is free software; You can redistribute it and/or modify > +# it under the GNU General Public License as published by the Free > +# Software Foundation; either version 2, or (at your option) any > +# later version > +# > +# The file "LICENSE" distributed along with this file provides full > +# details of the terms and conditions > +# > + > +=pod > + > +=head1 NAME > + > +network/000-install-image.t - install network test image > + > +=head1 DESCRIPTION > + > +The test case creates and install a 2GB fedora virtual > +disk via kickstart file from the network. > + > +=cut > + > +use strict; > +use warnings; > + > +use Test::More tests => 1; > + > +use Sys::Virt::TCK; > + > +my $tck = Sys::Virt::TCK->new(); > +my $conn = eval { $tck->setup(); }; > +BAIL_OUT "failed to setup test harness: $@" if $@; > +END { $tck->cleanup if $tck; } > + > +# variables which may need to be adapted > +my $domain_name ="f12nwtest"; > +my $disk_name = "/var/lib/libvirt/images/${domain_name}.img"; > +my $disk_size = "2147483648"; You want to avoid hardcoding any disk paths in the test cases. There's a convenient helper function if you want to create a scratch disk for a guest my $path = $tck->create_sparse_disk("nwfilter", "root.img", 2147483648); > +my $kickstart_file ="http://192.168.122.1/ks.cfg"; > +my $cmdline = "ip=dhcp gateway=192.168.122.1 ks=${kickstart_file}"; In the 'conf/default.cfg' file we list the kernel+initrd images for various distros. We could likely also have an optional kickstart file listed there, so we move this configuration info out of the test cases. > +# see if the domain already exits > +my $already_defined = 0; > +diag "searching if ${domain_name} is already defined"; > +my $nnames = $conn->num_of_defined_domains(); > +my @names = $conn->list_defined_domain_names($nnames); > +foreach (@names){ > + if (/${domain_name}/) { > + print "$_ already exists, no need to redefine\n"; > + $already_defined = 1; > + } > +} > +diag $already_defined; All guests created by the TCK test cases should have a name prefix of 'tck' to avoid clashing with existing guests. Each test case should destroy + undefine all guests it creates. The TCK setup code will validate that there are no guests having a name starting with a 'tck' prefix & optionally destroy them. So each of your test cases should assume the guest doesn't already exist + clean it up before exiting. NB, the disk files are not deleted between runs, so you will still avoid the overhead of installation. I'm talking with the libguestfs folks to see if we can re-use their guest appliance + daemon so we can avoid the actual install + ssh stuff at some point in the future. > +# check for installation disk and build it if not exists > +my $already_installed = 0; > +my $pool = $conn->get_storage_pool_by_name("default"); > +my $nnames = $pool->num_of_storage_volumes(); > +my @volNames = $pool->list_storage_vol_names($nnames); > +foreach (@volNames){ > + if (/${domain_name}/) { > + print "$_ already exists, no need to install\n"; > + $already_installed = 1; > + } > +} > + > +my $volumexml = "<volume>". > +" <name>${domain_name}.img</name>". > +" <key>${disk_name}</key>". > +" <source>". > +" </source>". > +" <capacity>${disk_size}</capacity>". > +" <allocation>4096</allocation>". > +" <target>". > +" <path>${disk_name}</path>". > +" <format type='raw'/>". > +" <permissions>". > +" <mode>0644</mode>". > +" <owner>0</owner>". > +" <group>0</group>". > +" </permissions>". > +" </target>". > +"</volume>"; If using the create_sparse_disk helper I mentioned, you can avoid this chunk of code. Just do a check to see if $path exists or not. > +# prepare image > +if ($already_installed == 0) { > + diag "Creating ${disk_name}"; > + diag $volumexml; > + my $vol = $pool->create_volume($volumexml) > +# system("qemu-img create ${disk_name} ${disk_size}"); > +} > + > +my $topxml = " <name>${domain_name}</name>". > +" <memory>524288</memory>". > +" <currentMemory>524288</currentMemory>". > +" <vcpu>1</vcpu>"; > + > +my $osxml = " <os>". > +" <type arch='x86_64' machine='fedora-13'>hvm</type>". > +" <kernel>/var/cache/libvirt-tck/os-i686-hvm/vmlinuz</kernel>". > +" <initrd>/var/cache/libvirt-tck/os-i686-hvm/initrd</initrd>". > +" <cmdline>${cmdline}</cmdline>". > +" <boot dev='hd'/>". > +" </os>"; > + > +my $bottomxml = " <features>". > +" <acpi/>". > +" <apic/>". > +" </features>". > +" <clock offset='utc'/>". > +" <on_poweroff>destroy</on_poweroff>". > +" <on_reboot>restart</on_reboot>". > +" <on_crash>restart</on_crash>". > +" <devices>". > +" <emulator>/usr/bin/qemu-kvm</emulator>". > +" <disk type='file' device='disk'>". > +" <driver name='qemu' type='raw'/>". > +" <source file='${disk_name}'/>". > +" <target dev='hda' bus='ide'/>". > +" </disk>". > +" <controller type='ide' index='0'>". > +" </controller>". > +" <interface type='network'>". > +" <source network='default'/>". > +" <target dev='vnet0'/>". > +" <model type='virtio'/>". > +" </interface>". > +" <serial type='pty'>". > +" <target port='0'/>". > +" </serial>". > +" <console type='pty'>". > +" <target port='0'/>". > +" </console>". > +" <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' > keymap='de'/>". > +" <video>". > +" <model type='cirrus' vram='9216' heads='1'/>". > +" </video>". > +" </devices>"; > + > +my $xml = "<domain type='kvm'>" . > +$topxml. > +$osxml. > +$bottomxml. > +"</domain>"; There are a couple of helpers for building XML in a portable manner. In the Sys::Virt::TCK class there's a method 'generic_domain' which will find you a bare minimum VM matching the best kernel / arch found in the TCK config file. It returns an instance of the the class Sys::Virt::TCK::DomainBuilder which lets you then add extra XML to the guest. eg, so you could do my $guest = $tck->generic_domain(); $guest->boot_cmdline($cmdline); The 'boot_cmdline' method doesn't exist yet, but feel frree to extend the shared helper modules as needed. Its much nicer to use them than to build up XML manually. > + > +diag $xml; > +diag "Defining an inactive domain config"; > +my $dom; > + > +# no need to start if already installed > +if (($already_installed == 0) && ($already_defined == 0)) { > + ok_domain(sub { $dom = $conn->define_domain($xml) }, "defined > persistent domain config"); > + $xml = $dom->get_xml_description; > + diag $xml; > + diag "Starting inactive domain config"; > + $dom->create; > + > + # wait for completion of installation > + diag "wait for installation to finish .. "; > + while($dom->is_active()) { > + sleep(10); > + diag ".. to view progress connect to virtual machine ${domain_name} .. > "; > + } > + sleep(10); > + diag " .. done"; > + # cleanup > + $dom->undefine; > +} else { > + ok_domain { $dom = $conn->get_domain_by_name($domain_name) } "the > existing domain object"; > +} > + > + > + > + > + > Index: libvirt-tck/scripts/network/001-boot-image.t > =================================================================== > --- /dev/null > +++ libvirt-tck/scripts/network/001-boot-image.t > @@ -0,0 +1,133 @@ > +# -*- perl -*- > +# > +# Copyright (C) 2010 IBM Corp. > +# > +# This program is free software; You can redistribute it and/or modify > +# it under the GNU General Public License as published by the Free > +# Software Foundation; either version 2, or (at your option) any > +# later version > +# > +# The file "LICENSE" distributed along with this file provides full > +# details of the terms and conditions > +# > + > +=pod > + > +=head1 NAME > + > +network/001-boot-image.t - boot installed test image > + > +=head1 DESCRIPTION > + > +The test case defines and boots a VM which uses the > +fedora virtual disk create ny 000-install-image > + > +=cut > + > +use strict; > +use warnings; > + > +use Test::More tests => 2; > + > +use Sys::Virt::TCK; > +use XML::LibXML; > + > +require 'scripts/network/common_functions.pl'; > + > +my $tck = Sys::Virt::TCK->new(); > +my $conn = eval { $tck->setup(); }; > +BAIL_OUT "failed to setup test harness: $@" if $@; > +END { $tck->cleanup if $tck; } > + > +my $already_defined = 0; > +my $domain_name ="f12nwtest"; > + > +# see if the domain already exits > +diag "searching if ${domain_name} is already defined"; > +my $nnames = $conn->num_of_defined_domains(); > +my @names = $conn->list_defined_domain_names($nnames); > +foreach (@names){ > + if (/${domain_name}/) { > + print "$_ already exists, no need to redefine\n"; > + $already_defined = 1; > + } > +} > +diag $already_defined; > + > +my $dom; > +my $xml; > + > +if ($already_defined == 1) { > + ok_domain { $dom = $conn->get_domain_by_name($domain_name) } "the > existing domain object"; > +} else { > + my $topxml = " <name>${domain_name}</name>". > + " <memory>524288</memory>". > + " <currentMemory>524288</currentMemory>". > + " <vcpu>1</vcpu>"; > + > + my $osxml = " <os>". > + " <type arch='x86_64' machine='fedora-13'>hvm</type>". > + " <boot dev='hd'/>". > + " </os>"; > + > + my $bottomxml = " <features>". > + " <acpi/>". > + " <apic/>". > + " </features>". > + " <clock offset='utc'/>". > + " <on_poweroff>destroy</on_poweroff>". > + " <on_reboot>restart</on_reboot>". > + " <on_crash>restart</on_crash>". > + " <devices>". > + " <emulator>/usr/bin/qemu-kvm</emulator>". > + " <disk type='file' device='disk'>". > + " <driver name='qemu' type='raw'/>". > + " <source file='/var/lib/libvirt/images/${domain_name}.img'/>". > + " <target dev='hda' bus='ide'/>". > + " </disk>". > + " <controller type='ide' index='0'>". > + " </controller>". > + " <interface type='network'>". > + " <source network='default'/>". > + " <filterref filter='no-spoofing'/>". > + " <target dev='vnet0'/>". > + " <model type='virtio'/>". > + " </interface>". > + " <serial type='pty'>". > + " <target port='0'/>". > + " </serial>". > + " <console type='pty'>". > + " <target port='0'/>". > + " </console>". > + " <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' > keymap='de'/>". > + " <video>". > + " <model type='cirrus' vram='9216' heads='1'/>". > + " </video>". > + " </devices>"; > + > + $xml = "<domain type='kvm'>" . > + $topxml. > + $osxml. > + $bottomxml. > + "</domain>"; Again, I think you'd be better using the '$tck->generic_domain' method, and adding support to Sys::Virt::TCK::DomainBuilder for adding the '<filterref filter='no-spoofing'/>' bit of the XML. > Index: libvirt-tck/scripts/network/100-ping-still-working.t > =================================================================== > --- /dev/null > +++ libvirt-tck/scripts/network/100-ping-still-working.t > @@ -0,0 +1,71 @@ > +# -*- perl -*- > +# > +# Copyright (C) 2010 IBM Corp. > +# > +# This program is free software; You can redistribute it and/or modify > +# it under the GNU General Public License as published by the Free > +# Software Foundation; either version 2, or (at your option) any > +# later version > +# > +# The file "LICENSE" distributed along with this file provides full > +# details of the terms and conditions > +# > + > +=pod > + > +=head1 NAME > + > +network/100-ping-still-working.t - verify machines can be pinged from > host > + > +=head1 DESCRIPTION > + > +The test case validates that it is possible to ping a guest machine > from > +the host. > + > +=cut > + > +use strict; > +use warnings; > + > +use Test::More tests => 4; > + > +use Sys::Virt::TCK; > +use Test::Exception; > +use Net::SSH::Perl; > +use XML::LibXML; > + > +require 'scripts/network/common_functions.pl'; > + > +my $tck = Sys::Virt::TCK->new(); > +my $conn = eval { $tck->setup(); }; > +BAIL_OUT "failed to setup test harness: $@" if $@; > +END { > + $tck->cleanup if $tck; > +} > + > +# create first domain and start it > +diag "Trying domain lookup by name"; > +my $dom1; > +my $domain_name ="f12nwtest"; This is where you'd want to start the guest domain > + > +ok_domain { $dom1 = $conn->get_domain_by_name($domain_name) } "the > running domain object"; > +my $xml = $dom1->get_xml_description; > +diag $xml; > +ok($dom1->get_id() > 0, "running domain has an ID > 0"); > +my $mac1 = get_macaddress($xml); > +diag $mac1; > +my $guestip1 = get_ip_from_leases($mac1); > +diag "ip is $guestip1"; > + > +# check ebtables entry > +my $ebtable1 = `/sbin/ebtables -L;/sbin/ebtables -t nat -L`; > +diag $ebtable1; > +# fixme to include mac adress > +ok($ebtable1 =~ "vnet0", "check ebtables entry"); > + > +# ping guest1 > +my $ping1 = `ping -c 10 $guestip1`; > +diag $ping1; > +ok($ping1 =~ "10 received", "ping $guestip1 test"); And run 'destroy' here. If you use a transient guest when initially booting ($conn->create_domain($xml)) then you'll not have to worry about the undefine step. > + > +sub get_macaddress { > + my $xmldesc = shift; > + > + my $mac; > + my $parser = XML::LibXML->new(); > + > + my $doc = $parser->parse_string($xmldesc); > + > + my $rootel = $doc -> getDocumentElement(); > + > + my @devices = $rootel->getChildrenByTagName("devices"); > + foreach my $device(@devices) { > + my @interfaces = $device->getChildrenByTagName("interface"); > + foreach my $interface(@interfaces) { > + my @targets = $interface->getChildrenByTagName("mac"); > + foreach my $target(@targets) { > + $mac = $target->getAttribute("address"); > + } > + } > + } > + utf8::decode($mac); > + return $mac; > +} You can simplify this using XPath - the $tck object provides a convenient method for performing an XPath query on a $dom object. eg that whole method can just be my $resultt = xpath($dom, "/domain/devices/interface/mac/\@address") my @macaddrs = map { $_->getNodeValue} $result->get_nodelist; Daniel -- |: 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list