Merge pull request #21584 from yuwata/network-wireguard-cleanups

network: cleanups for wireguard

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;
 

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"
@@ -24,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;
@@ -40,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);
         }
@@ -59,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);
 }
 
@@ -287,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);
@@ -295,134 +299,142 @@ 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));
+        peer->resolve_query = sd_resolve_query_unref(peer->resolve_query);
-
-        SWAP_TWO(w->peers_with_unresolved_endpoint, w->peers_with_failed_endpoint);
-
-        resolve_endpoints(netdev);
 
+        (void) peer_resolve_endpoint(peer);
         return 0;
 }
 
-/*
+static usec_t peer_next_resolve_usec(WireguardPeer *peer) {
- * Given the number of retries this function will return will an exponential
+        usec_t usec;
- * increasing time in milliseconds to wait starting at 200ms and capped at 25 seconds.
+
- */
+        /* Given the number of retries this function will return an exponential increasing amount of
-static int exponential_backoff_milliseconds(unsigned n_retries) {
+         * milliseconds to wait starting at 200ms and capped at 25 seconds. */
-        return (2 << MIN(n_retries, 7U)) * 100 * USEC_PER_MSEC;
+
+        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,
+static int wireguard_peer_resolve_handler(
+              sd_resolve_query *q,
               int ret,
               const struct addrinfo *ai,
-                                     WireguardPeer *peer) {
+              void *userdata) {
+
+        WireguardPeer *peer = userdata;
         NetDev *netdev;
-        Wireguard *w;
         int r;
 
         assert(peer);
         assert(peer->wireguard);
 
-        w = peer->wireguard;
+        netdev = NETDEV(peer->wireguard);
-        netdev = NETDEV(w);
 
         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));
+                log_netdev_warning(netdev, "Failed to resolve host '%s:%s', ignoring: %s",
+                                   peer->endpoint_host, peer->endpoint_port, gai_strerror(ret));
+                peer->n_retries++;
 
-                r = set_ensure_put(&w->peers_with_failed_endpoint, NULL, peer);
+        } else {
-                if (r < 0) {
+                bool found = false;
-                        log_netdev_error(netdev, "Failed to save a peer, dropping the peer: %m");
+                for (; ai; ai = ai->ai_next) {
-                        peer->section->invalid = true;
+                        if (!IN_SET(ai->ai_family, AF_INET, AF_INET6))
-                        goto resolve_next;
+                                continue;
-                }
+
+                        if (ai->ai_addrlen != (ai->ai_family == AF_INET ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6)))
+                                continue;
 
-        } 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);
-        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);
-
-resolve_next:
-        if (!set_isempty(w->peers_with_unresolved_endpoint)) {
-                resolve_endpoints(netdev);
-                return 0;
-        }
-
                         (void) wireguard_set_interface(netdev);
+                        peer->n_retries = 0;
+                        found = true;
+                        break;
+                }
 
-        if (!set_isempty(w->peers_with_failed_endpoint)) {
+                if (!found) {
-                usec_t usec;
+                        log_netdev_warning(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint %s:%s, ignoring the endpoint.",
-
+                                           peer->endpoint_host, peer->endpoint_port);
-                w->n_retries++;
+                        peer->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) {
+        LIST_FOREACH(peers, peer, w->peers)
-                r = resolve_getaddrinfo(netdev->manager->resolve,
+                if (peer_resolve_endpoint(peer) == -ENOBUFS)
-                                        NULL,
+                        /* Too many requests. Let's resolve remaining endpoints later. */
-                                        peer->endpoint_host,
-                                        peer->endpoint_port,
-                                        &hints,
-                                        wireguard_resolve_handler,
-                                        wireguard_peer_destroy_callback,
-                                        peer);
-                if (r == -ENOBUFS)
                         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) {
@@ -430,7 +442,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;
 }
 
@@ -723,65 +735,85 @@ 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 family, r;
-        int 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);
+        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);
+
+                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();
+
+        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 = free_and_strdup(&peer->endpoint_port, end);
-        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;
 }
 
@@ -1028,14 +1060,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);
 }

src/network/netdev/wireguard.h

@@ -7,6 +7,9 @@ typedef struct Wireguard Wireguard;
 #include <netinet/in.h>
 #include <linux/wireguard.h>
 
+#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;

src/network/networkd-address.c

@@ -1394,20 +1394,6 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message,
                         address->scope = tmp->scope;
                         address_set_lifetime(address, &cinfo);
                         address_enter_configured(address);
-
-                        if (address->family == AF_INET6 &&
-                            address->source == NETWORK_CONFIG_SOURCE_FOREIGN &&
-                            FLAGS_SET(address->flags, IFA_F_SECONDARY | IFA_F_DEPRECATED)) {
-
-                                /* The temporary address became deprecated, i.e. preferred lifetime == 0. */
-                                log_address_debug(address, "Received deprecated", link);
-                                r = address_remove(address);
-                                if (r < 0)
-                                        link_enter_failed(link);
-
-                                return 0;
-                        }
-
                         log_address_debug(address, "Received updated", link);
                 } else {
                         address_set_lifetime(tmp, &cinfo);