From 4c9bb70854745b34a3d63ce3c27afd769b1350c1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 2 Dec 2021 08:31:55 +0900 Subject: [PATCH 1/4] parse-util: refuse leading white space in port number When parse_ip_port() is directly used in a conf parser, then that's fine, as the rvalue is already truncated. When parse_ip_port() is used when e.g. parsing IP address with port, then we should really refuse white space after colon. --- src/basic/parse-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index d03a6eeb05..2888ab6523 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -644,7 +644,7 @@ int parse_ip_port(const char *s, uint16_t *ret) { uint16_t l; int r; - r = safe_atou16(s, &l); + r = safe_atou16_full(s, SAFE_ATO_REFUSE_LEADING_WHITESPACE, &l); if (r < 0) return r; From 4a897d29f1b1aa760f9a938acf98390c0d285482 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 29 Nov 2021 22:07:29 +0900 Subject: [PATCH 2/4] network/wireguard: do not resolve Endpoint= if an IP address is specified Also verify the domain name and port. --- src/network/netdev/wireguard.c | 106 ++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 40 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index f254b05f86..2a802a3739 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -12,6 +12,7 @@ #include "sd-resolve.h" #include "alloc-util.h" +#include "dns-domain.h" #include "event-util.h" #include "fd-util.h" #include "fileio.h" @@ -723,65 +724,90 @@ int config_parse_wireguard_endpoint( void *userdata) { _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; - const char *begin, *end; + _cleanup_free_ char *host = NULL; + union in_addr_union addr; + const char *p; + uint16_t port; Wireguard *w; - size_t len; - int r; + int family, r; - assert(data); + assert(filename); assert(rvalue); + assert(userdata); - w = WIREGUARD(data); + w = WIREGUARD(userdata); assert(w); - if (rvalue[0] == '[') { - begin = &rvalue[1]; - end = strchr(rvalue, ']'); - if (!end) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Unable to find matching brace of endpoint, ignoring assignment: %s", - rvalue); - return 0; - } - len = end - begin; - ++end; - if (*end != ':' || !*(end + 1)) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Unable to find port of endpoint, ignoring assignment: %s", - rvalue); - return 0; - } - ++end; - } else { - begin = rvalue; - end = strrchr(rvalue, ':'); - if (!end || !*(end + 1)) { - log_syntax(unit, LOG_WARNING, filename, line, 0, - "Unable to find port of endpoint, ignoring assignment: %s", - rvalue); - return 0; - } - len = end - begin; - ++end; - } - r = wireguard_peer_new_static(w, filename, section_line, &peer); if (r < 0) return log_oom(); - r = free_and_strndup(&peer->endpoint_host, begin, len); - if (r < 0) + r = in_addr_port_ifindex_name_from_string_auto(rvalue, &family, &addr, &port, NULL, NULL); + if (r >= 0) { + if (family == AF_INET) + peer->endpoint.in = (struct sockaddr_in) { + .sin_family = AF_INET, + .sin_addr = addr.in, + .sin_port = htobe16(port), + }; + else if (family == AF_INET6) + peer->endpoint.in6 = (struct sockaddr_in6) { + .sin6_family = AF_INET6, + .sin6_addr = addr.in6, + .sin6_port = htobe16(port), + }; + else + assert_not_reached(); + + peer->endpoint_host = mfree(peer->endpoint_host); + peer->endpoint_port = mfree(peer->endpoint_port); + set_remove(w->peers_with_unresolved_endpoint, peer); + + TAKE_PTR(peer); + return 0; + } + + p = strrchr(rvalue, ':'); + if (!p) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Unable to find port of endpoint, ignoring assignment: %s", + rvalue); + return 0; + } + + host = strndup(rvalue, p - rvalue); + if (!host) return log_oom(); - r = free_and_strdup(&peer->endpoint_port, end); + if (!dns_name_is_valid(host)) { + log_syntax(unit, LOG_WARNING, filename, line, 0, + "Invalid domain name of endpoint, ignoring assignment: %s", + rvalue); + return 0; + } + + p++; + r = parse_ip_port(p, &port); + if (r < 0) { + log_syntax(unit, LOG_WARNING, filename, line, r, + "Invalid port of endpoint, ignoring assignment: %s", + rvalue); + return 0; + } + + peer->endpoint = (union sockaddr_union) {}; + + free_and_replace(peer->endpoint_host, host); + + r = free_and_strdup(&peer->endpoint_port, p); if (r < 0) return log_oom(); r = set_ensure_put(&w->peers_with_unresolved_endpoint, NULL, peer); if (r < 0) return log_oom(); - TAKE_PTR(peer); /* The peer may already have been in the hash map, that is fine too. */ + TAKE_PTR(peer); /* The peer may already have been in the hash map, that is fine too. */ return 0; } From 8bf7e3b61cf5d85c07e4d537192f56f31db70141 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 29 Nov 2021 21:21:21 +0900 Subject: [PATCH 3/4] network/wireguard: cleanups for resolving endpoints This makes - drop peers_with_unresolved_endpoint and peers_with_failed_endpoint, - drop destroy handler for sd_resolve_query, and manage each query by peer, - add random fluctuation to the timeout for retry handler, - retry timer event source is now managed by peer, - use sd_event_source_disable_unref(). --- src/network/netdev/wireguard.c | 190 ++++++++++++++++----------------- src/network/netdev/wireguard.h | 13 +-- 2 files changed, 98 insertions(+), 105 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 2a802a3739..2c60ba197d 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -25,12 +25,14 @@ #include "networkd-util.h" #include "parse-util.h" #include "path-util.h" +#include "random-util.h" #include "resolve-private.h" #include "string-util.h" #include "strv.h" #include "wireguard.h" -static void resolve_endpoints(NetDev *netdev); +static void wireguard_resolve_endpoints(NetDev *netdev); +static int peer_resolve_endpoint(WireguardPeer *peer); static WireguardPeer* wireguard_peer_free(WireguardPeer *peer) { WireguardIPmask *mask; @@ -41,9 +43,6 @@ static WireguardPeer* wireguard_peer_free(WireguardPeer *peer) { if (peer->wireguard) { LIST_REMOVE(peers, peer->wireguard->peers, peer); - set_remove(peer->wireguard->peers_with_unresolved_endpoint, peer); - set_remove(peer->wireguard->peers_with_failed_endpoint, peer); - if (peer->section) hashmap_remove(peer->wireguard->peers_by_section, peer->section); } @@ -60,6 +59,9 @@ static WireguardPeer* wireguard_peer_free(WireguardPeer *peer) { free(peer->preshared_key_file); explicit_bzero_safe(peer->preshared_key, WG_KEY_LEN); + sd_event_source_disable_unref(peer->resolve_retry_event_source); + sd_resolve_query_unref(peer->resolve_query); + return mfree(peer); } @@ -288,7 +290,8 @@ static int wireguard_set_interface(NetDev *netdev) { return 0; } -static void wireguard_peer_destroy_callback(WireguardPeer *peer) { +static int on_resolve_retry(sd_event_source *s, usec_t usec, void *userdata) { + WireguardPeer *peer = userdata; NetDev *netdev; assert(peer); @@ -296,134 +299,132 @@ static void wireguard_peer_destroy_callback(WireguardPeer *peer) { netdev = NETDEV(peer->wireguard); - if (section_is_invalid(peer->section)) - wireguard_peer_free(peer); - - netdev_unref(netdev); -} - -static int on_resolve_retry(sd_event_source *s, usec_t usec, void *userdata) { - NetDev *netdev = userdata; - Wireguard *w; - - assert(netdev); - w = WIREGUARD(netdev); - assert(w); - if (!netdev_is_managed(netdev)) return 0; - assert(set_isempty(w->peers_with_unresolved_endpoint)); - - SWAP_TWO(w->peers_with_unresolved_endpoint, w->peers_with_failed_endpoint); - - resolve_endpoints(netdev); + peer->resolve_query = sd_resolve_query_unref(peer->resolve_query); + (void) peer_resolve_endpoint(peer); return 0; } -/* - * Given the number of retries this function will return will an exponential - * increasing time in milliseconds to wait starting at 200ms and capped at 25 seconds. - */ -static int exponential_backoff_milliseconds(unsigned n_retries) { - return (2 << MIN(n_retries, 7U)) * 100 * USEC_PER_MSEC; +static usec_t peer_next_resolve_usec(WireguardPeer *peer) { + usec_t usec; + + /* Given the number of retries this function will return an exponential increasing amount of + * milliseconds to wait starting at 200ms and capped at 25 seconds. */ + + assert(peer); + + usec = (2 << MIN(peer->n_retries, 7U)) * 100 * USEC_PER_MSEC; + + return random_u64_range(usec / 10) + usec * 9 / 10; } -static int wireguard_resolve_handler(sd_resolve_query *q, - int ret, - const struct addrinfo *ai, - WireguardPeer *peer) { +static int wireguard_peer_resolve_handler( + sd_resolve_query *q, + int ret, + const struct addrinfo *ai, + void *userdata) { + + WireguardPeer *peer = userdata; NetDev *netdev; - Wireguard *w; int r; assert(peer); assert(peer->wireguard); - w = peer->wireguard; - netdev = NETDEV(w); + netdev = NETDEV(peer->wireguard); if (!netdev_is_managed(netdev)) return 0; if (ret != 0) { - log_netdev_error(netdev, "Failed to resolve host '%s:%s': %s", peer->endpoint_host, peer->endpoint_port, gai_strerror(ret)); - - r = set_ensure_put(&w->peers_with_failed_endpoint, NULL, peer); - if (r < 0) { - log_netdev_error(netdev, "Failed to save a peer, dropping the peer: %m"); - peer->section->invalid = true; - goto resolve_next; - } + log_netdev_warning(netdev, "Failed to resolve host '%s:%s', ignoring: %s", + peer->endpoint_host, peer->endpoint_port, gai_strerror(ret)); + peer->n_retries++; } else if ((ai->ai_family == AF_INET && ai->ai_addrlen == sizeof(struct sockaddr_in)) || - (ai->ai_family == AF_INET6 && ai->ai_addrlen == sizeof(struct sockaddr_in6))) + (ai->ai_family == AF_INET6 && ai->ai_addrlen == sizeof(struct sockaddr_in6))) { + memcpy(&peer->endpoint, ai->ai_addr, ai->ai_addrlen); - else - log_netdev_error(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint %s:%s, ignoring the address.", - peer->endpoint_host, peer->endpoint_port); + (void) wireguard_set_interface(netdev); + peer->n_retries = 0; -resolve_next: - if (!set_isempty(w->peers_with_unresolved_endpoint)) { - resolve_endpoints(netdev); - return 0; + } else { + log_netdev_warning(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint %s:%s, ignoring the endpoint.", + peer->endpoint_host, peer->endpoint_port); + peer->n_retries++; } - (void) wireguard_set_interface(netdev); - - if (!set_isempty(w->peers_with_failed_endpoint)) { - usec_t usec; - - w->n_retries++; - usec = usec_add(now(CLOCK_MONOTONIC), exponential_backoff_milliseconds(w->n_retries)); - r = event_reset_time(netdev->manager->event, &w->resolve_retry_event_source, - CLOCK_MONOTONIC, usec, 0, on_resolve_retry, netdev, - 0, "wireguard-resolve-retry", true); - if (r < 0) { - log_netdev_warning_errno(netdev, r, "Could not arm resolve retry handler: %m"); - return 0; - } + if (peer->n_retries > 0) { + r = event_reset_time_relative(netdev->manager->event, + &peer->resolve_retry_event_source, + clock_boottime_or_monotonic(), + peer_next_resolve_usec(peer), 0, + on_resolve_retry, peer, 0, "wireguard-resolve-retry", true); + if (r < 0) + log_netdev_warning_errno(netdev, r, "Could not arm resolve retry handler for endpoint %s:%s, ignoring: %m", + peer->endpoint_host, peer->endpoint_port); } + wireguard_resolve_endpoints(netdev); return 0; } -static void resolve_endpoints(NetDev *netdev) { +static int peer_resolve_endpoint(WireguardPeer *peer) { static const struct addrinfo hints = { .ai_family = AF_UNSPEC, .ai_socktype = SOCK_DGRAM, .ai_protocol = IPPROTO_UDP }; + NetDev *netdev; + int r; + + assert(peer); + assert(peer->wireguard); + + netdev = NETDEV(peer->wireguard); + + if (!peer->endpoint_host || !peer->endpoint_port) + /* Not necessary to resolve the endpoint. */ + return 0; + + if (event_source_is_enabled(peer->resolve_retry_event_source) > 0) + /* Timer event source is enabled. The endpoint will be resolved later. */ + return 0; + + if (peer->resolve_query) + /* Being resolved, or already resolved. */ + return 0; + + r = sd_resolve_getaddrinfo(netdev->manager->resolve, + &peer->resolve_query, + peer->endpoint_host, + peer->endpoint_port, + &hints, + wireguard_peer_resolve_handler, + peer); + if (r < 0) + return log_netdev_full_errno(netdev, r == -ENOBUFS ? LOG_DEBUG : LOG_WARNING, r, + "Failed to create endpoint resolver for %s:%s, ignoring: %m", + peer->endpoint_host, peer->endpoint_port); + + return 0; +} + +static void wireguard_resolve_endpoints(NetDev *netdev) { WireguardPeer *peer; Wireguard *w; - int r; assert(netdev); w = WIREGUARD(netdev); assert(w); - SET_FOREACH(peer, w->peers_with_unresolved_endpoint) { - r = resolve_getaddrinfo(netdev->manager->resolve, - NULL, - peer->endpoint_host, - peer->endpoint_port, - &hints, - wireguard_resolve_handler, - wireguard_peer_destroy_callback, - peer); - if (r == -ENOBUFS) + LIST_FOREACH(peers, peer, w->peers) + if (peer_resolve_endpoint(peer) == -ENOBUFS) + /* Too many requests. Let's resolve remaining endpoints later. */ break; - if (r < 0) { - log_netdev_error_errno(netdev, r, "Failed to create resolver: %m"); - continue; - } - - /* Avoid freeing netdev. It will be unrefed by the destroy callback. */ - netdev_ref(netdev); - - (void) set_remove(w->peers_with_unresolved_endpoint, peer); - } } static int netdev_wireguard_post_create(NetDev *netdev, Link *link, sd_netlink_message *m) { @@ -431,7 +432,7 @@ static int netdev_wireguard_post_create(NetDev *netdev, Link *link, sd_netlink_m assert(WIREGUARD(netdev)); (void) wireguard_set_interface(netdev); - resolve_endpoints(netdev); + wireguard_resolve_endpoints(netdev); return 0; } @@ -761,7 +762,6 @@ int config_parse_wireguard_endpoint( peer->endpoint_host = mfree(peer->endpoint_host); peer->endpoint_port = mfree(peer->endpoint_port); - set_remove(w->peers_with_unresolved_endpoint, peer); TAKE_PTR(peer); return 0; @@ -803,10 +803,6 @@ int config_parse_wireguard_endpoint( if (r < 0) return log_oom(); - r = set_ensure_put(&w->peers_with_unresolved_endpoint, NULL, peer); - if (r < 0) - return log_oom(); - TAKE_PTR(peer); /* The peer may already have been in the hash map, that is fine too. */ return 0; } @@ -1054,14 +1050,10 @@ static void wireguard_done(NetDev *netdev) { w = WIREGUARD(netdev); assert(w); - sd_event_source_unref(w->resolve_retry_event_source); - explicit_bzero_safe(w->private_key, WG_KEY_LEN); free(w->private_key_file); hashmap_free_with_destructor(w->peers_by_section, wireguard_peer_free); - set_free(w->peers_with_unresolved_endpoint); - set_free(w->peers_with_failed_endpoint); set_free(w->routes); } diff --git a/src/network/netdev/wireguard.h b/src/network/netdev/wireguard.h index 5d4b6da45e..29bacdc771 100644 --- a/src/network/netdev/wireguard.h +++ b/src/network/netdev/wireguard.h @@ -7,6 +7,9 @@ typedef struct Wireguard Wireguard; #include #include +#include "sd-event.h" +#include "sd-resolve.h" + #include "in-addr-util.h" #include "netdev.h" #include "socket-util.h" @@ -33,6 +36,10 @@ typedef struct WireguardPeer { char *endpoint_host; char *endpoint_port; + unsigned n_retries; + sd_event_source *resolve_retry_event_source; + sd_resolve_query *resolve_query; + uint32_t route_table; uint32_t route_priority; bool route_table_set; @@ -53,14 +60,8 @@ struct Wireguard { uint32_t fwmark; Hashmap *peers_by_section; - Set *peers_with_unresolved_endpoint; - Set *peers_with_failed_endpoint; - LIST_HEAD(WireguardPeer, peers); - unsigned n_retries; - sd_event_source *resolve_retry_event_source; - Set *routes; uint32_t route_table; uint32_t route_priority; From 38ef464e415860630e3f8297c10618cf4425e7cc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 30 Nov 2021 00:20:03 +0900 Subject: [PATCH 4/4] network/wireguard: search valid address of the endpoint from all struct addrinfo entries --- src/network/netdev/wireguard.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 2c60ba197d..e5cfb35c95 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -344,17 +344,27 @@ static int wireguard_peer_resolve_handler( peer->endpoint_host, peer->endpoint_port, gai_strerror(ret)); peer->n_retries++; - } else if ((ai->ai_family == AF_INET && ai->ai_addrlen == sizeof(struct sockaddr_in)) || - (ai->ai_family == AF_INET6 && ai->ai_addrlen == sizeof(struct sockaddr_in6))) { - - memcpy(&peer->endpoint, ai->ai_addr, ai->ai_addrlen); - (void) wireguard_set_interface(netdev); - peer->n_retries = 0; - } else { - log_netdev_warning(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint %s:%s, ignoring the endpoint.", - peer->endpoint_host, peer->endpoint_port); - peer->n_retries++; + bool found = false; + for (; ai; ai = ai->ai_next) { + if (!IN_SET(ai->ai_family, AF_INET, AF_INET6)) + continue; + + if (ai->ai_addrlen != (ai->ai_family == AF_INET ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6))) + continue; + + memcpy(&peer->endpoint, ai->ai_addr, ai->ai_addrlen); + (void) wireguard_set_interface(netdev); + peer->n_retries = 0; + found = true; + break; + } + + if (!found) { + log_netdev_warning(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint %s:%s, ignoring the endpoint.", + peer->endpoint_host, peer->endpoint_port); + peer->n_retries++; + } } if (peer->n_retries > 0) {