Without this option, attempts to ssh into the test appliance will fail unless someone has previously ssh'ed into the appliance manually and accepted its key. Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- New in V2. This isn't necessarily related to the $subject of the cover letter, but it's easier to send it along with the other patches. scripts/nwfilter/210-no-mac-spoofing.t | 3 ++- scripts/nwfilter/220-no-ip-spoofing.t | 3 ++- scripts/nwfilter/230-no-mac-broadcast.t | 3 ++- scripts/nwfilter/240-no-arp-spoofing.t | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/scripts/nwfilter/210-no-mac-spoofing.t b/scripts/nwfilter/210-no-mac-spoofing.t index 3438f4a..148fbeb 100644 --- a/scripts/nwfilter/210-no-mac-spoofing.t +++ b/scripts/nwfilter/210-no-mac-spoofing.t @@ -92,7 +92,8 @@ ok($ping =~ "10 received", "ping $guestip test"); diag "ssh'ing into $guestip"; my $ssh = Net::OpenSSH->new($guestip, user => "root", - password => $tck->root_password()); + password => $tck->root_password(), + master_opts => [-o => "StrictHostKeyChecking=no"]); # now bring eth0 down, change MAC and bring it up again diag "fiddling with mac"; diff --git a/scripts/nwfilter/220-no-ip-spoofing.t b/scripts/nwfilter/220-no-ip-spoofing.t index 9e1bb70..09bd51c 100644 --- a/scripts/nwfilter/220-no-ip-spoofing.t +++ b/scripts/nwfilter/220-no-ip-spoofing.t @@ -75,7 +75,8 @@ ok($ebtable =~ "$guestip", "check ebtables entry"); diag "ssh'ing into $guestip"; my $ssh = Net::OpenSSH->new($guestip, user => "root", - password => $tck->root_password()); + password => $tck->root_password(), + master_opts => [-o => "StrictHostKeyChecking=no"]); # now bring eth0 down, change IP and bring it up again diag "preparing ip spoof"; diff --git a/scripts/nwfilter/230-no-mac-broadcast.t b/scripts/nwfilter/230-no-mac-broadcast.t index 758005c..6f5318a 100644 --- a/scripts/nwfilter/230-no-mac-broadcast.t +++ b/scripts/nwfilter/230-no-mac-broadcast.t @@ -86,7 +86,8 @@ system("/usr/sbin/tcpdump -v -i virbr0 -n host 192.168.122.255 and ether host ff diag "ssh'ing into $guestip"; my $ssh = Net::OpenSSH->new($guestip, user => "root", - password => $tck->root_password()); + password => $tck->root_password(), + master_opts => [-o => "StrictHostKeyChecking=no"]); # now generate a mac broadcast paket diag "generate mac broadcast"; diff --git a/scripts/nwfilter/240-no-arp-spoofing.t b/scripts/nwfilter/240-no-arp-spoofing.t index dfc8e08..a8ab7a5 100644 --- a/scripts/nwfilter/240-no-arp-spoofing.t +++ b/scripts/nwfilter/240-no-arp-spoofing.t @@ -89,7 +89,8 @@ system("/usr/sbin/tcpdump -v -i virbr0 not ip > /tmp/tcpdump.log &"); diag "ssh'ing into $guestip"; my $ssh = Net::OpenSSH->new($guestip, user => "root", - password => $tck->root_password()); + password => $tck->root_password(), + master_opts => [-o => "StrictHostKeyChecking=no"]); # now generate a arp spoofing packets diag "generate arpspoof script"; -- 2.14.3

This function gets the first IP address for the named virtual network. It is returned as a Net::IP object, so that we will have info about its netmask/prefix and can easily get it broadcast address and perform arithmetic on the address. Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- Change from V1: return a NetAddr::IP object instead of a string. lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm index 5f563e5..7bbce62 100644 --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm @@ -1,4 +1,5 @@ use Sys::Virt::TCK qw(xpath); +use NetAddr::IP qw(:lower); use strict; use utf8; @@ -9,6 +10,27 @@ sub get_first_macaddress { return $mac; } +sub get_network_ip { + my $conn = shift; + my $netname = shift; + diag "getting ip for network $netname"; + my $net = $conn->get_network_by_name($netname); + my $net_ip = xpath($net, "string(/network/ip[1]/\@address"); + my $net_mask = xpath($net, "string(/network/ip[1]/\@netmask"); + my $net_prefix = xpath($net, "string(/network/ip[1]/\@prefix"); + my $ip; + + if ($net_mask) { + $ip = NetAddr::IP->new($net_ip, $net_mask); + } elsif ($net_prefix) { + $ip = NetAddr::IP->new("$net_ip/$net_mask"); + } else { + $ip = NetAddr::IP->new("$net_ip"); + } + return $ip; +} + + sub get_ip_from_leases{ my $conn = shift; my $netname = shift; -- 2.14.3

On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote: I originally assumed that, but remembered seeing "something somewhere" about implicit dependencies and decided to try it out by not listing it in the specfile - on both Fedora and RHEL7 the dependency was properly pulled in and it was installed. This leads to one of three possibilities: 1) implicit dependencies are figured out properly by yum and dnf (at least for RHEL7, don't know about RHEL6). 2) (1), but it's just coincidentally happening and not guaranteed. 3) I wasn't paying attention when I tested, and what I say isn't actually true. I don't have any problem putting in the explicit Requires though. Can I assumed a Reviewed-by with that in place?

On 03/05/2018 10:15 AM, Daniel P. Berrangé wrote: Well, being the perl-idiot that I am, I didn't realize that the unit tests even *existed* until you said that and I investigated to see what you were talking about :-) So what you're saying is that if any function in NetworkHelpers is either directly or indirectly used in the unit tests (which are in the "t" subdirectory of the source tree, right?) then we need to list is in BuildRequires, correct? I looked in the unit tests  and while NetworkBuilders is pulled it, NetworkHelpers isn't (and NetworkBuilders doesn't pull in NetworkHelpers), so I *think* we're safe, but I'll try building on a system where I've removed the NetAddr::IP package to verify that it's still successful.. Ah, for non-rpm-based installations, right? Okay, I'll add that and resend.

Adding these parameters to the clean-traffic filter causes a significant extra piece of code to be executed (a separate thread is started up, which uses libpcap to capture DHCP traffic and learn the IP address of the guest / test appliance), so let's get some test coverage on that code. Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- Change from V1: * set %filterparams to () instead of undef when not specified. (undef caused a runtime error that I hadn't noticed, since the result was the same) * adjust to use NetAddr::IP object instead of string for networkip. lib/Sys/Virt/TCK.pm | 11 ++++++++--- lib/Sys/Virt/TCK/DomainBuilder.pm | 8 +++++++- scripts/nwfilter/220-no-ip-spoofing.t | 11 ++++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index 3f650a8..f9d9f30 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -767,6 +767,7 @@ sub generic_machine_domain { my $ostype = exists $params{ostype} ? $params{ostype} : "hvm"; my $fullos = exists $params{fullos} ? $params{fullos} : 0; my $filterref = exists $params{filterref} ? $params{filterref} : undef; + my %filterparams = exists $params{filterparams} ? %{$params{filterparams}} : (); if ($fullos) { my %config = $self->get_image($caps, $ostype); @@ -793,7 +794,8 @@ sub generic_machine_domain { source => "default", model => "virtio", mac => "52:54:00:11:11:11", - filterref => $filterref); + filterref => $filterref, + filterparams => \%filterparams); my $xml = $b->as_xml(); # Cleanup the temporary interface $b->rminterface(); @@ -898,6 +900,7 @@ sub generic_domain { my $fullos = exists $params{fullos} ? $params{fullos} : 0; my $netmode = exists $params{netmode} ? $params{netmode} : undef; my $filterref = exists $params{filterref} ? $params{filterref} : undef; + my %filterparams = exists $params{filterparams} ? %{$params{filterparams}} : (); my $caps = Sys::Virt::TCK::Capabilities->new(xml => $self->conn->get_capabilities); @@ -918,7 +921,8 @@ sub generic_domain { caps => $caps, ostype => $ostype, fullos => $fullos, - filterref => $filterref); + filterref => $filterref, + filterparams => \%filterparams); } if ($netmode) { if ($netmode eq "vepa") { @@ -934,7 +938,8 @@ sub generic_domain { source => "default", model => "virtio", mac => "52:54:00:11:11:11", - filterref => $filterref); + filterref => $filterref, + filterparams => \%filterparams); } } return $b; diff --git a/lib/Sys/Virt/TCK/DomainBuilder.pm b/lib/Sys/Virt/TCK/DomainBuilder.pm index fb9a31f..83cea15 100644 --- a/lib/Sys/Virt/TCK/DomainBuilder.pm +++ b/lib/Sys/Virt/TCK/DomainBuilder.pm @@ -459,8 +459,14 @@ sub as_xml { type => $interface->{model}); } if ($interface->{filterref}) { - $w->emptyTag("filterref", + $w->startTag("filterref", filter => $interface->{filterref}); + foreach my $paramname (keys %{$interface->{filterparams}}) { + $w->emptyTag("parameter", + name => $paramname, + value => $interface->{filterparams}->{$paramname}); + } + $w->endTag("filterref"); } $w->endTag("interface"); } diff --git a/scripts/nwfilter/220-no-ip-spoofing.t b/scripts/nwfilter/220-no-ip-spoofing.t index 09bd51c..2f454c5 100644 --- a/scripts/nwfilter/220-no-ip-spoofing.t +++ b/scripts/nwfilter/220-no-ip-spoofing.t @@ -42,10 +42,19 @@ END { $tck->cleanup if $tck; } +my $networkip = get_network_ip($conn, "default"); +my $networkipaddr = $networkip->addr(); +diag "network ip is $networkip, individual ip is $networkipaddr"; + + # create first domain and start it my $xml = $tck->generic_domain(name => "tck", fullos => 1, netmode => "network", - filterref => "clean-traffic")->as_xml(); + filterref => "clean-traffic", + filterparams => { + CTRL_IP_LEARNING => "dhcp", + DHCPSERVER => $networkipaddr + })->as_xml(); my $dom; ok_domain(sub { $dom = $conn->define_domain($xml) }, "created persistent domain object"); -- 2.14.3

The nwfilter tests have a few places that hardcode 192.168.122 as the address of libvirt's default network. Remove all of these and replace them with addresses that are dynamically determined based on get_network_ip(). (This will have the immediate effect of helping the tests to succeed when libvirt-tck is run in a virtual machine, since virtual machines often have their default network set to a different subnet (in order to avoid conflict with the L0 host's default network)). Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- New in V2. Another patch not necessarily related to $subject of the cover letter, but useful to have. scripts/nwfilter/210-no-mac-spoofing.t | 9 ++++++--- scripts/nwfilter/220-no-ip-spoofing.t | 14 ++++++++++---- scripts/nwfilter/230-no-mac-broadcast.t | 8 ++++++-- scripts/nwfilter/240-no-arp-spoofing.t | 19 ++++++++++++++----- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/scripts/nwfilter/210-no-mac-spoofing.t b/scripts/nwfilter/210-no-mac-spoofing.t index 148fbeb..7b74f94 100644 --- a/scripts/nwfilter/210-no-mac-spoofing.t +++ b/scripts/nwfilter/210-no-mac-spoofing.t @@ -42,6 +42,10 @@ END { $tck->cleanup if $tck; } +my $networkip = get_network_ip($conn, "default"); +my $networkipaddr = $networkip->addr(); +diag "network ip is $networkip, individual ip is $networkipaddr"; + # create first domain and start it my $xml = $tck->generic_domain(name => "tck", fullos => 1, netmode => "network", @@ -71,7 +75,7 @@ my $mac = get_first_macaddress($dom); diag "mac is $mac"; my $guestip = get_ip_from_leases($conn, "default", $mac); -diag "ip is $guestip"; +diag "guest ip is $guestip"; # check ebtables entry my $ebtables = (-e '/sbin/ebtables') ? '/sbin/ebtables' : '/usr/sbin/ebtables'; @@ -82,7 +86,6 @@ $_ = $mac; s/00/0/g; ok($ebtable =~ $_, "check ebtables entry"); -my $gateway = "192.168.122.1"; my $macfalse = "52:54:00:f9:21:22"; my $ping = `ping -c 10 $guestip`; diag $ping; @@ -104,7 +107,7 @@ ip link set \\\$DEV down ip link set \\\$DEV address ${macfalse} ip link set \\\$DEV up ip addr show dev \\\$DEV -ping -c 10 ${gateway} 2>&1 +ping -c 10 ${networkipaddr} 2>&1 ip link set \\\$DEV down ip link set \\\$DEV address ${mac} ip link set \\\$DEV up diff --git a/scripts/nwfilter/220-no-ip-spoofing.t b/scripts/nwfilter/220-no-ip-spoofing.t index 2f454c5..85c4807 100644 --- a/scripts/nwfilter/220-no-ip-spoofing.t +++ b/scripts/nwfilter/220-no-ip-spoofing.t @@ -45,7 +45,6 @@ END { my $networkip = get_network_ip($conn, "default"); my $networkipaddr = $networkip->addr(); diag "network ip is $networkip, individual ip is $networkipaddr"; - # create first domain and start it my $xml = $tck->generic_domain(name => "tck", fullos => 1, @@ -71,7 +70,14 @@ my $mac = get_first_macaddress($dom); diag "mac is $mac"; my $guestip = get_ip_from_leases($conn, "default", $mac); -diag "ip is $guestip"; +diag "guest ip is $guestip"; + +my $spoofip = $networkip + 1; +if ($spoofip->addr() eq $guestip) { + $spoofip++; +} +my $spoofipaddr = $spoofip->addr(); +diag "spoof ip is $spoofipaddr"; # check ebtables entry my $ebtables = (-e '/sbin/ebtables') ? '/sbin/ebtables' : '/usr/sbin/ebtables'; @@ -96,11 +102,11 @@ ip addr show \\\$DEV kill \\\$(pidof dhclient) ip link set \\\$DEV down ip addr flush dev \\\$DEV -ip addr add 192.168.122.183/\\\$MASK dev \\\$DEV +ip addr add ${spoofipaddr}/\\\$MASK dev \\\$DEV ip link set \\\$DEV up ip addr show \\\$DEV sleep 1 -ping -c 1 192.168.122.1 +ping -c 1 ${networkipaddr} ip link set \\\$DEV down ip addr flush dev \\\$DEV ip addr add ${guestip}/\\\$MASK dev \\\$DEV diff --git a/scripts/nwfilter/230-no-mac-broadcast.t b/scripts/nwfilter/230-no-mac-broadcast.t index 6f5318a..08695ae 100644 --- a/scripts/nwfilter/230-no-mac-broadcast.t +++ b/scripts/nwfilter/230-no-mac-broadcast.t @@ -41,6 +41,10 @@ END { $tck->cleanup if $tck; } +my $networkip = get_network_ip($conn, "default"); +my $networkipbroadcast = $networkip->broadcast()->addr(); +diag "network ip is $networkip, broadcast address is $networkipbroadcast"; + # create first domain and start it my $xml = $tck->generic_domain(name => "tck", fullos => 1, netmode => "network", @@ -80,7 +84,7 @@ ok($ebtable =~ "-d Broadcast -j DROP", "check ebtables entry for \"-d Broadcast # prepare tcpdump diag "prepare tcpdump"; -system("/usr/sbin/tcpdump -v -i virbr0 -n host 192.168.122.255 and ether host ff:ff:ff:ff:ff:ff 2> /tmp/tcpdump.log &"); +system("/usr/sbin/tcpdump -v -i virbr0 -n host $networkipbroadcast and ether host ff:ff:ff:ff:ff:ff 2> /tmp/tcpdump.log &"); # log into guest diag "ssh'ing into $guestip"; @@ -92,7 +96,7 @@ my $ssh = Net::OpenSSH->new($guestip, # now generate a mac broadcast paket diag "generate mac broadcast"; my $cmdfile = <<EOF; -echo 'ping -c 1 192.168.122.255 -b' > /test.sh +echo 'ping -c 1 $networkipbroadcast -b' > /test.sh EOF diag $cmdfile; my ($stdout, $stderr) = $ssh->capture2($cmdfile); diff --git a/scripts/nwfilter/240-no-arp-spoofing.t b/scripts/nwfilter/240-no-arp-spoofing.t index a8ab7a5..350b604 100644 --- a/scripts/nwfilter/240-no-arp-spoofing.t +++ b/scripts/nwfilter/240-no-arp-spoofing.t @@ -34,8 +34,6 @@ use Test::Exception; use Net::OpenSSH; use File::Spec::Functions qw(catfile catdir rootdir); -my $spoofid = "192.168.122.183"; - my $tck = Sys::Virt::TCK->new(); my $conn = eval { $tck->setup(); }; BAIL_OUT "failed to setup test harness: $@" if $@; @@ -43,6 +41,10 @@ END { $tck->cleanup if $tck; } +my $networkip = get_network_ip($conn, "default"); +my $networkipaddr = $networkip->addr(); +diag "network ip is $networkip, individual ip is $networkipaddr"; + # create first domain and start it my $xml = $tck->generic_domain(name => "tck", fullos => 1, netmode => "network", @@ -72,7 +74,14 @@ my $mac = get_first_macaddress($dom); diag "mac is $mac"; my $guestip = get_ip_from_leases($conn, "default", $mac); -diag "ip is $guestip"; +diag "guest ip is $guestip"; + +my $spoofip = $networkip + 1; +if ($spoofip->addr() eq $guestip) { + $spoofip++; +} +my $spoofipaddr = $spoofip->addr(); +diag "spoof ip is $spoofipaddr"; # check ebtables entry my $ebtables = (-e '/sbin/ebtables') ? '/sbin/ebtables' : '/usr/sbin/ebtables'; @@ -95,7 +104,7 @@ my $ssh = Net::OpenSSH->new($guestip, # now generate a arp spoofing packets diag "generate arpspoof script"; my $cmdfile = <<EOF; -echo "arpspoof ${spoofid} & +echo "arpspoof ${spoofipaddr} & sleep 10 kill -15 \\\$(pidof arpspoof)" > /test.sh EOF @@ -127,7 +136,7 @@ system("kill -15 `/sbin/pidof tcpdump`"); diag "tcpdump.log:"; my $tcpdumplog = `cat /tmp/tcpdump.log`; diag($tcpdumplog); -ok($tcpdumplog !~ "${spoofid} is-at", "tcpdump expected to capture no arp reply packets"); +ok($tcpdumplog !~ "${spoofipaddr} is-at", "tcpdump expected to capture no arp reply packets"); shutdown_vm_gracefully($dom); -- 2.14.3