[libvirt] [tck PATCH v2 0/4] set CTRL_IP_LEARNING and DHCPSERVER in filter during no-ip-spoofing test
We've recently discovered two separate bugs that caused libvirt's "DHCP Snooping" feature to not work: https://bugzilla.redhat.com/show_bug.cgi?id=1529338 - libvirt regression https://bugzilla.redhat.com/show_bug.cgi?id=1547237 - libpcap regression Since we didn't have any test suite covering that code, we had the embarrassment of learning of it from someone else's QE (RHV/oVirt QE at Red Hat). This series adds the necessary stuff to the test domain config of libvirt-tck's "no-ip-spoofing" test to exercise the DHCPSnoop thread. (There may be a much better way of dealing with a hash-inside-a-hash; I am an imbecile at perl, and arrived at this code by trial, error, and google searches). Laine Stump (4): nwfilter tests: auto-add test appliance ssh key to known_hosts on host new NetworkHelper function get_network_ip() set CTRL_IP_LEARNING and DHCPSERVER in filter during no-ip-spoofing test nwfilter tests: remove all hardcoded references to 192.168.122 network lib/Sys/Virt/TCK.pm | 11 ++++++++--- lib/Sys/Virt/TCK/DomainBuilder.pm | 8 +++++++- lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++ scripts/nwfilter/210-no-mac-spoofing.t | 12 ++++++++---- scripts/nwfilter/220-no-ip-spoofing.t | 26 +++++++++++++++++++++----- scripts/nwfilter/230-no-mac-broadcast.t | 11 ++++++++--- scripts/nwfilter/240-no-arp-spoofing.t | 22 ++++++++++++++++------ 7 files changed, 90 insertions(+), 22 deletions(-) -- 2.14.3
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@laine.org> --- 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
On Thu, Mar 01, 2018 at 09:49:57PM -0500, Laine Stump wrote:
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@laine.org> ---
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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@laine.org> --- 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 Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
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@laine.org> ---
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);
This isn't part of base perl, so you'll need to list it in Build.PL and the RPM spec file.
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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
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@laine.org> ---
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); This isn't part of base perl, so you'll need to list it in Build.PL and
On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote: the RPM spec file.
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?
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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Regards, Daniel
On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote:
On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
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@laine.org> ---
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); This isn't part of base perl, so you'll need to list it in Build.PL and
On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote: the RPM spec file.
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?
We don't need to list it with a Requires: tag because automatic dependancies take care of that. We need it listed as BuildRequires though, *if* the NetworkHelpers mod is pulled in by any of the unit tests which I thought it was (but I could be wrong there). Still need it in the Build.PL no matter what, as that's what CPAN and other Perl tools use. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 03/05/2018 10:15 AM, Daniel P. Berrangé wrote:
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@laine.org> ---
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); This isn't part of base perl, so you'll need to list it in Build.PL and
On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote: the RPM spec file. 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
On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote: 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? We don't need to list it with a Requires: tag because automatic dependancies take care of that. We need it listed as BuildRequires
On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote: though, *if* the NetworkHelpers mod is pulled in by any of the unit tests which I thought it was (but I could be wrong there).
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..
Still need it in the Build.PL no matter what, as that's what CPAN and other Perl tools use.
Ah, for non-rpm-based installations, right? Okay, I'll add that and resend.
On Mon, Mar 05, 2018 at 03:32:02PM -0500, Laine Stump wrote:
On 03/05/2018 10:15 AM, Daniel P. Berrangé wrote:
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@laine.org> ---
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); This isn't part of base perl, so you'll need to list it in Build.PL and
On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote: the RPM spec file. 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
On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote: 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? We don't need to list it with a Requires: tag because automatic dependancies take care of that. We need it listed as BuildRequires
On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote: though, *if* the NetworkHelpers mod is pulled in by any of the unit tests which I thought it was (but I could be wrong there).
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..
Yep, that sounds fine then.
Still need it in the Build.PL no matter what, as that's what CPAN and other Perl tools use.
Ah, for non-rpm-based installations, right?
Yes, exactly - the cpan CLI uses the info there to figure out what dependancies need installing. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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@laine.org> --- 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
On Thu, Mar 01, 2018 at 09:49:59PM -0500, Laine Stump wrote:
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@laine.org> ---
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(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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@laine.org> --- 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
On Thu, Mar 01, 2018 at 09:50:00PM -0500, Laine Stump wrote:
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@laine.org> ---
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(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Laine Stump