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"); diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index f0c58e8842b..7f5577f6ae3 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -177,47 +177,19 @@ 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; +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; - - 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; @@ -267,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) @@ -278,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; @@ -420,43 +409,16 @@ 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; } -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; - - 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); -} - static int ndisc_remove_redirect_route(Link *link, sd_ndisc_redirect *rd) { _cleanup_(route_unrefp) Route *route = NULL; int r; @@ -548,9 +510,58 @@ 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) { + sd_ndisc_redirect *existing; struct in6_addr router, sender; - usec_t lifetime_usec, now_usec; + int r; + + assert(link); + assert(rd); + + /* 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); +} + +static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { int r; assert(link); @@ -560,97 +571,57 @@ static int ndisc_redirect_handler(Link *link, sd_ndisc_redirect *rd) { if (!link->network->ndisc_use_redirect) return 0; - /* 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) + r = ndisc_redirect_verify_sender(link, rd); + 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; - - r = sd_ndisc_redirect_get_sender_address(rd, &sender); - if (r < 0) - return r; - - if (!in6_addr_equal(&sender, &router)) - return 0; /* The redirect message is sent from a non-default router. */ - - /* 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_router_update_redirect(Link *link) { +static int ndisc_drop_redirect(Link *link, const struct in6_addr *router) { 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")); - } + if (router) { + struct in6_addr target; - return ret; -} - -static int ndisc_drop_redirect(Link *link, const struct in6_addr *router, bool remove) { - int r; - - 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; - - 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) { @@ -774,7 +745,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"); } @@ -798,7 +769,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"); } @@ -1253,7 +1224,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"); @@ -1432,7 +1403,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 { @@ -1926,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; @@ -1937,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 */ @@ -2140,7 +2110,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); @@ -2181,7 +2150,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) @@ -2215,16 +2183,8 @@ 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) { - 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; @@ -2255,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; } @@ -2535,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 3732e7a0d7e..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') @@ -5668,15 +5672,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 +5681,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',