From 29336aa2e27a77c8c042f57f604d694a1f24c536 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Feb 2021 15:45:44 +0100 Subject: [PATCH 1/2] platform: ensure NM_SOCK_ADDR_UNION_INIT_UNSPEC() fully initializes union In C, initialization of a union does not define that excess memory is initialized. Ensure that, by initializing the largest member of the NMSockAddrUnion union. (cherry picked from commit 7bf2ddf73f8d8c2d882c4932e88b76eef337cd0a) --- src/core/platform/nmp-object.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/platform/nmp-object.h b/src/core/platform/nmp-object.h index dc2cc86b65..19f6bcd7ba 100644 --- a/src/core/platform/nmp-object.h +++ b/src/core/platform/nmp-object.h @@ -34,10 +34,14 @@ typedef union { struct sockaddr_in6 in6; } NMSockAddrUnion; +G_STATIC_ASSERT(sizeof(NMSockAddrUnion) == sizeof(((NMSockAddrUnion *) NULL)->in6)); + +/* we initialize the largest union member, to ensure that all fields are initialized. */ + #define NM_SOCK_ADDR_UNION_INIT_UNSPEC \ { \ - .sa = { \ - .sa_family = AF_UNSPEC, \ + .in6 = { \ + .sin6_family = AF_UNSPEC, \ }, \ } From 2e00d161b2a909d78dde2d17113926582f75a816 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Feb 2021 15:47:31 +0100 Subject: [PATCH 2/2] wireguard: prefer last resolved IP from resolving endpoint from DNS We periodically re-resolve the DNS name for entpoints. Since WireGuard has no concept of being connected, we want to eventually pick up if the DNS name resolves to a different IP address. However, on resolution failure, we will never clear the endpoint we already have. Thus, resolving names can only give a better endpoint, not remove an IP address entirely. DNS names might do Round-Robin load distribution and the name of the endpoint might resolve to multiple IP addresses. Improve to stick to the IP address that we already have -- provided that the IP address is still among the new resolution result. Otherwise, we continue to pick the first IP address that was resolved. (cherry picked from commit 98348ee5396dde5756fbb82ebf16b90790b6b32d) --- src/core/devices/nm-device-wireguard.c | 42 ++++++++++++++++---------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/core/devices/nm-device-wireguard.c b/src/core/devices/nm-device-wireguard.c index fd057ded9c..5bee09e6f1 100644 --- a/src/core/devices/nm-device-wireguard.c +++ b/src/core/devices/nm-device-wireguard.c @@ -729,7 +729,7 @@ _peers_resolve_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) PeerData * peer_data; gs_free_error GError *resolv_error = NULL; GList * list; - gboolean changed = FALSE; + gboolean changed; NMSockAddrUnion sockaddr; gint64 retry_in_msec; char s_sockaddr[100]; @@ -775,36 +775,49 @@ _peers_resolve_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) } sockaddr = (NMSockAddrUnion) NM_SOCK_ADDR_UNION_INIT_UNSPEC; + changed = FALSE; if (!resolv_error) { GList *iter; for (iter = list; iter; iter = iter->next) { - GInetAddress *a = iter->data; - GSocketFamily f = g_inet_address_get_family(a); + GInetAddress * a = iter->data; + NMSockAddrUnion sockaddr_tmp; + NMSockAddrUnion *s; - if (f == G_SOCKET_FAMILY_IPV4) { + s = sockaddr.sa.sa_family == AF_UNSPEC ? &sockaddr : &sockaddr_tmp; + + switch (g_inet_address_get_family(a)) { + case G_SOCKET_FAMILY_IPV4: nm_assert(g_inet_address_get_native_size(a) == sizeof(struct in_addr)); - sockaddr.in = (struct sockaddr_in){ + s->in = (struct sockaddr_in){ .sin_family = AF_INET, .sin_port = htons(nm_sock_addr_endpoint_get_port( _nm_wireguard_peer_get_endpoint(peer_data->peer))), }; - memcpy(&sockaddr.in.sin_addr, g_inet_address_to_bytes(a), sizeof(struct in_addr)); + memcpy(&s->in.sin_addr, g_inet_address_to_bytes(a), sizeof(struct in_addr)); break; - } - if (f == G_SOCKET_FAMILY_IPV6) { + case G_SOCKET_FAMILY_IPV6: nm_assert(g_inet_address_get_native_size(a) == sizeof(struct in6_addr)); - sockaddr.in6 = (struct sockaddr_in6){ + s->in6 = (struct sockaddr_in6){ .sin6_family = AF_INET6, .sin6_port = htons(nm_sock_addr_endpoint_get_port( _nm_wireguard_peer_get_endpoint(peer_data->peer))), .sin6_scope_id = 0, .sin6_flowinfo = 0, }; - memcpy(&sockaddr.in6.sin6_addr, - g_inet_address_to_bytes(a), - sizeof(struct in6_addr)); + memcpy(&s->in6.sin6_addr, g_inet_address_to_bytes(a), sizeof(struct in6_addr)); + break; + default: + continue; + } + + changed = TRUE; + if (peer_data->ep_resolv.sockaddr.sa.sa_family == AF_UNSPEC) + break; + + if (nm_sock_addr_union_cmp(&peer_data->ep_resolv.sockaddr, &sockaddr) == 0) { + changed = FALSE; break; } } @@ -819,11 +832,8 @@ _peers_resolve_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) * a possibly good IP address, since WireGuard supports automatic roaming * anyway. Either the IP address is still good (and we would wrongly * reject it), or it isn't -- in which case it does not hurt much. */ - } else { - if (nm_sock_addr_union_cmp(&peer_data->ep_resolv.sockaddr, &sockaddr) != 0) - changed = TRUE; + } else if (changed) peer_data->ep_resolv.sockaddr = sockaddr; - } if (resolv_error || peer_data->ep_resolv.sockaddr.sa.sa_family == AF_UNSPEC) { /* while it technically did not fail, something is probably odd. Retry frequently to