Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: filter out non-ICMP packets in the tun test #4543

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

evverx
Copy link
Contributor

@evverx evverx commented Oct 1, 2024

When systemd-resolved with its LLMNR/mDNS responders enabled is run it starts sending its probes as soon as the tun interface pops up:

00:36:36.643708 IP c > igmp.mcast.net: igmp v3 report, 2 group record(s)
00:36:36.644530 IP c.mdns > mdns.mcast.net.mdns: 0 [1n] ANY (QM)? c.local. (44)
00:36:36.645307 IP c.llmnr > 224.0.0.252.llmnr: UDP, length 22

and that interferes with the test. Since the test is interested in ICMP packets only it can safely skip everything else.

Fixes:

 #(006)=[failed] Send ping packets from Linux into Scapy
 ...
 assert len(icmp4_sequences) == 3
 AssertionError

(The tap test seems to fail too but as far as I can see it has something to do with cd01ec1 (according to git bisect). I'll try to take a closer look later. As far as I can tell src candidates in conf.route6 are different from what the test expects there)

When systemd-resolved with its LLMNR/mDNS responders enabled is run it
starts sending its probes as soon as the tun interface pops up:
```
00:36:36.643708 IP c > igmp.mcast.net: igmp v3 report, 2 group record(s)
00:36:36.644530 IP c.mdns > mdns.mcast.net.mdns: 0 [1n] ANY (QM)? c.local. (44)
00:36:36.645307 IP c.llmnr > 224.0.0.252.llmnr: UDP, length 22
```
and that interferes with the test. Since the test is interested in
ICMP packets only it can safely skip everything else.

Fixes:
```
 #(006)=[failed] Send ping packets from Linux into Scapy
 ...
 assert len(icmp4_sequences) == 3
 AssertionError
```
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.61%. Comparing base (1f9061d) to head (856fc91).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4543      +/-   ##
==========================================
+ Coverage   81.24%   81.61%   +0.37%     
==========================================
  Files         356      356              
  Lines       85533    85533              
==========================================
+ Hits        69491    69808     +317     
+ Misses      16042    15725     -317     

see 19 files with indirect coverage changes

Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch ! Thanks

@gpotter2 gpotter2 merged commit 79974f2 into secdev:master Oct 1, 2024
24 checks passed
@evverx
Copy link
Contributor Author

evverx commented Oct 1, 2024

The tap test seems to fail too but as far as I can see it has something to do with cd01ec1 (according to git bisect)

Looks like it could have been caught by the CI if it had run the IPv6 tests: https://github.com/evverx/scapy/actions/runs/11134596243/job/30943071529?pr=2

Here's what conf.route6 looks like with the master branch (where the test fails):

2024-10-01T22:29:56.1737307Z Destination                  Next Hop  Iface  Src candidates             Metric
2024-10-01T22:29:56.1737973Z ::1/128                      ::        lo     ::1                        0     
2024-10-01T22:29:56.1738645Z 2001:db8::1/128              ::        tun0   2001:db8::2                0     
2024-10-01T22:29:56.1739262Z fe80::6245:bdff:feee:423d/_  ::        eth0   fe80::6245:bdff:feee:423d  0     
2024-10-01T22:29:56.1739926Z fe80::8208:2d66:c4fb:3987/_  ::        tun0   fe80::8208:2d66:c4fb:3987  0     
2024-10-01T22:29:56.1740607Z ::1/128                      ::        lo     ::1                        256   
2024-10-01T22:29:56.1741424Z 2001:db8::1/128              ::        tun0   2001:db8::2                256   
2024-10-01T22:29:56.1742015Z 2001:db8::2/128              ::        tun0   2001:db8::2                256   
2024-10-01T22:29:56.1742658Z fe80::/64                    ::        eth0   fe80::6245:bdff:feee:423d  256   
2024-10-01T22:29:56.1743226Z fe80::/64                    ::        tun0   fe80::8208:2d66:c4fb:3987  256   
2024-10-01T22:29:56.1743763Z ff00::/8                     ::        eth0   fe80::6245:bdff:feee:423d  250   
2024-10-01T22:29:56.1744364Z ff00::/8                     ::        tun0   2001:db8::2                250   
2024-10-01T22:29:56.1744903Z                                               fe80::8208:2d66:c4fb:3987        
2024-10-01T22:29:56.1745220Z 

Here's what it looks like without cd01ec1 (where the test passes):

2024-10-01T22:42:27.5093454Z Destination                  Next Hop  Iface  Src candidates             Metric
2024-10-01T22:42:27.5094391Z 2001:db8::1/128              ::        tun0   2001:db8::1                256   
2024-10-01T22:42:27.5095307Z 2001:db8::2/128              ::        tun0   2001:db8::1                256   
2024-10-01T22:42:27.5096165Z fe80::/64                    ::        eth0   fe80::20d:3aff:fed4:567b   256   
2024-10-01T22:42:27.5096977Z fe80::/64                    ::        tun0   fe80::4cbf:cb15:d001:d665  256   
2024-10-01T22:42:27.5097768Z ::1/128                      ::        lo     ::1                        0     
2024-10-01T22:42:27.5098521Z 2001:db8::1/128              ::        tun0   2001:db8::1                0     
2024-10-01T22:42:27.5099447Z fe80::20d:3aff:fed4:567b/1_  ::        eth0   fe80::20d:3aff:fed4:567b   0     
2024-10-01T22:42:27.5100391Z fe80::4cbf:cb15:d001:d665/_  ::        tun0   fe80::4cbf:cb15:d001:d665  0     

As far as can tell the test fails because 2001:db8::1 is no longer among the src candidates so src is 2001:db8::2 where 2001:db8::1 is expected.

<IPv6  version=6 tc=0 fl=0 plen=8 nh=ICMPv6 hlim=64 src=2001:db8::2 dst=2001:db8::2 |<ICMPv6EchoRequest  type=Echo Request code=0 cksum=0x2446 id=0x0 seq=0x1 data=b'' |>>
<IPv6  version=6 tc=0 fl=0 plen=8 nh=ICMPv6 hlim=64 src=2001:db8::2 dst=2001:db8::2 |<ICMPv6EchoRequest  type=Echo Request code=0 cksum=0x2445 id=0x0 seq=0x2 data=b'' |>>
<IPv6  version=6 tc=0 fl=0 plen=8 nh=ICMPv6 hlim=64 src=2001:db8::2 dst=2001:db8::2 |<ICMPv6EchoRequest  type=Echo Request code=0 cksum=0x2444 id=0x0 seq=0x3 data=b'' |>>

I'll take a closer look a bit later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants