From ee2126a4e25123d7ea17027e397f0a92cb58aaf8 Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Mon, 14 Oct 2019 10:29:58 +0200 Subject: [PATCH 1/3] tests/gnrc_rpl_srh: fix test assumption If the destination address or an address within the source route is multicast within a RPL source routing header, a receiving node is supposed to just discard the packets, but not to send an ICMPv6 error message, as the test assumes at the moment. Source: https://tools.ietf.org/html/rfc6554#section-4.2 --- tests/gnrc_rpl_srh/tests/01-run.py | 40 ++++++++++++++++++------------ 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/tests/gnrc_rpl_srh/tests/01-run.py b/tests/gnrc_rpl_srh/tests/01-run.py index c86d7ca0a0..47a50215dd 100755 --- a/tests/gnrc_rpl_srh/tests/01-run.py +++ b/tests/gnrc_rpl_srh/tests/01-run.py @@ -13,7 +13,7 @@ import sys import subprocess import threading -from scapy.all import Ether, IPv6, \ +from scapy.all import Ether, IPv6, UDP, \ IPv6ExtHdrHopByHop, IPv6ExtHdrDestOpt, \ IPv6ExtHdrFragment, IPv6ExtHdrRouting, \ ICMPv6ParamProblem, ICMPv6TimeExceeded, \ @@ -194,29 +194,37 @@ def test_seg_left_gt_len_addresses(child, iface, hw_dst, ll_dst, ll_src): def test_multicast_dst(child, iface, hw_dst, ll_dst, ll_src): # sniffing for ICMPv6 parameter problem message - sniffer.start_sniff(lambda p: p.haslayer(ICMPv6ParamProblem)) + sniffer.start_sniff(lambda p: p.haslayer(ICMPv6ParamProblem) or + (p.haslayer(UDP) and (p[IPv6].dst != "ff02::1"))) # send routing header with multicast destination sendp(Ether(dst=hw_dst) / IPv6(dst="ff02::1", src=ll_src) / - IPv6ExtHdrRouting(type=3, segleft=1, addresses=["abcd::1"]), - iface=iface, verbose=0) + IPv6ExtHdrRouting(type=3, segleft=1, addresses=["abcd::1"]) / + UDP(dport=2606), iface=iface, verbose=0) ps = sniffer.wait_for_sniff_results() - p = [p for p in ps if ICMPv6ParamProblem in p] - assert(len(p) > 0) - p = p[0] - assert(p[ICMPv6ParamProblem].code == 0) # erroneous header field encountered - assert(p[ICMPv6ParamProblem].ptr == 24) # IPv6 headers destination field + p = [p for p in ps if (ICMPv6ParamProblem in p) or + ((UDP in p) and (p[UDP].dport == 2606) and + (p[IPv6].dst != "ff02::1"))] + # packet should be discarded silently: + # see https://tools.ietf.org/html/rfc6554#section-4.2 + assert(len(p) == 0) pktbuf_empty(child) def test_multicast_addr(child, iface, hw_dst, ll_dst, ll_src): + # sniffing for ICMPv6 parameter problem message + sniffer.start_sniff(lambda p: p.haslayer(ICMPv6ParamProblem) or + (p.haslayer(UDP) and (p[IPv6].dst != ll_dst))) # Send routing header with multicast address in its destinations - p = srp1(Ether(dst=hw_dst) / IPv6(dst=ll_dst, src=ll_src) / - IPv6ExtHdrRouting(type=3, segleft=1, addresses=["ff02::1"]), - iface=iface, timeout=1, verbose=0) - assert(p is not None) - assert(ICMPv6ParamProblem in p) - assert(p[ICMPv6ParamProblem].code == 0) # erroneous header field encountered - assert(p[ICMPv6ParamProblem].ptr == 48) # first address in routing header + sendp(Ether(dst=hw_dst) / IPv6(dst=ll_dst, src=ll_src) / + IPv6ExtHdrRouting(type=3, segleft=1, addresses=["abcd::1"]) / + UDP(dport=2606), iface=iface, verbose=0) + ps = sniffer.wait_for_sniff_results() + p = [p for p in ps if (ICMPv6ParamProblem in p) or + ((UDP in p) and (p[UDP].dport == 2606) and + (p[IPv6].dst != ll_dst))] + # packet should be discarded silently: + # see https://tools.ietf.org/html/rfc6554#section-4.2 + assert(len(p) == 0) pktbuf_empty(child) From a2792286631ef127afc50116631a8ec78b3bde91 Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Mon, 14 Oct 2019 11:25:53 +0200 Subject: [PATCH 2/3] gnrc_rpl_srh: don't send error message on multicast error A node is not supposed to send an ICMPv6 error message when the destination or one of the addresses in the source route is multicast but is supposed to be silently discarded (see [RFC4443] and [RFC6554]). If we leave the `err_ptr` unset, the [node will not send an error message][err_ptr set]. [RFC 4443]: https://tools.ietf.org/html/rfc4443#section-2.4 [RFC 6554]: https://tools.ietf.org/html/rfc6554#section-4.2 [err_ptr set]: https://github.com/RIOT-OS/RIOT/blob/9bc600a/sys/net/gnrc/network_layer/ipv6/ext/rh/gnrc_ipv6_ext_rh.c#L100-L105 --- sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c b/sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c index fdf72ccbe7..1bf47f5dc0 100644 --- a/sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c +++ b/sys/net/gnrc/routing/rpl/srh/gnrc_rpl_srh.c @@ -99,12 +99,10 @@ int gnrc_rpl_srh_process(ipv6_hdr_t *ipv6, gnrc_rpl_srh_t *rh, void **err_ptr) if (ipv6_addr_is_multicast(&ipv6->dst)) { DEBUG("RPL SRH: found a multicast destination address - discard\n"); - *err_ptr = &ipv6->dst; return GNRC_IPV6_EXT_RH_ERROR; } if (ipv6_addr_is_multicast(&addr)) { DEBUG("RPL SRH: found a multicast addres in next address - discard\n"); - *err_ptr = current_address; return GNRC_IPV6_EXT_RH_ERROR; } From 3e7f25566feba228525b709b9552cf5a9362478e Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Mon, 14 Oct 2019 12:04:35 +0200 Subject: [PATCH 3/3] tests/gnrc_rpl_srh: fix unittests for `gnrc_rpl_srh` behavior change --- tests/gnrc_rpl_srh/main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/gnrc_rpl_srh/main.c b/tests/gnrc_rpl_srh/main.c index 26bb43adba..63a6aca249 100644 --- a/tests/gnrc_rpl_srh/main.c +++ b/tests/gnrc_rpl_srh/main.c @@ -85,7 +85,7 @@ static void test_rpl_srh_dst_multicast(void) static const ipv6_addr_t mcast = IPV6_MCAST_ADDR; gnrc_rpl_srh_t *srh; uint8_t *vec; - void *err_ptr; + void *err_ptr = NULL; int res; _init_hdrs(&srh, &vec, &mcast); @@ -96,7 +96,7 @@ static void test_rpl_srh_dst_multicast(void) res = gnrc_rpl_srh_process(&hdr, srh, &err_ptr); TEST_ASSERT_EQUAL_INT(res, GNRC_IPV6_EXT_RH_ERROR); - TEST_ASSERT((&hdr.dst) == err_ptr); + TEST_ASSERT_NULL(err_ptr); } static void test_rpl_srh_route_multicast(void) @@ -106,7 +106,7 @@ static void test_rpl_srh_route_multicast(void) static const ipv6_addr_t dst = IPV6_DST; gnrc_rpl_srh_t *srh; uint8_t *vec; - void *err_ptr; + void *err_ptr = NULL; int res; _init_hdrs(&srh, &vec, &dst); @@ -117,7 +117,7 @@ static void test_rpl_srh_route_multicast(void) res = gnrc_rpl_srh_process(&hdr, srh, &err_ptr); TEST_ASSERT_EQUAL_INT(res, GNRC_IPV6_EXT_RH_ERROR); - TEST_ASSERT(vec == err_ptr); + TEST_ASSERT_NULL(err_ptr); } static void test_rpl_srh_too_many_seg_left(void)