SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

network: don't forget old RAs when a new one arrives

Browse Source 
IPv6 Neighbor Discovery lets us autoconfigure a link's IPv6 addresses,
routes, DNS servers, and DNS search domains by listening for Router
Advertisement (RA) packets broadcast by one or more routers on the link.
Each RA can contain zero or more "options," each describing one piece of
configuration (e.g. a single route).

Currently, when we receive an RA from a router, we delete any addresses,
routes, etc. that originated from that router's previous RAs unless
they're also present as options in the new RA.

That behavior is a violation of RFC 4861[1]. In Section 9, the RFC
states that

    Senders MAY send a subset of options in different packets. ... Thus,
    a receiver MUST NOT associate any action with the absence of an
    option in a particular packet. This protocol specifies that
    receivers should only act on the expiration of timers and on the
    information that is received in the packets.

Several other passages in the RFC reiterate this. Section 6.2.3:

    A router MAY choose not to include some or all options when sending
    unsolicited Router Advertisements.

Section 6.3.4:

    Hosts accept the union of all received information; the receipt of a
    Router Advertisement MUST NOT invalidate all information received in
    a previous advertisement or from another source.

At least one consumer router in production today, the Google Nest Wifi,
often sends RAs that omit its global IPv6 prefix. When current versions
of systemd-networkd receive those RAs, they immediately delete the
interface's global IPv6 address, which breaks IPv6 connectivity.

Fix the issue by removing the invalidation logic entirely. It's not
needed at all, since we already invalidate addresses, routes, and DNS
configuration when the interface goes down or their lifetimes expire.

This fix does have the side effect of preventing changes to the .network
file (e.g. denylisted prefixes, whether to add routes from RAs) from
taking effect as soon as a new RA arrives. Instead, a full interface
reconfiguration is needed. But triggering those changes on RA receipt
was already rather arbitrary and out of the administrator's control, so
I think this change is fine.

commit 69203fba70 ("network: ndisc: remove old addresses and routes
after at least one SLAAC address becomes ready") introduced this
behavior. commit 50550722e3 fixed it partially, by preventing one
router's RAs from invalidating another router's configuration.

[1] https://www.rfc-editor.org/rfc/rfc4861

Fixes: 69203fba70 ("network: ndisc: remove old addresses and routes after at least one SLAAC address becomes ready")
This commit is contained in:
Thomas Hebb
2022-10-03 09:59:37 +09:00
committed by Yu Watanabe
parent 64ebc0da03
commit 2ccada8dc4
9 changed files with 14 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/network/networkd-address.c
+1 -5
View File
@@ -160,7 +160,7 @@ bool address_is_ready(const Address *a) {
return true;
}
void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in6_addr *router) {
void link_mark_addresses(Link *link, NetworkConfigSource source) {
Address *a;
assert(link);
@@ -169,10 +169,6 @@ void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in
if (a->source != source)
continue;
if (source == NETWORK_CONFIG_SOURCE_NDISC &&
router && !in6_addr_equal(router, &a->provider.in6))
continue;
address_mark(a);
}
}
src/network/networkd-address.h
+1 -1
View File
@@ -120,7 +120,7 @@ int address_compare_func(const Address *a1, const Address *a2);
DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Address, address);
void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in6_addr *router);
void link_mark_addresses(Link *link, NetworkConfigSource source);
CONFIG_PARSER_PROTOTYPE(config_parse_address);
CONFIG_PARSER_PROTOTYPE(config_parse_broadcast);
src/network/networkd-dhcp-prefix-delegation.c
+2 -2
View File
@@ -572,8 +572,8 @@ static int dhcp_pd_prepare(Link *link) {
if (link_radv_enabled(link) && link->network->dhcp_pd_announce && !link->radv)
return 0;
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP_PD, NULL);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP_PD, NULL);
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP_PD);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP_PD);
return 1;
}
src/network/networkd-dhcp4.c
+2 -2
View File
@@ -899,8 +899,8 @@ static int dhcp4_request_address_and_routes(Link *link, bool announce) {
assert(link);
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP4, NULL);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP4, NULL);
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP4);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP4);
r = dhcp4_request_address(link, announce);
if (r < 0)
src/network/networkd-dhcp6.c
+2 -2
View File
@@ -304,8 +304,8 @@ static int dhcp6_lease_ip_acquired(sd_dhcp6_client *client, Link *link) {
sd_dhcp6_lease *lease;
int r;
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP6, NULL);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP6, NULL);
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_DHCP6);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_DHCP6);
r = sd_dhcp6_client_get_lease(client, &lease);
if (r < 0)
src/network/networkd-ndisc.c
+4 -108
View File
@@ -72,72 +72,6 @@ void network_adjust_ipv6_accept_ra(Network *network) {
network->ndisc_deny_listed_route_prefix = set_free_free(network->ndisc_deny_listed_route_prefix);
}
static int ndisc_remove(Link *link, struct in6_addr *router) {
bool updated = false;
NDiscDNSSL *dnssl;
NDiscRDNSS *rdnss;
Address *address;
Route *route;
int k, r = 0;
assert(link);
SET_FOREACH(route, link->routes) {
if (route->source != NETWORK_CONFIG_SOURCE_NDISC)
continue;
if (!route_is_marked(route))
continue;
if (router && !in6_addr_equal(router, &route->provider.in6))
continue;
k = route_remove(route);
if (k < 0)
r = k;
route_cancel_request(route, link);
}
SET_FOREACH(address, link->addresses) {
if (address->source != NETWORK_CONFIG_SOURCE_NDISC)
continue;
if (!address_is_marked(address))
continue;
if (router && !in6_addr_equal(router, &address->provider.in6))
continue;
k = address_remove(address);
if (k < 0)
r = k;
address_cancel_request(address);
}
SET_FOREACH(rdnss, link->ndisc_rdnss) {
if (!rdnss->marked)
continue;
if (router && !in6_addr_equal(router, &rdnss->router))
continue;
free(set_remove(link->ndisc_rdnss, rdnss));
updated = true;
}
SET_FOREACH(dnssl, link->ndisc_dnssl) {
if (!dnssl->marked)
continue;
if (router && !in6_addr_equal(router, &dnssl->router))
continue;
free(set_remove(link->ndisc_dnssl, dnssl));
updated = true;
}
if (updated)
link_dirty(link);
return r;
}
static int ndisc_check_ready(Link *link);
static int ndisc_address_ready_callback(Address *address) {
@@ -156,7 +90,6 @@ static int ndisc_address_ready_callback(Address *address) {
static int ndisc_check_ready(Link *link) {
bool found = false, ready = false;
Address *address;
int r;
assert(link);
@@ -189,10 +122,6 @@ static int ndisc_check_ready(Link *link) {
link->ndisc_configured = true;
log_link_debug(link, "SLAAC addresses and routes set.");
r = ndisc_remove(link, NULL);
if (r < 0)
return r;
link_check_ready(link);
return 0;
}
@@ -216,7 +145,6 @@ static int ndisc_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request
static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) {
_cleanup_(route_freep) Route *route = in;
struct in6_addr router;
Route *existing;
int r;
assert(route);
@@ -236,10 +164,8 @@ static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) {
if (!route->protocol_set)
route->protocol = RTPROT_RA;
if (route_get(NULL, link, route, &existing) < 0)
if (route_get(NULL, link, route, NULL) < 0)
link->ndisc_configured = false;
else
route_unmark(existing);
return link_request_route(link, TAKE_PTR(route), true, &link->ndisc_messages,
ndisc_route_handler, NULL);
@@ -264,7 +190,6 @@ static int ndisc_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques
static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) {
_cleanup_(address_freep) Address *address = in;
struct in6_addr router;
Address *existing;
int r;
assert(address);
@@ -282,10 +207,8 @@ static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) {
if (r < 0)
return r;
if (address_get(link, address, &existing) < 0)
if (address_get(link, address, NULL) < 0)
link->ndisc_configured = false;
else
address_unmark(existing);
return link_request_address(link, TAKE_PTR(address), true, &link->ndisc_messages,
ndisc_address_handler, NULL);
@@ -723,7 +646,6 @@ static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) {
rdnss = set_get(link->ndisc_rdnss, &d);
if (rdnss) {
rdnss->marked = false;
rdnss->router = router;
rdnss->lifetime_usec = lifetime_usec;
continue;
@@ -822,7 +744,6 @@ static int ndisc_router_process_dnssl(Link *link, sd_ndisc_router *rt) {
dnssl = set_get(link->ndisc_dnssl, s);
if (dnssl) {
dnssl->marked = false;
dnssl->router = router;
dnssl->lifetime_usec = lifetime_usec;
continue;
@@ -893,25 +814,6 @@ static int ndisc_router_process_options(Link *link, sd_ndisc_router *rt) {
}
}
static void ndisc_mark(Link *link, const struct in6_addr *router) {
NDiscRDNSS *rdnss;
NDiscDNSSL *dnssl;
assert(link);
assert(router);
link_mark_addresses(link, NETWORK_CONFIG_SOURCE_NDISC, router);
link_mark_routes(link, NETWORK_CONFIG_SOURCE_NDISC, router);
SET_FOREACH(rdnss, link->ndisc_rdnss)
if (in6_addr_equal(&rdnss->router, router))
rdnss->marked = true;
SET_FOREACH(dnssl, link->ndisc_dnssl)
if (in6_addr_equal(&dnssl->router, router))
dnssl->marked = true;
}
static int ndisc_start_dhcp6_client(Link *link, sd_ndisc_router *rt) {
int r;
@@ -978,8 +880,6 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) {
return 0;
}
ndisc_mark(link, &router);
r = ndisc_start_dhcp6_client(link, rt);
if (r < 0)
return r;
@@ -992,13 +892,9 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) {
if (r < 0)
return r;
if (link->ndisc_messages == 0) {
if (link->ndisc_messages == 0)
link->ndisc_configured = true;
r = ndisc_remove(link, &router);
if (r < 0)
return r;
} else
else
log_link_debug(link, "Setting SLAAC addresses and router.");
if (!link->ndisc_configured)
src/network/networkd-ndisc.h
-4
View File
@@ -16,8 +16,6 @@ typedef enum IPv6AcceptRAStartDHCP6Client {
} IPv6AcceptRAStartDHCP6Client;
typedef struct NDiscRDNSS {
/* Used when GC'ing old DNS servers when configuration changes. */
bool marked;
struct in6_addr router;
/* This is an absolute point in time, and NOT a timespan/duration.
* Must be specified with clock_boottime_or_monotonic(). */
@@ -26,8 +24,6 @@ typedef struct NDiscRDNSS {
} NDiscRDNSS;
typedef struct NDiscDNSSL {
/* Used when GC'ing old domains when configuration changes. */
bool marked;
struct in6_addr router;
/* This is an absolute point in time, and NOT a timespan/duration.
* Must be specified with clock_boottime_or_monotonic(). */
src/network/networkd-route.c
+1 -5
View File
@@ -534,7 +534,7 @@ static int route_convert(Manager *manager, const Route *route, ConvertedRoutes *
return 1;
}
void link_mark_routes(Link *link, NetworkConfigSource source, const struct in6_addr *router) {
void link_mark_routes(Link *link, NetworkConfigSource source) {
Route *route;
assert(link);
@@ -543,10 +543,6 @@ void link_mark_routes(Link *link, NetworkConfigSource source, const struct in6_a
if (route->source != source)
continue;
if (source == NETWORK_CONFIG_SOURCE_NDISC &&
router && !in6_addr_equal(router, &route->provider.in6))
continue;
route_mark(route);
}
}
src/network/networkd-route.h
+1 -1
View File
@@ -109,7 +109,7 @@ int network_add_default_route_on_device(Network *network);
void network_drop_invalid_routes(Network *network);
DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Route, route);
void link_mark_routes(Link *link, NetworkConfigSource source, const struct in6_addr *router);
void link_mark_routes(Link *link, NetworkConfigSource source);
CONFIG_PARSER_PROTOTYPE(config_parse_gateway);
CONFIG_PARSER_PROTOTYPE(config_parse_preferred_src);