From 2e73aa507b9f1d5d74e43cd9b812fc1cfff795cb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Apr 2024 12:05:07 +0900 Subject: [PATCH 1/7] network/ndisc: do not set per-route MTU and hop limit Setting MTU announced in RA message to routes is problematic, as the value may be larger than the device MTU (IFLA_MTU), and in such case the route cannot be used. These two properties are now set per-interface, and gracefully handled such invalid cases. Hence not necessary to set them to each route. Follow-up for #32195. --- src/network/networkd-ndisc.c | 21 +-------------------- test/test-network/systemd-networkd-tests.py | 21 +++++++++------------ 2 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index f0c58e8842b..73a88bd4096 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -179,8 +179,6 @@ static void ndisc_set_route_priority(Link *link, Route *route) { static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { struct in6_addr router; - uint8_t hop_limit = 0; - uint32_t mtu = 0; bool is_new; int r; @@ -194,30 +192,13 @@ static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { if (r < 0) return r; - if (link->network->ndisc_use_mtu) { - r = sd_ndisc_router_get_mtu(rt, &mtu); - if (r < 0 && r != -ENODATA) - return log_link_warning_errno(link, r, "Failed to get MTU from RA: %m"); - } - - if (link->network->ndisc_use_hop_limit) { - r = sd_ndisc_router_get_hop_limit(rt, &hop_limit); - if (r < 0 && r != -ENODATA) - return log_link_warning_errno(link, r, "Failed to get hop limit from RA: %m"); - } - route->source = NETWORK_CONFIG_SOURCE_NDISC; route->provider.in6 = router; if (!route->table_set) route->table = link_get_ndisc_route_table(link); if (!route->protocol_set) route->protocol = RTPROT_RA; - r = route_metric_set(&route->metric, RTAX_MTU, mtu); - if (r < 0) - return r; - r = route_metric_set(&route->metric, RTAX_HOPLIMIT, hop_limit); - if (r < 0) - return r; + r = route_metric_set(&route->metric, RTAX_QUICKACK, link->network->ndisc_quickack); if (r < 0) return r; diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 3732e7a0d7e..aced559f048 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5668,15 +5668,6 @@ class NetworkdRATests(unittest.TestCase, Utilities): self.assertIn('2002:da8:1:0:b47e:7975:fc7a:7d6e', output) self.assertIn('2002:da8:2:0:f689:561a:8eda:7443', output) - def check_router_hop_limit(self, hop_limit): - self.wait_route('client', rf'default via fe80::1034:56ff:fe78:9a99 proto ra .* hoplimit {hop_limit}', ipv='-6', timeout_sec=10) - - output = check_output('ip -6 route show dev client default via fe80::1034:56ff:fe78:9a99') - print(output) - self.assertIn(f'hoplimit {hop_limit}', output) - - self.check_ipv6_sysctl_attr('client', 'hop_limit', f'{hop_limit}') - def test_router_hop_limit(self): copy_network_unit('25-veth-client.netdev', '25-veth-router.netdev', @@ -5686,18 +5677,24 @@ class NetworkdRATests(unittest.TestCase, Utilities): '25-veth-router-hop-limit.network', '25-bridge99.network') start_networkd() - self.wait_online('client-p:enslaved', + self.wait_online('client:routable', 'client-p:enslaved', 'router:degraded', 'router-p:enslaved', 'bridge99:routable') - self.check_router_hop_limit(42) + self.check_ipv6_sysctl_attr('client', 'hop_limit', '42') with open(os.path.join(network_unit_dir, '25-veth-router-hop-limit.network'), mode='a', encoding='utf-8') as f: f.write('\n[IPv6SendRA]\nHopLimit=43\n') networkctl_reload() - self.check_router_hop_limit(43) + for _ in range(20): + output = read_ipv6_sysctl_attr('client', 'hop_limit') + if output == '43': + break + time.sleep(0.5) + + self.check_ipv6_sysctl_attr('client', 'hop_limit', '43') def test_router_preference(self): copy_network_unit('25-veth-client.netdev', From 33cab1d4ef7ff338546a39cff0c24d284c512510 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 11 Apr 2024 10:16:39 +0900 Subject: [PATCH 2/7] network/ndisc: introduce ndisc_request_router_route() Then, make ndisc_request_route() generic and usable for configuring routes based on both Router Advertisement and Redirect messages. Note, ndisc_request_router() never set lifetime, so the dropped comment in ndisc_request_redirect_route() is wrong. No functional change, just refactoring. --- src/network/networkd-ndisc.c | 52 +++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 73a88bd4096..3486621808b 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -177,27 +177,18 @@ static void ndisc_set_route_priority(Link *link, Route *route) { } } -static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { - struct in6_addr router; - bool is_new; +static int ndisc_request_route(Route *route, Link *link) { int r; assert(route); assert(link); assert(link->manager); assert(link->network); - assert(rt); - - r = sd_ndisc_router_get_sender_address(rt, &router); - if (r < 0) - return r; route->source = NETWORK_CONFIG_SOURCE_NDISC; - route->provider.in6 = router; + if (!route->table_set) route->table = link_get_ndisc_route_table(link); - if (!route->protocol_set) - route->protocol = RTPROT_RA; r = route_metric_set(&route->metric, RTAX_QUICKACK, link->network->ndisc_quickack); if (r < 0) @@ -248,7 +239,7 @@ static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { route->pref = pref_original; ndisc_set_route_priority(link, route); - is_new = route_get(link->manager, route, NULL) < 0; + bool is_new = route_get(link->manager, route, NULL) < 0; r = link_request_route(link, route, &link->ndisc_messages, ndisc_route_handler); if (r < 0) @@ -259,6 +250,23 @@ static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { return 0; } +static int ndisc_request_router_route(Route *route, Link *link, sd_ndisc_router *rt) { + int r; + + assert(route); + assert(link); + assert(rt); + + r = sd_ndisc_router_get_sender_address(rt, &route->provider.in6); + if (r < 0) + return r; + + if (!route->protocol_set) + route->protocol = RTPROT_RA; + + return ndisc_request_route(route, link); +} + static int ndisc_remove_route(Route *route, Link *link) { int r; @@ -401,6 +409,11 @@ static int ndisc_redirect_route_new(sd_ndisc_redirect *rd, Route **ret) { } route->dst.in6 = destination; route->dst_prefixlen = 128; + route->protocol = RTPROT_REDIRECT; + + r = sd_ndisc_redirect_get_sender_address(rd, &route->provider.in6); + if (r < 0) + return r; *ret = TAKE_PTR(route); return 0; @@ -430,12 +443,7 @@ static int ndisc_request_redirect_route(Link *link, sd_ndisc_redirect *rd) { if (r < 0) return r; - route->protocol = RTPROT_REDIRECT; - route->protocol_set = true; /* To make ndisc_request_route() not override the protocol. */ - - /* Redirect message does not have the lifetime, let's use the lifetime of the default router, and - * update the lifetime of the redirect route every time when we receive RA. */ - return ndisc_request_route(route, link, link->ndisc_default_router); + return ndisc_request_route(route, link); } static int ndisc_remove_redirect_route(Link *link, sd_ndisc_redirect *rd) { @@ -755,7 +763,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { route->nexthop.gw.in6 = gateway; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(route, link, rt); + r = ndisc_request_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request default route: %m"); } @@ -779,7 +787,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { route->pref = preference; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(route, link, rt); + r = ndisc_request_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request gateway: %m"); } @@ -1234,7 +1242,7 @@ static int ndisc_router_process_onlink_prefix(Link *link, sd_ndisc_router *rt) { route->pref = preference; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(route, link, rt); + r = ndisc_request_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request prefix route: %m"); @@ -1413,7 +1421,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) { route->lifetime_usec = lifetime_usec; if (lifetime_usec != 0) { - r = ndisc_request_route(route, link, rt); + r = ndisc_request_router_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request additional route: %m"); } else { From ef6495ebb2d06c6b15e3bff550d5376f0ddf6e73 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Apr 2024 14:58:31 +0900 Subject: [PATCH 3/7] network/ndisc: redirect routes do not have lifetime Hence, ndisc_router_update_redirect() does nothing. Let's remove it. Also, ndisc_request_route() does not set lifetime for the route, it is not necessary to set the third argument. --- src/network/networkd-ndisc.c | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 3486621808b..859217b14b1 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -591,23 +591,6 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { return ndisc_request_redirect_route(link, rd); } -static int ndisc_router_update_redirect(Link *link) { - int r, ret = 0; - - assert(link); - - /* Reconfigure redirect routes to update their lifetime. */ - - sd_ndisc_redirect *rd; - SET_FOREACH(rd, link->ndisc_redirects) { - r = ndisc_request_redirect_route(link, rd); - if (r < 0) - RET_GATHER(ret, log_link_warning_errno(link, r, "Failed to update lifetime of the Redirect route: %m")); - } - - return ret; -} - static int ndisc_drop_redirect(Link *link, const struct in6_addr *router, bool remove) { int r; @@ -2129,7 +2112,6 @@ static int ndisc_start_dhcp6_client(Link *link, sd_ndisc_router *rt) { static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) { struct in6_addr router; usec_t timestamp_usec; - bool is_default; int r; assert(link); @@ -2170,7 +2152,6 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) { r = ndisc_remember_default_router(link, rt); if (r < 0) return r; - is_default = r; r = ndisc_start_dhcp6_client(link, rt); if (r < 0) @@ -2204,12 +2185,7 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) { if (r < 0) return r; - if (is_default) { - r = ndisc_router_update_redirect(link); - if (r < 0) - return r; - - } else if (sd_ndisc_router_get_lifetime(rt, NULL) <= 0) { + if (sd_ndisc_router_get_lifetime(rt, NULL) <= 0) { r = ndisc_drop_redirect(link, &router, /* remove = */ true); if (r < 0) return r; From d9688518ff04890216382547f7d1f69cb4157d55 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Apr 2024 15:04:11 +0900 Subject: [PATCH 4/7] network/ndisc: drop ndisc_request_redirect_route() It is now called by only ndisc_redirect_handler(), and the check in ndisc_request_redirect_route() is redundant and already done by ndisc_redirect_verify_sender(). No functional change, just refactoring. --- src/network/networkd-ndisc.c | 37 +++++++++--------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 859217b14b1..b215fb4c1d0 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -419,33 +419,6 @@ static int ndisc_redirect_route_new(sd_ndisc_redirect *rd, Route **ret) { return 0; } -static int ndisc_request_redirect_route(Link *link, sd_ndisc_redirect *rd) { - struct in6_addr router, sender; - int r; - - assert(link); - assert(link->ndisc_default_router); - assert(rd); - - r = sd_ndisc_router_get_sender_address(link->ndisc_default_router, &router); - if (r < 0) - return r; - - r = sd_ndisc_redirect_get_sender_address(rd, &sender); - if (r < 0) - return r; - - if (!in6_addr_equal(&sender, &router)) - return 0; - - _cleanup_(route_unrefp) Route *route = NULL; - r = ndisc_redirect_route_new(rd, &route); - if (r < 0) - return r; - - return ndisc_request_route(route, link); -} - static int ndisc_remove_redirect_route(Link *link, sd_ndisc_redirect *rd) { _cleanup_(route_unrefp) Route *route = NULL; int r; @@ -578,17 +551,25 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { /* OK, the Redirect message is sent from the current default router. */ + /* First, drop conflicting redirect route, if exists. */ r = ndisc_redirect_drop_conflict(link, rd); if (r < 0) return r; + /* Then, remember the received message. */ r = set_ensure_put(&link->ndisc_redirects, &ndisc_redirect_hash_ops, rd); if (r < 0) return r; sd_ndisc_redirect_ref(rd); - return ndisc_request_redirect_route(link, rd); + /* Finally, request the corresponding route. */ + _cleanup_(route_unrefp) Route *route = NULL; + r = ndisc_redirect_route_new(rd, &route); + if (r < 0) + return r; + + return ndisc_request_route(route, link); } static int ndisc_drop_redirect(Link *link, const struct in6_addr *router, bool remove) { From f76814757ddbaf7f54d8828661b5978f066dea25 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Apr 2024 14:52:57 +0900 Subject: [PATCH 5/7] network/ndisc: split out ndisc_redirect_verify_sender() No functional change, preparation for later commits. --- src/network/networkd-ndisc.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index b215fb4c1d0..c2c15c48e8f 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -510,18 +510,14 @@ static int ndisc_redirect_drop_conflict(Link *link, sd_ndisc_redirect *rd) { return ndisc_remove_redirect_route(link, existing); } -static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { +static int ndisc_redirect_verify_sender(Link *link, sd_ndisc_redirect *rd) { struct in6_addr router, sender; usec_t lifetime_usec, now_usec; int r; assert(link); - assert(link->network); assert(rd); - if (!link->network->ndisc_use_redirect) - return 0; - /* Ignore all Redirect messages from non-default router. */ if (!link->ndisc_default_router) @@ -546,10 +542,23 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { if (r < 0) return r; - if (!in6_addr_equal(&sender, &router)) - return 0; /* The redirect message is sent from a non-default router. */ + /* The sender must be the default router. */ + return in6_addr_equal(&sender, &router); +} - /* OK, the Redirect message is sent from the current default router. */ +static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { + int r; + + assert(link); + assert(link->network); + assert(rd); + + if (!link->network->ndisc_use_redirect) + return 0; + + r = ndisc_redirect_verify_sender(link, rd); + if (r <= 0) + return r; /* First, drop conflicting redirect route, if exists. */ r = ndisc_redirect_drop_conflict(link, rd); From 9944629eeeeadd02b7e79592d213707696ea9347 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Apr 2024 15:07:30 +0900 Subject: [PATCH 6/7] network/ndisc: fix verification of sender of Redirect message The sender must be the first-hop router of the destination. Previously, we only accepted Redirect messages whose sender is the current default router with the highest priority. See RFC 4861 section 8.1 for more details. Fixes #31981. --- src/network/networkd-ndisc.c | 104 +++++++++++--------- test/test-network/systemd-networkd-tests.py | 24 +++-- 2 files changed, 70 insertions(+), 58 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index c2c15c48e8f..7f5577f6ae3 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -511,37 +511,52 @@ static int ndisc_redirect_drop_conflict(Link *link, sd_ndisc_redirect *rd) { } static int ndisc_redirect_verify_sender(Link *link, sd_ndisc_redirect *rd) { + sd_ndisc_redirect *existing; struct in6_addr router, sender; - usec_t lifetime_usec, now_usec; int r; assert(link); assert(rd); - /* Ignore all Redirect messages from non-default router. */ - - if (!link->ndisc_default_router) - return 0; - - r = sd_ndisc_router_get_lifetime_timestamp(link->ndisc_default_router, CLOCK_BOOTTIME, &lifetime_usec); - if (r < 0) - return r; - - r = sd_event_now(link->manager->event, CLOCK_BOOTTIME, &now_usec); - if (r < 0) - return r; - - if (lifetime_usec <= now_usec) - return 0; /* The default router is outdated. Ignore the redirect message. */ - - r = sd_ndisc_router_get_sender_address(link->ndisc_default_router, &router); - if (r < 0) - return r; + /* RFC 4861 section 8.1 + * The IP source address of the Redirect is the same as the current first-hop router for the specified + * ICMP Destination Address. */ r = sd_ndisc_redirect_get_sender_address(rd, &sender); if (r < 0) return r; + existing = set_get(link->ndisc_redirects, rd); + if (existing) { + struct in6_addr target, dest; + + /* If we have received Redirect message for the host, the sender must be the previous target. */ + + r = sd_ndisc_redirect_get_target_address(existing, &target); + if (r < 0) + return r; + + if (in6_addr_equal(&sender, &target)) + return true; + + /* If the existing redirect route is on-link, that is, the destination and target address are + * equivalent, then also accept Redirect message from the current default router. This is not + * mentioned by the RFC, but without this, we cannot update on-link redirect route. */ + r = sd_ndisc_redirect_get_destination_address(existing, &dest); + if (r < 0) + return r; + + if (!in6_addr_equal(&dest, &target)) + return false; + } + + if (!link->ndisc_default_router) + return false; + + r = sd_ndisc_router_get_sender_address(link->ndisc_default_router, &router); + if (r < 0) + return r; + /* The sender must be the default router. */ return in6_addr_equal(&sender, &router); } @@ -581,38 +596,32 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { return ndisc_request_route(route, link); } -static int ndisc_drop_redirect(Link *link, const struct in6_addr *router, bool remove) { - int r; +static int ndisc_drop_redirect(Link *link, const struct in6_addr *router) { + int r, ret = 0; assert(link); - /* If the router is purged, then drop the redirect routes configured with the Redirect message sent - * by the router. */ - - if (!router) - return 0; - sd_ndisc_redirect *rd; SET_FOREACH(rd, link->ndisc_redirects) { - struct in6_addr sender; + if (router) { + struct in6_addr target; - r = sd_ndisc_redirect_get_sender_address(rd, &sender); - if (r < 0) - return r; - - if (!in6_addr_equal(&sender, router)) - continue; - - if (remove) { - r = ndisc_remove_redirect_route(link, rd); + r = sd_ndisc_redirect_get_target_address(rd, &target); if (r < 0) return r; + + if (!in6_addr_equal(&target, router)) + continue; } + r = ndisc_remove_redirect_route(link, rd); + if (r < 0) + RET_GATHER(ret, log_link_warning_errno(link, r, "Failed to remove redirect route, ignoring: %m")); + sd_ndisc_redirect_unref(set_remove(link->ndisc_redirects, rd)); } - return 0; + return ret; } static int ndisc_update_redirect_sender(Link *link, const struct in6_addr *original_address, const struct in6_addr *current_address) { @@ -1888,10 +1897,6 @@ static int ndisc_drop_outdated(Link *link, const struct in6_addr *router, usec_t if (r < 0) RET_GATHER(ret, log_link_warning_errno(link, r, "Failed to drop outdated default router, ignoring: %m")); - r = ndisc_drop_redirect(link, router, /* remove = */ false); - if (r < 0) - RET_GATHER(ret, log_link_warning_errno(link, r, "Failed to drop outdated Redirect messages, ignoring: %m")); - SET_FOREACH(route, link->manager->routes) { if (route->source != NETWORK_CONFIG_SOURCE_NDISC) continue; @@ -1899,6 +1904,9 @@ static int ndisc_drop_outdated(Link *link, const struct in6_addr *router, usec_t if (route->nexthop.ifindex != link->ifindex) continue; + if (route->protocol == RTPROT_REDIRECT) + continue; /* redirect route will be dropped by ndisc_drop_redirect(). */ + if (route->lifetime_usec > timestamp_usec) continue; /* the route is still valid */ @@ -2175,11 +2183,8 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) { if (r < 0) return r; - if (sd_ndisc_router_get_lifetime(rt, NULL) <= 0) { - r = ndisc_drop_redirect(link, &router, /* remove = */ true); - if (r < 0) - return r; - } + if (sd_ndisc_router_get_lifetime(rt, NULL) <= 0) + (void) ndisc_drop_redirect(link, &router); if (link->ndisc_messages == 0) link->ndisc_configured = true; @@ -2210,6 +2215,8 @@ static int ndisc_neighbor_handle_non_router_message(Link *link, sd_ndisc_neighbo return r; (void) ndisc_drop_outdated(link, /* router = */ &address, /* timestamp_usec = */ USEC_INFINITY); + (void) ndisc_drop_redirect(link, &address); + return 0; } @@ -2490,6 +2497,7 @@ void ndisc_flush(Link *link) { /* Remove all addresses, routes, RDNSS, DNSSL, and Captive Portal entries, without exception. */ (void) ndisc_drop_outdated(link, /* router = */ NULL, /* timestamp_usec = */ USEC_INFINITY); + (void) ndisc_drop_redirect(link, /* router = */ NULL); link->ndisc_rdnss = set_free(link->ndisc_rdnss); link->ndisc_dnssl = set_free(link->ndisc_dnssl); diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index aced559f048..0c12c432913 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5551,17 +5551,17 @@ class NetworkdRATests(unittest.TestCase, Utilities): # Introduce two redirect routes. check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address 2002:da8:1:1:1a:2b:3c:4d --redirect-destination 2002:da8:1:1:1a:2b:3c:4d') - check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address 2002:da8:1::1 --redirect-destination 2002:da8:1:2:1a:2b:3c:4d') - self.wait_route('veth99', r'2002:da8:1:1:1a:2b:3c:4d proto redirect', ipv='-6', timeout_sec=10) - self.wait_route('veth99', r'2002:da8:1:2:1a:2b:3c:4d via 2002:da8:1::1 proto redirect', ipv='-6', timeout_sec=10) + check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address 2002:da8:1:2:1a:2b:3c:4d --redirect-destination 2002:da8:1:2:1a:2b:3c:4d') + self.wait_route('veth99', '2002:da8:1:1:1a:2b:3c:4d proto redirect', ipv='-6', timeout_sec=10) + self.wait_route('veth99', '2002:da8:1:2:1a:2b:3c:4d proto redirect', ipv='-6', timeout_sec=10) # Change the target address of the redirects. - check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address 2002:da8:1::2 --redirect-destination 2002:da8:1:1:1a:2b:3c:4d') - check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address 2002:da8:1::3 --redirect-destination 2002:da8:1:2:1a:2b:3c:4d') - self.wait_route_dropped('veth99', r'2002:da8:1:1:1a:2b:3c:4d proto redirect', ipv='-6', timeout_sec=10) - self.wait_route_dropped('veth99', r'2002:da8:1:2:1a:2b:3c:4d via 2002:da8:1::1 proto redirect', ipv='-6', timeout_sec=10) - self.wait_route('veth99', r'2002:da8:1:1:1a:2b:3c:4d via 2002:da8:1::2 proto redirect', ipv='-6', timeout_sec=10) - self.wait_route('veth99', r'2002:da8:1:2:1a:2b:3c:4d via 2002:da8:1::3 proto redirect', ipv='-6', timeout_sec=10) + check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address fe80::1 --redirect-destination 2002:da8:1:1:1a:2b:3c:4d') + check_output(f'{test_ndisc_send} --interface veth-peer --type redirect --target-address fe80::2 --redirect-destination 2002:da8:1:2:1a:2b:3c:4d') + self.wait_route_dropped('veth99', '2002:da8:1:1:1a:2b:3c:4d proto redirect', ipv='-6', timeout_sec=10) + self.wait_route_dropped('veth99', '2002:da8:1:2:1a:2b:3c:4d proto redirect', ipv='-6', timeout_sec=10) + self.wait_route('veth99', '2002:da8:1:1:1a:2b:3c:4d via fe80::1 proto redirect', ipv='-6', timeout_sec=10) + self.wait_route('veth99', '2002:da8:1:2:1a:2b:3c:4d via fe80::2 proto redirect', ipv='-6', timeout_sec=10) # Send Neighbor Advertisement without the router flag to announce the default router is not available anymore. # Then, verify that all redirect routes and the default route are dropped. @@ -5569,9 +5569,13 @@ class NetworkdRATests(unittest.TestCase, Utilities): veth_peer_ipv6ll = re.search('fe80:[:0-9a-f]*', output).group() print(f'veth-peer IPv6LL address: {veth_peer_ipv6ll}') check_output(f'{test_ndisc_send} --interface veth-peer --type neighbor-advertisement --target-address {veth_peer_ipv6ll} --is-router no') - self.wait_route_dropped('veth99', 'proto redirect', ipv='-6', timeout_sec=10) self.wait_route_dropped('veth99', 'proto ra', ipv='-6', timeout_sec=10) + output = check_output('ip -6 route show dev veth99') + print(output) + self.assertIn('2002:da8:1:1:1a:2b:3c:4d via fe80::1 proto redirect', output) + self.assertIn('2002:da8:1:2:1a:2b:3c:4d via fe80::2 proto redirect', output) + def check_ndisc_mtu(self, mtu): for _ in range(20): output = read_ipv6_sysctl_attr('veth99', 'mtu') From bffa1c48895fbe8cb86ffe47420dc33d96aba121 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Apr 2024 15:36:59 +0900 Subject: [PATCH 7/7] sd-ndisc-redirect: fix verification of target address See RFC 4861 section 8.1. --- src/libsystemd-network/sd-ndisc-redirect.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/sd-ndisc-redirect.c b/src/libsystemd-network/sd-ndisc-redirect.c index 3e21b76fffe..a1fceb2dff3 100644 --- a/src/libsystemd-network/sd-ndisc-redirect.c +++ b/src/libsystemd-network/sd-ndisc-redirect.c @@ -55,14 +55,19 @@ int ndisc_redirect_parse(sd_ndisc *nd, sd_ndisc_redirect *rd) { rd->target_address = a->nd_rd_target; rd->destination_address = a->nd_rd_dst; - if (in6_addr_is_null(&rd->target_address) || in6_addr_is_multicast(&rd->target_address)) - return log_ndisc_errno(nd, SYNTHETIC_ERRNO(EBADMSG), - "Received Redirect message with an invalid target address, ignoring datagram: %m"); - + /* RFC 4861 section 8.1 + * The ICMP Destination Address field in the redirect message does not contain a multicast address. */ if (in6_addr_is_null(&rd->destination_address) || in6_addr_is_multicast(&rd->destination_address)) return log_ndisc_errno(nd, SYNTHETIC_ERRNO(EBADMSG), "Received Redirect message with an invalid destination address, ignoring datagram: %m"); + /* RFC 4861 section 8.1 + * The ICMP Target Address is either a link-local address (when redirected to a router) or the same + * as the ICMP Destination Address (when redirected to the on-link destination). */ + if (!in6_addr_is_link_local(&rd->target_address) && !in6_addr_equal(&rd->target_address, &rd->destination_address)) + return log_ndisc_errno(nd, SYNTHETIC_ERRNO(EBADMSG), + "Received Redirect message with an invalid target address, ignoring datagram: %m"); + r = ndisc_parse_options(rd->packet, &rd->options); if (r < 0) return log_ndisc_errno(nd, r, "Failed to parse NDisc options in Redirect message, ignoring datagram: %m");