From 98f28ddf2e9cbf04f2af99a9059c52f6d56eaa99 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 Sep 2018 13:19:36 +0200 Subject: platform/netlink: fix nl_errno() to get absolute error number value --- src/platform/nm-netlink.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/nm-netlink.h b/src/platform/nm-netlink.h index 24565d074d..e20388e200 100644 --- a/src/platform/nm-netlink.h +++ b/src/platform/nm-netlink.h @@ -62,7 +62,7 @@ nl_errno (int err) * normalizes the error and returns its positive value. */ return err >= 0 ? err - : ((err == G_MININT) ? NLE_BUG : -errno); + : ((err == G_MININT) ? NLE_BUG : -err); } static inline int -- cgit v1.2.1 From 1fb8fbbc9967bce6bdda707d87b694c1f18aa860 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 08:02:35 +0200 Subject: shared: add nm_memdup() as replacement for g_memdup() I think g_memdup() is dangerous for integer overflow. There is no need for accepting this danger, just use our own nm_memdup() which does not have this flaw. --- shared/nm-utils/nm-shared-utils.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 171544feab..ce5fd9986e 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -190,6 +190,37 @@ nm_ip_addr_set (int addr_family, gpointer dst, const NMIPAddr *src) /*****************************************************************************/ +/* like g_memdup(). The difference is that the @size argument is of type + * gsize, while g_memdup() has type guint. Since, the size of container types + * like GArray is guint as well, this means trying to g_memdup() an + * array, + * g_memdup (array->data, array->len * sizeof (ElementType)) + * will lead to integer overflow, if there are more than G_MAXUINT/sizeof(ElementType) + * bytes. That seems unnecessarily dangerous to me. + * nm_memdup() avoids that, because its size argument is always large enough + * to contain all data that a GArray can hold. + * + * Another minor difference to g_memdup() is that the glib version also + * returns %NULL if @data is %NULL. E.g. g_memdup(NULL, 1) + * gives %NULL, but nm_memdup(NULL, 1) crashes. I think that + * is desirable, because @size MUST be correct at all times. @size + * may be zero, but one must not claim to have non-zero bytes when + * passing a %NULL @data pointer. + */ +static inline gpointer +nm_memdup (gconstpointer data, gsize size) +{ + gpointer p; + + if (size == 0) + return NULL; + p = g_malloc (size); + memcpy (p, data, size); + return p; +} + +/*****************************************************************************/ + extern const void *const _NM_PTRARRAY_EMPTY[1]; #define NM_PTRARRAY_EMPTY(type) ((type const*) _NM_PTRARRAY_EMPTY) -- cgit v1.2.1 From 085a3694463caf2c7a8e214fea351353c66dbad4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Sep 2018 08:08:38 +0200 Subject: all: avoid g_memdup() By using nm_memdup(). Except in shared/nm-utils/nm-compat.c, which may not include "shared/nm-utils/nm-shared-utils.h". --- shared/nm-utils/nm-compat.c | 3 ++- src/nm-session-monitor.c | 2 +- src/nm-test-utils-core.h | 8 ++++---- src/platform/nm-netlink.c | 2 +- src/platform/nmp-object.c | 6 +++--- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/shared/nm-utils/nm-compat.c b/shared/nm-utils/nm-compat.c index 90328c065f..aa7c42f14e 100644 --- a/shared/nm-utils/nm-compat.c +++ b/shared/nm-utils/nm-compat.c @@ -60,7 +60,8 @@ _get_keys (NMSettingVpn *setting, if (len) { g_ptr_array_sort (a, nm_strcmp_p); g_ptr_array_add (a, NULL); - keys = g_memdup (a->pdata, a->len * sizeof (gpointer)); + keys = g_malloc (a->len * sizeof (gpointer)); + memcpy (keys, a->pdata, a->len * sizeof (gpointer)); /* we need to cache the keys *somewhere*. */ g_object_set_qdata_full (G_OBJECT (setting), diff --git a/src/nm-session-monitor.c b/src/nm-session-monitor.c index 7dbe835b8e..b67c537c8b 100644 --- a/src/nm-session-monitor.c +++ b/src/nm-session-monitor.c @@ -188,7 +188,7 @@ ck_load_cache (GHashTable *cache) if (error) goto out; - g_hash_table_insert (cache, GUINT_TO_POINTER (uid), g_memdup (&session, sizeof session)); + g_hash_table_insert (cache, GUINT_TO_POINTER (uid), nm_memdup (&session, sizeof session)); } finished = TRUE; diff --git a/src/nm-test-utils-core.h b/src/nm-test-utils-core.h index c76f510f2f..5e89cb39f1 100644 --- a/src/nm-test-utils-core.h +++ b/src/nm-test-utils-core.h @@ -216,8 +216,8 @@ nmtst_platform_ip4_routes_equal (const NMPlatformIP4Route *a, const NMPlatformIP g_assert (b); if (ignore_order) { - a = c_a = g_memdup (a, sizeof (NMPlatformIP4Route) * len); - b = c_b = g_memdup (b, sizeof (NMPlatformIP4Route) * len); + a = c_a = nm_memdup (a, sizeof (NMPlatformIP4Route) * len); + b = c_b = nm_memdup (b, sizeof (NMPlatformIP4Route) * len); g_qsort_with_data (c_a, len, sizeof (NMPlatformIP4Route), _nmtst_platform_ip4_routes_equal_sort, NULL); g_qsort_with_data (c_b, len, sizeof (NMPlatformIP4Route), _nmtst_platform_ip4_routes_equal_sort, NULL); } @@ -269,8 +269,8 @@ nmtst_platform_ip6_routes_equal (const NMPlatformIP6Route *a, const NMPlatformIP g_assert (b); if (ignore_order) { - a = c_a = g_memdup (a, sizeof (NMPlatformIP6Route) * len); - b = c_b = g_memdup (b, sizeof (NMPlatformIP6Route) * len); + a = c_a = nm_memdup (a, sizeof (NMPlatformIP6Route) * len); + b = c_b = nm_memdup (b, sizeof (NMPlatformIP6Route) * len); g_qsort_with_data (c_a, len, sizeof (NMPlatformIP6Route), _nmtst_platform_ip6_routes_equal_sort, NULL); g_qsort_with_data (c_b, len, sizeof (NMPlatformIP6Route), _nmtst_platform_ip6_routes_equal_sort, NULL); } diff --git a/src/platform/nm-netlink.c b/src/platform/nm-netlink.c index 2064a4bba2..5ca7bb406c 100644 --- a/src/platform/nm-netlink.c +++ b/src/platform/nm-netlink.c @@ -1470,7 +1470,7 @@ retry: continue; if (cmsg->cmsg_type != SCM_CREDENTIALS) continue; - tmpcreds = g_memdup (CMSG_DATA(cmsg), sizeof (*tmpcreds)); + tmpcreds = nm_memdup (CMSG_DATA(cmsg), sizeof (*tmpcreds)); break; } } diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index 431fc2b124..a8e6beb399 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -340,7 +340,7 @@ _vlan_xgress_qos_mappings_cpy (guint *dst_n_map, g_clear_pointer (dst_map, g_free); *dst_n_map = src_n_map; if (src_n_map > 0) - *dst_map = g_memdup (src_map, sizeof (*src_map) * src_n_map); + *dst_map = nm_memdup (src_map, sizeof (*src_map) * src_n_map); } } @@ -452,9 +452,9 @@ _wireguard_peers_cpy (gsize *dst_n_peers, g_clear_pointer (dst_peers, g_free); *dst_n_peers = src_n_peers; if (src_n_peers > 0) - *dst_peers = g_memdup (src_peers, sizeof (*src_peers) * src_n_peers); + *dst_peers = nm_memdup (src_peers, sizeof (*src_peers) * src_n_peers); for (i = 0; i < src_n_peers; i++) { - dst_peers[i]->allowedips = g_memdup (src_peers[i].allowedips, sizeof (src_peers[i].allowedips) * src_peers[i].allowedips_len); + dst_peers[i]->allowedips = nm_memdup (src_peers[i].allowedips, sizeof (src_peers[i].allowedips) * src_peers[i].allowedips_len); dst_peers[i]->allowedips_len = src_peers[i].allowedips_len; } } -- cgit v1.2.1 From 0feeeaac636e39e724a2b3dc848b0fe12858d859 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 Sep 2018 14:17:22 +0200 Subject: shared: add nm_utils_mem_all_zero() --- shared/nm-utils/nm-shared-utils.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index ce5fd9986e..778938e09c 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -190,6 +190,22 @@ nm_ip_addr_set (int addr_family, gpointer dst, const NMIPAddr *src) /*****************************************************************************/ +static inline gboolean +nm_utils_mem_all_zero (gconstpointer mem, gsize len) +{ + const guint8 *p; + + for (p = mem; len-- > 0; p++) { + if (*p != 0) + return FALSE; + } + + /* incidentally, a buffer with len==0, is also *all-zero*. */ + return TRUE; +} + +/*****************************************************************************/ + /* like g_memdup(). The difference is that the @size argument is of type * gsize, while g_memdup() has type guint. Since, the size of container types * like GArray is guint as well, this means trying to g_memdup() an -- cgit v1.2.1 From 0a8248af104c3be18bf6bd3ed8b8b3e88039a3f5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 Sep 2018 15:20:21 +0200 Subject: shared: add nm_utils_strbuf_seek_end() helper --- shared/nm-utils/nm-shared-utils.c | 84 ++++++++++++++++++++++++++++++++++++++- shared/nm-utils/nm-shared-utils.h | 1 + src/tests/test-general.c | 43 +++++++++++++++++++- 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index b8f95f3d62..19b17f68f1 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -106,7 +106,7 @@ nm_utils_strbuf_append (char **buf, gsize *len, const char *format, ...) retval = g_vsnprintf (p, *len, format, args); va_end (args); - if (retval >= *len) { + if ((gsize) retval >= *len) { *buf = &p[*len]; *len = 0; } else { @@ -115,6 +115,88 @@ nm_utils_strbuf_append (char **buf, gsize *len, const char *format, ...) } } +/** + * nm_utils_strbuf_seek_end: + * @buf: the input/output buffer + * @len: the input/output lenght of the buffer. + * + * Commonly, one uses nm_utils_strbuf_append*(), to incrementally + * append strings to the buffer. However, sometimes we need to use + * existing API to write to the buffer. + * After doing so, we want to adjust the buffer counter. + * Essentially, + * + * g_snprintf (buf, len, ...); + * nm_utils_strbuf_seek_end (&buf, &len); + * + * is almost the same as + * + * nm_utils_strbuf_append (&buf, &len, ...); + * + * They only behave differently, if the string fits exactly + * into the buffer without truncation. The former cannot distinguish + * the two cases, while the latter can. + */ +void +nm_utils_strbuf_seek_end (char **buf, gsize *len) +{ + gsize l; + char *end; + + nm_assert (len); + nm_assert (buf && *buf); + + if (*len == 0) + return; + + end = memchr (*buf, 0, *len); + if (!end) { + /* hm, no NUL character within len bytes. + * Just NUL terminate the array and consume them + * all. */ + *buf += *len; + (*buf)[-1] = '\0'; + *len = 0; + return; + } + + l = end - *buf; + nm_assert (l < *len); + + *buf = end; + *len -= l; + if (*len == 1) { + /* the last character of a buffer is the '\0'. There are two + * cases why that may happen: + * - but string was truncated + * - the string fit exactly into the buffer. + * Here we cannot distinguish between the two, so assume the string + * was truncated and signal that by setting @len to 0 and pointing the + * buffer *past* the end (like all other nm_utils_strbuf_*() functions). + * + * Note that nm_utils_strbuf_append_str() can distinguish between + * the two cases, and leaves @len at 1, if the string was not actually + * truncated. + * + * For consistancy, it might be better not to do this and just + * seek to end of the buffer (not past it). However, that would mean, + * in a series of + * g_snprintf() + * nm_utils_strbuf_seek_end() + * the length would never reach zero, but stay at 1. With this, + * it reaches len 0 early. + * It seems better to declare the buffer as fully consumed and set + * the length to zero. + * + * If the caller does not care about truncation, then this behavior + * is more sensible. If the caller cares about truncation, it must + * check earlier (right when the truncation occures). + */ + (*buf)++; + *len = 0; + } +} + /*****************************************************************************/ /** diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 778938e09c..b3099738f3 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -257,6 +257,7 @@ _nm_utils_strbuf_init (char *buf, gsize len, char **p_buf_ptr, gsize *p_buf_len) void nm_utils_strbuf_append (char **buf, gsize *len, const char *format, ...) _nm_printf (3, 4); void nm_utils_strbuf_append_c (char **buf, gsize *len, char c); void nm_utils_strbuf_append_str (char **buf, gsize *len, const char *str); +void nm_utils_strbuf_seek_end (char **buf, gsize *len); const char *nm_strquote (char *buf, gsize buf_len, const char *str); diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 3b83a66e60..1c8b944512 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -1445,7 +1445,7 @@ test_nm_utils_strbuf_append (void) t_buf = buf; t_len = buf_len; - test_mode = nmtst_get_rand_int () % 4; + test_mode = nmtst_get_rand_int () % 5; switch (test_mode) { case 0: @@ -1466,6 +1466,47 @@ test_nm_utils_strbuf_append (void) case 3: nm_utils_strbuf_append (&t_buf, &t_len, "%s", str); break; + case 4: + g_snprintf (t_buf, t_len, "%s", str); + if ( t_len > 0 + && strlen (str) >= buf_len + && (nmtst_get_rand_int () % 2)) { + /* the string was truncated by g_snprintf(). That means, at the last position in the + * buffer is now NUL. + * Replace the NUL by the actual character, and check that nm_utils_strbuf_seek_end() + * does the right thing: NUL terminate the buffer and seek past the end of the buffer. */ + g_assert_cmpmem (t_buf, t_len - 1, str, t_len - 1); + g_assert (t_buf[t_len - 1] == '\0'); + g_assert (str[t_len - 1] != '\0'); + t_buf[t_len - 1] = str[t_len - 1]; + nm_utils_strbuf_seek_end (&t_buf, &t_len); + g_assert (t_len == 0); + g_assert (t_buf == &buf[buf_len]); + g_assert (t_buf[-1] == '\0'); + } else { + nm_utils_strbuf_seek_end (&t_buf, &t_len); + if (strlen (str) + 1 == buf_len) { + /* Special case: we appended a string that fit into the buffer + * exactly, without truncation. + * If we would append the string via nm_utils_strbuf_append(), + * then it would have recognized that the string was not truncated + * and leave len==1, and pointing the buffer to the terminating NUL + * (at the very end, not past it). + * + * But nm_utils_strbuf_seek_end() cannot distinguish whether + * truncation occured, and assumes the buffer was indeed truncated. + * + * Assert for that, but also adjust the numbers, so that the assertions + * below pass (the assertions below theck for the nm_utils_strbuf_append() + * case). */ + g_assert (t_len == 0); + g_assert (t_buf == &buf[buf_len]); + g_assert (t_buf[-1] == '\0'); + t_len = 1; + t_buf--; + } + } + break; } /* Assert that the source-buffer is unmodified. */ -- cgit v1.2.1 From 09aaeb83b7a18d11ac269e96dba5678990c50540 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 Sep 2018 15:05:36 +0200 Subject: platform: fix printing all-info about NMPObjectLink instances When we print info about the link, we also want to print info about the referenced lnk instance, which commonly contains link-specific data. For NMP_OBJECT_TO_STRING_PUBLIC this was done correctly, by calling to-string of public fields on the lnk object. For NMP_OBJECT_TO_STRING_ALL, we wrongly just delegated to the public to-string function, this means, for the lnk object we would not print all fields. Fix that. --- src/platform/nmp-object.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index a8e6beb399..4358f8d714 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -760,31 +760,35 @@ static const char * _vt_cmd_obj_to_string_link (const NMPObject *obj, NMPObjectToStringMode to_string_mode, char *buf, gsize buf_size) { const NMPClass *klass = NMP_OBJECT_GET_CLASS (obj); - char buf2[sizeof (_nm_utils_to_string_buffer)]; - char buf3[sizeof (_nm_utils_to_string_buffer)]; + char *b = buf; switch (to_string_mode) { case NMP_OBJECT_TO_STRING_ID: return klass->cmd_plobj_to_string_id (&obj->object, buf, buf_size); case NMP_OBJECT_TO_STRING_ALL: - g_snprintf (buf, buf_size, - "[%s,%p,%u,%calive,%cvisible,%cin-nl,%p; %s]", - klass->obj_type_name, obj, obj->parent._ref_count, - nmp_object_is_alive (obj) ? '+' : '-', - nmp_object_is_visible (obj) ? '+' : '-', - obj->_link.netlink.is_in_netlink ? '+' : '-', - obj->_link.udev.device, - nmp_object_to_string (obj, NMP_OBJECT_TO_STRING_PUBLIC, buf2, sizeof (buf2))); + nm_utils_strbuf_append (&b, &buf_size, + "[%s,%p,%u,%calive,%cvisible,%cin-nl,%p; ", + klass->obj_type_name, obj, obj->parent._ref_count, + nmp_object_is_alive (obj) ? '+' : '-', + nmp_object_is_visible (obj) ? '+' : '-', + obj->_link.netlink.is_in_netlink ? '+' : '-', + obj->_link.udev.device); + NMP_OBJECT_GET_CLASS (obj)->cmd_plobj_to_string (&obj->object, b, buf_size); + nm_utils_strbuf_seek_end (&b, &buf_size); + if (obj->_link.netlink.lnk) { + nm_utils_strbuf_append_str (&b, &buf_size, "; "); + nmp_object_to_string (obj->_link.netlink.lnk, NMP_OBJECT_TO_STRING_ALL, b, buf_size); + nm_utils_strbuf_seek_end (&b, &buf_size); + } + nm_utils_strbuf_append_c (&b, &buf_size, ']'); return buf; case NMP_OBJECT_TO_STRING_PUBLIC: + NMP_OBJECT_GET_CLASS (obj)->cmd_plobj_to_string (&obj->object, b, buf_size); if (obj->_link.netlink.lnk) { - NMP_OBJECT_GET_CLASS (obj)->cmd_plobj_to_string (&obj->object, buf2, sizeof (buf2)); - nmp_object_to_string (obj->_link.netlink.lnk, NMP_OBJECT_TO_STRING_PUBLIC, buf3, sizeof (buf3)); - g_snprintf (buf, buf_size, - "%s; %s", - buf2, buf3); - } else - NMP_OBJECT_GET_CLASS (obj)->cmd_plobj_to_string (&obj->object, buf, buf_size); + nm_utils_strbuf_seek_end (&b, &buf_size); + nm_utils_strbuf_append_str (&b, &buf_size, "; "); + nmp_object_to_string (obj->_link.netlink.lnk, NMP_OBJECT_TO_STRING_PUBLIC, b, buf_size); + } return buf; default: g_return_val_if_reached ("ERROR"); -- cgit v1.2.1 From 5fd4ca8a5b1d8fca842559134b7d005de655f2e1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 15 Jul 2018 14:18:56 +0200 Subject: platform/netlink: drop nlmsg_alloc_inherit() function It's only used internally, and it seems not very useful to have. As it is confusing to have multiple functions for doing something similar, drop it -- since it's not really used. I also cannot imagine a good use-case for it. --- src/platform/nm-netlink.c | 33 +++++++-------------------------- src/platform/nm-netlink.h | 2 -- 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/platform/nm-netlink.c b/src/platform/nm-netlink.c index 5ca7bb406c..c057acd9a1 100644 --- a/src/platform/nm-netlink.c +++ b/src/platform/nm-netlink.c @@ -331,27 +331,6 @@ nlmsg_alloc (void) return nlmsg_alloc_size (get_default_page_size ()); } -/** - * Allocate a new netlink message with maximum payload size specified. - */ -struct nl_msg * -nlmsg_alloc_inherit (struct nlmsghdr *hdr) -{ - struct nl_msg *nm; - - nm = nlmsg_alloc (); - if (hdr) { - struct nlmsghdr *new = nm->nm_nlh; - - new->nlmsg_type = hdr->nlmsg_type; - new->nlmsg_flags = hdr->nlmsg_flags; - new->nlmsg_seq = hdr->nlmsg_seq; - new->nlmsg_pid = hdr->nlmsg_pid; - } - - return nm; -} - struct nl_msg * nlmsg_alloc_convert (struct nlmsghdr *hdr) { @@ -365,12 +344,14 @@ nlmsg_alloc_convert (struct nlmsghdr *hdr) struct nl_msg * nlmsg_alloc_simple (int nlmsgtype, int flags) { - struct nlmsghdr nlh = { - .nlmsg_type = nlmsgtype, - .nlmsg_flags = flags, - }; + struct nl_msg *nm; + struct nlmsghdr *new; - return nlmsg_alloc_inherit (&nlh); + nm = nlmsg_alloc (); + new = nm->nm_nlh; + new->nlmsg_type = nlmsgtype; + new->nlmsg_flags = flags; + return nm; } int diff --git a/src/platform/nm-netlink.h b/src/platform/nm-netlink.h index e20388e200..187685d824 100644 --- a/src/platform/nm-netlink.h +++ b/src/platform/nm-netlink.h @@ -314,8 +314,6 @@ struct nl_msg *nlmsg_alloc (void); struct nl_msg *nlmsg_alloc_size (size_t max); -struct nl_msg *nlmsg_alloc_inherit (struct nlmsghdr *hdr); - struct nl_msg *nlmsg_alloc_convert (struct nlmsghdr *hdr); struct nl_msg *nlmsg_alloc_simple (int nlmsgtype, int flags); -- cgit v1.2.1 From e9cf8b196deebdfcb4263bb6a6606f72d9cd0b6f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 15 Jul 2018 14:21:04 +0200 Subject: platform/trivial: reorder code --- src/platform/nm-netlink.c | 66 ++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/src/platform/nm-netlink.c b/src/platform/nm-netlink.c index c057acd9a1..cec3ba8fe9 100644 --- a/src/platform/nm-netlink.c +++ b/src/platform/nm-netlink.c @@ -259,20 +259,6 @@ nlmsg_reserve (struct nl_msg *n, size_t len, int pad) /*****************************************************************************/ -static int - get_default_page_size (void) -{ - static int val = 0; - int v; - - if (G_UNLIKELY (val == 0)) { - v = getpagesize (); - g_assert (v > 0); - val = v; - } - return val; -} - struct nlattr * nla_reserve (struct nl_msg *msg, int attrtype, int attrlen) { @@ -298,6 +284,22 @@ nla_reserve (struct nl_msg *msg, int attrtype, int attrlen) return nla; } +/*****************************************************************************/ + +static int +get_default_page_size (void) +{ + static int val = 0; + int v; + + if (G_UNLIKELY (val == 0)) { + v = getpagesize (); + g_assert (v > 0); + val = v; + } + return val; +} + struct nl_msg * nlmsg_alloc_size (size_t len) { @@ -354,6 +356,24 @@ nlmsg_alloc_simple (int nlmsgtype, int flags) return nm; } +void nlmsg_free (struct nl_msg *msg) +{ + if (!msg) + return; + + if (msg->nm_refcnt < 1) + g_return_if_reached (); + + msg->nm_refcnt--; + + if (msg->nm_refcnt <= 0) { + g_free (msg->nm_nlh); + g_slice_free (struct nl_msg, msg); + } +} + +/*****************************************************************************/ + int nlmsg_append (struct nl_msg *n, void *data, size_t len, int pad) { @@ -367,6 +387,8 @@ nlmsg_append (struct nl_msg *n, void *data, size_t len, int pad) return 0; } +/*****************************************************************************/ + int nlmsg_parse (struct nlmsghdr *nlh, int hdrlen, struct nlattr *tb[], int maxtype, const struct nla_policy *policy) @@ -620,22 +642,6 @@ errout: /*****************************************************************************/ -void nlmsg_free (struct nl_msg *msg) -{ - if (!msg) - return; - - if (msg->nm_refcnt < 1) - g_return_if_reached (); - - msg->nm_refcnt--; - - if (msg->nm_refcnt <= 0) { - g_free (msg->nm_nlh); - g_slice_free (struct nl_msg, msg); - } -} - int nlmsg_get_proto (struct nl_msg *msg) { -- cgit v1.2.1 From a30dd1eff041751fd26d5e3dba1a5f5693569ba5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 15 Jul 2018 14:26:03 +0200 Subject: platform/netlink: drop ref-counting for "struct nl_msg" It was unused. --- src/platform/nm-netlink.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/platform/nm-netlink.c b/src/platform/nm-netlink.c index cec3ba8fe9..7b79a772c0 100644 --- a/src/platform/nm-netlink.c +++ b/src/platform/nm-netlink.c @@ -52,7 +52,6 @@ struct nl_msg { struct ucred nm_creds; struct nlmsghdr * nm_nlh; size_t nm_size; - int nm_refcnt; }; struct nl_sock { @@ -310,7 +309,6 @@ nlmsg_alloc_size (size_t len) nm = g_slice_new0 (struct nl_msg); - nm->nm_refcnt = 1; nm->nm_protocol = -1; nm->nm_size = len; nm->nm_nlh = g_malloc0 (len); @@ -361,15 +359,8 @@ void nlmsg_free (struct nl_msg *msg) if (!msg) return; - if (msg->nm_refcnt < 1) - g_return_if_reached (); - - msg->nm_refcnt--; - - if (msg->nm_refcnt <= 0) { - g_free (msg->nm_nlh); - g_slice_free (struct nl_msg, msg); - } + g_free (msg->nm_nlh); + g_slice_free (struct nl_msg, msg); } /*****************************************************************************/ -- cgit v1.2.1 From 9740d3a68cb344674e2cd7db41a5af0fbe3065d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 Sep 2018 11:26:45 +0200 Subject: platform/netlink: assert that callbacks don't return positive error code --- src/platform/nm-netlink.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/platform/nm-netlink.c b/src/platform/nm-netlink.c index 7b79a772c0..d148b7c855 100644 --- a/src/platform/nm-netlink.c +++ b/src/platform/nm-netlink.c @@ -1109,6 +1109,10 @@ do { \ case NL_STOP: \ goto stop; \ default: \ + if (err >= 0) { \ + nm_assert_not_reached (); \ + err = -NLE_BUG; \ + } \ goto out; \ } \ } \ @@ -1216,11 +1220,12 @@ continue_reading: else if (err == NL_SKIP) goto skip; else if (err == NL_STOP) { - err = -e->error; + err = -nl_syserr2nlerr (e->error); goto out; } + nm_assert (err == NL_OK); } else { - err = -e->error; + err = -nl_syserr2nlerr (e->error); goto out; } } else @@ -1251,6 +1256,7 @@ out: if (interrupted) err = -NLE_DUMP_INTR; + nm_assert (err <= 0); return err ?: nrecv; } -- cgit v1.2.1 From 7042cd5e193faef230aefcf134b998749d7536f6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 4 Sep 2018 11:16:28 +0200 Subject: platform: cleanup error paths - drop "goto error_label" in favor of returning right away. At most places, there was no need to do any cleanup or the cleanup is handled via nm_auto(). - adjust the return types of wireguard functions to return a boolean success/failure, instead of some error code which we didn't use. - the change to _wireguard_get_link_properties() is intentional. This was wrong previously, because in _wireguard_get_link_properties() obj is always a newly created instance, and never has a genl family ID set. This will be improved later. --- src/platform/nm-linux-platform.c | 242 ++++++++++++------------------ src/platform/nm-netlink.c | 42 +++--- src/platform/wifi/nm-wifi-utils-nl80211.c | 22 ++- 3 files changed, 127 insertions(+), 179 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 70559c576a..7bcd5a6c77 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -557,18 +557,6 @@ _support_rta_pref_get (void) return _support_rta_pref >= 0; } -/***************************************************************************** - * Support Generic Netlink family - *****************************************************************************/ - -static int -_support_genl_family (struct nl_sock *genl, const char *name) -{ - int family_id = genl_ctrl_resolve (genl, name); - _LOG2D ("kernel-support: genetlink: %s: %s", name, family_id ? "detected" : "not detected"); - return family_id; -} - /****************************************************************** * Various utilities ******************************************************************/ @@ -1058,18 +1046,21 @@ _nl_addattr_l (struct nlmsghdr *n, * NMPObject/netlink functions ******************************************************************/ -#define _check_addr_or_errout(tb, attr, addr_len) \ +#define _check_addr_or_return_val(tb, attr, addr_len, ret_val) \ ({ \ const struct nlattr *__t = (tb)[(attr)]; \ \ if (__t) { \ if (nla_len (__t) != (addr_len)) { \ - goto errout; \ + return ret_val; \ } \ } \ !!__t; \ }) +#define _check_addr_or_return_null(tb, attr, addr_len) \ + _check_addr_or_return_val (tb, attr, addr_len, NULL) + /*****************************************************************************/ /* Copied and heavily modified from libnl3's inet6_parse_protinfo(). */ @@ -1096,20 +1087,19 @@ _parse_af_inet6 (NMPlatform *platform, gboolean token_valid = FALSE; gboolean addr_gen_mode_valid = FALSE; guint8 i6_addr_gen_mode_inv = 0; - gboolean success = FALSE; err = nla_parse_nested (tb, IFLA_INET6_MAX, attr, policy); if (err < 0) - goto errout; + return FALSE; if (tb[IFLA_INET6_CONF] && nla_len(tb[IFLA_INET6_CONF]) % 4) - goto errout; + return FALSE; if (tb[IFLA_INET6_STATS] && nla_len(tb[IFLA_INET6_STATS]) % 8) - goto errout; + return FALSE; if (tb[IFLA_INET6_ICMP6STATS] && nla_len(tb[IFLA_INET6_ICMP6STATS]) % 8) - goto errout; + return FALSE; - if (_check_addr_or_errout (tb, IFLA_INET6_TOKEN, sizeof (struct in6_addr))) { + if (_check_addr_or_return_val (tb, IFLA_INET6_TOKEN, sizeof (struct in6_addr), FALSE)) { nla_memcpy (&i6_token, tb[IFLA_INET6_TOKEN], sizeof (struct in6_addr)); token_valid = TRUE; } @@ -1125,12 +1115,11 @@ _parse_af_inet6 (NMPlatform *platform, if (i6_addr_gen_mode_inv == 0) { /* an inverse addrgenmode of zero is unexpected. We need to reserve zero * to signal "unset". */ - goto errout; + return FALSE; } addr_gen_mode_valid = TRUE; } - success = TRUE; if (token_valid) { *out_token_valid = token_valid; nm_utils_ipv6_interface_identifier_get_from_addr (out_token, &i6_token); @@ -1139,8 +1128,7 @@ _parse_af_inet6 (NMPlatform *platform, *out_addr_gen_mode_valid = addr_gen_mode_valid; *out_addr_gen_mode_inv = i6_addr_gen_mode_inv; } -errout: - return success; + return TRUE; } /*****************************************************************************/ @@ -1858,7 +1846,7 @@ struct _wireguard_device_buf { GArray *allowedips; }; -static int +static gboolean _wireguard_update_from_allowedips_nla (struct _wireguard_device_buf *buf, struct nlattr *allowedip_attr) { @@ -1872,11 +1860,11 @@ _wireguard_update_from_allowedips_nla (struct _wireguard_device_buf *buf, NMWireGuardAllowedIP *allowedip; NMWireGuardAllowedIP new_allowedip = {0}; int addr_len; - int ret; + int nlerr; - ret = nla_parse_nested (tba, WGALLOWEDIP_A_MAX, allowedip_attr, allowedip_policy); - if (ret) - goto errout; + nlerr = nla_parse_nested (tba, WGALLOWEDIP_A_MAX, allowedip_attr, allowedip_policy); + if (nlerr < 0) + return FALSE; g_array_append_val (buf->allowedips, new_allowedip); allowedip = &g_array_index (buf->allowedips, NMWireGuardAllowedIP, buf->allowedips->len - 1); @@ -1889,25 +1877,19 @@ _wireguard_update_from_allowedips_nla (struct _wireguard_device_buf *buf, addr_len = sizeof (in_addr_t); else if (allowedip->family == AF_INET6) addr_len = sizeof (struct in6_addr); - else { - ret = -EAFNOSUPPORT; - goto errout; - } + else + return FALSE; - ret = -EMSGSIZE; - _check_addr_or_errout (tba, WGALLOWEDIP_A_IPADDR, addr_len); + _check_addr_or_return_val (tba, WGALLOWEDIP_A_IPADDR, addr_len, FALSE); if (tba[WGALLOWEDIP_A_IPADDR]) nla_memcpy (&allowedip->ip, tba[WGALLOWEDIP_A_IPADDR], addr_len); - if (tba[WGALLOWEDIP_A_CIDR_MASK]) allowedip->mask = nla_get_u8 (tba[WGALLOWEDIP_A_CIDR_MASK]); - ret = 0; -errout: - return ret; + return TRUE; } -static int +static gboolean _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, struct nlattr *peer_attr) { @@ -1923,69 +1905,60 @@ _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, [WGPEER_A_ALLOWEDIPS] = { .type = NLA_NESTED }, }; struct nlattr *tbp[WGPEER_A_MAX + 1]; - NMWireGuardPeer * const last = buf->peers->len ? &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1) : NULL; + NMWireGuardPeer *const last = buf->peers->len ? &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1) : NULL; NMWireGuardPeer *peer; - NMWireGuardPeer new_peer = {0}; - int ret; + NMWireGuardPeer new_peer = { 0 }; - if (nla_parse_nested (tbp, WGPEER_A_MAX, peer_attr, peer_policy)) { - ret = -EBADMSG; - goto errout; - } + if (nla_parse_nested (tbp, WGPEER_A_MAX, peer_attr, peer_policy) < 0) + return FALSE; - if (!tbp[WGPEER_A_PUBLIC_KEY]) { - ret = -EBADMSG; - goto errout; - } + if (!tbp[WGPEER_A_PUBLIC_KEY]) + return FALSE; /* a peer with the same public key as last peer is just a continuation for extra AllowedIPs */ - if (last && !memcmp (nla_data (tbp[WGPEER_A_PUBLIC_KEY]), last->public_key, sizeof (last->public_key))) { + if ( last + && !memcmp (nla_data (tbp[WGPEER_A_PUBLIC_KEY]), last->public_key, sizeof (last->public_key))) peer = last; - goto add_allowedips; - } - - /* otherwise, start a new peer */ - g_array_append_val (buf->peers, new_peer); - peer = &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1); - - nla_memcpy (&peer->public_key, tbp[WGPEER_A_PUBLIC_KEY], sizeof (peer->public_key)); + else { + /* otherwise, start a new peer */ + g_array_append_val (buf->peers, new_peer); + peer = &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1); + + nla_memcpy (&peer->public_key, tbp[WGPEER_A_PUBLIC_KEY], sizeof (peer->public_key)); + + if (tbp[WGPEER_A_PRESHARED_KEY]) + nla_memcpy (&peer->preshared_key, tbp[WGPEER_A_PRESHARED_KEY], sizeof (peer->preshared_key)); + if (tbp[WGPEER_A_ENDPOINT]) { + struct sockaddr *addr = nla_data (tbp[WGPEER_A_ENDPOINT]); + if (addr->sa_family == AF_INET) + nla_memcpy (&peer->endpoint.addr4, tbp[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr4)); + else if (addr->sa_family == AF_INET6) + nla_memcpy (&peer->endpoint.addr6, tbp[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr6)); + } + if (tbp[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]) + peer->persistent_keepalive_interval = nla_get_u64 (tbp[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]); + if (tbp[WGPEER_A_LAST_HANDSHAKE_TIME]) + nla_memcpy (&peer->last_handshake_time, tbp[WGPEER_A_LAST_HANDSHAKE_TIME], sizeof (peer->last_handshake_time)); + if (tbp[WGPEER_A_RX_BYTES]) + peer->rx_bytes = nla_get_u64 (tbp[WGPEER_A_RX_BYTES]); + if (tbp[WGPEER_A_TX_BYTES]) + peer->tx_bytes = nla_get_u64 (tbp[WGPEER_A_TX_BYTES]); - if (tbp[WGPEER_A_PRESHARED_KEY]) - nla_memcpy (&peer->preshared_key, tbp[WGPEER_A_PRESHARED_KEY], sizeof (peer->preshared_key)); - if (tbp[WGPEER_A_ENDPOINT]) { - struct sockaddr *addr = nla_data (tbp[WGPEER_A_ENDPOINT]); - if (addr->sa_family == AF_INET) - nla_memcpy (&peer->endpoint.addr4, tbp[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr4)); - else if (addr->sa_family == AF_INET6) - nla_memcpy (&peer->endpoint.addr6, tbp[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr6)); + peer->allowedips = NULL; + peer->allowedips_len = 0; } - if (tbp[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]) - peer->persistent_keepalive_interval = nla_get_u64 (tbp[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]); - if (tbp[WGPEER_A_LAST_HANDSHAKE_TIME]) - nla_memcpy (&peer->last_handshake_time, tbp[WGPEER_A_LAST_HANDSHAKE_TIME], sizeof (peer->last_handshake_time)); - if (tbp[WGPEER_A_RX_BYTES]) - peer->rx_bytes = nla_get_u64 (tbp[WGPEER_A_RX_BYTES]); - if (tbp[WGPEER_A_TX_BYTES]) - peer->tx_bytes = nla_get_u64 (tbp[WGPEER_A_TX_BYTES]); - - peer->allowedips = NULL; - peer->allowedips_len = 0; -add_allowedips: if (tbp[WGPEER_A_ALLOWEDIPS]) { struct nlattr *attr; int rem; nla_for_each_nested (attr, tbp[WGPEER_A_ALLOWEDIPS], rem) { - ret = _wireguard_update_from_allowedips_nla (buf, attr); - if (ret) - goto errout; + if (!_wireguard_update_from_allowedips_nla (buf, attr)) + return FALSE; } } - ret = 0; -errout: - return ret; + return TRUE; } static int @@ -2005,11 +1978,11 @@ _wireguard_get_device_cb (struct nl_msg *msg, void *arg) struct nlattr *tbd[WGDEVICE_A_MAX + 1]; NMPlatformLnkWireGuard *props = &buf->obj->lnk_wireguard; struct nlmsghdr *nlh = nlmsg_hdr (msg); - int ret; + int nlerr; - ret = genlmsg_parse (nlh, 0, tbd, WGDEVICE_A_MAX, device_policy); - if (ret) - goto errout; + nlerr = genlmsg_parse (nlh, 0, tbd, WGDEVICE_A_MAX, device_policy); + if (nlerr < 0) + return NL_SKIP; if (tbd[WGDEVICE_A_PRIVATE_KEY]) nla_memcpy (props->private_key, tbd[WGDEVICE_A_PRIVATE_KEY], sizeof (props->private_key)); @@ -2025,15 +1998,12 @@ _wireguard_get_device_cb (struct nl_msg *msg, void *arg) int rem; nla_for_each_nested (attr, tbd[WGDEVICE_A_PEERS], rem) { - ret = _wireguard_update_from_peers_nla (buf, attr); - if (ret) - goto errout; + if (!_wireguard_update_from_peers_nla (buf, attr)) + return NL_SKIP; } } return NL_OK; -errout: - return NL_SKIP; } static gboolean _wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link, NMPObject *obj); @@ -2084,7 +2054,6 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr const char *nl_info_kind = NULL; int err; nm_auto_nmpobj NMPObject *obj = NULL; - NMPObject *obj_result = NULL; gboolean completed_from_cache_val = FALSE; gboolean *completed_from_cache = cache ? &completed_from_cache_val : NULL; const NMPObject *link_cached = NULL; @@ -2105,17 +2074,17 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr obj = nmp_object_new_link (ifi->ifi_index); if (id_only) - goto id_only_handled; + return g_steal_pointer (&obj); err = nlmsg_parse (nlh, sizeof (*ifi), tb, IFLA_MAX, policy); if (err < 0) - goto errout; + return NULL; if (!tb[IFLA_IFNAME]) - goto errout; + return NULL; nla_strlcpy(obj->link.name, tb[IFLA_IFNAME], IFNAMSIZ); if (!obj->link.name[0]) - goto errout; + return NULL; if (!tb[IFLA_MTU]) { /* Kernel has two places that send RTM_GETLINK messages: @@ -2130,14 +2099,14 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr * To some extent this is a hack and correct approach is to * merge objects per-field. */ - goto errout; + return NULL; } obj->link.mtu = nla_get_u32 (tb[IFLA_MTU]); if (tb[IFLA_LINKINFO]) { err = nla_parse_nested (li, IFLA_INFO_MAX, tb[IFLA_LINKINFO], policy_link_info); if (err < 0) - goto errout; + return NULL; if (li[IFLA_INFO_KIND]) nl_info_kind = nla_get_string (li[IFLA_INFO_KIND]); @@ -2357,13 +2326,8 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr } } - obj->_link.netlink.is_in_netlink = TRUE; -id_only_handled: - obj_result = obj; - obj = NULL; -errout: - return obj_result; + return g_steal_pointer (&obj); } /* Copied and heavily modified from libnl3's addr_msg_parser(). */ @@ -2380,7 +2344,6 @@ _new_from_nl_addr (struct nlmsghdr *nlh, gboolean id_only) int err; gboolean is_v4; nm_auto_nmpobj NMPObject *obj = NULL; - NMPObject *obj_result = NULL; int addr_len; guint32 lifetime, preferred, timestamp; @@ -2389,19 +2352,19 @@ _new_from_nl_addr (struct nlmsghdr *nlh, gboolean id_only) ifa = nlmsg_data(nlh); if (!NM_IN_SET (ifa->ifa_family, AF_INET, AF_INET6)) - goto errout; + return NULL; is_v4 = ifa->ifa_family == AF_INET; err = nlmsg_parse (nlh, sizeof(*ifa), tb, IFA_MAX, policy); if (err < 0) - goto errout; + return NULL; addr_len = is_v4 ? sizeof (in_addr_t) : sizeof (struct in6_addr); if (ifa->ifa_prefixlen > (is_v4 ? 32 : 128)) - goto errout; + return NULL; /*****************************************************************/ @@ -2410,8 +2373,8 @@ _new_from_nl_addr (struct nlmsghdr *nlh, gboolean id_only) obj->ip_address.ifindex = ifa->ifa_index; obj->ip_address.plen = ifa->ifa_prefixlen; - _check_addr_or_errout (tb, IFA_ADDRESS, addr_len); - _check_addr_or_errout (tb, IFA_LOCAL, addr_len); + _check_addr_or_return_null (tb, IFA_ADDRESS, addr_len); + _check_addr_or_return_null (tb, IFA_LOCAL, addr_len); if (is_v4) { /* For IPv4, kernel omits IFA_LOCAL/IFA_ADDRESS if (and only if) they * are effectively 0.0.0.0 (all-zero). */ @@ -2475,10 +2438,7 @@ _new_from_nl_addr (struct nlmsghdr *nlh, gboolean id_only) &obj->ip_address.lifetime, &obj->ip_address.preferred); - obj_result = obj; - obj = NULL; -errout: - return obj_result; + return g_steal_pointer (&obj); } /* Copied and heavily modified from libnl3's rtnl_route_parse() and parse_multipath(). */ @@ -2501,7 +2461,6 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) int err; gboolean is_v4; nm_auto_nmpobj NMPObject *obj = NULL; - NMPObject *obj_result = NULL; int addr_len; struct { gboolean is_present; @@ -2520,14 +2479,14 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) *****************************************************************/ if (!NM_IN_SET (rtm->rtm_family, AF_INET, AF_INET6)) - goto errout; + return NULL; if (rtm->rtm_type != RTN_UNICAST) - goto errout; + return NULL; err = nlmsg_parse (nlh, sizeof (struct rtmsg), tb, RTA_MAX, policy); if (err < 0) - goto errout; + return NULL; /*****************************************************************/ @@ -2537,7 +2496,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) : sizeof (struct in6_addr); if (rtm->rtm_dst_len > (is_v4 ? 32 : 128)) - goto errout; + return NULL; /***************************************************************** * parse nexthops. Only handle routes with one nh. @@ -2553,7 +2512,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) if (nh.is_present) { /* we don't support multipath routes. */ - goto errout; + return NULL; } nh.is_present = TRUE; @@ -2567,9 +2526,9 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) rtnh->rtnh_len - sizeof (*rtnh), policy); if (err < 0) - goto errout; + return NULL; - if (_check_addr_or_errout (ntb, RTA_GATEWAY, addr_len)) + if (_check_addr_or_return_null (ntb, RTA_GATEWAY, addr_len)) memcpy (&nh.gateway, nla_data (ntb[RTA_GATEWAY]), addr_len); } @@ -2586,7 +2545,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) if (tb[RTA_OIF]) ifindex = nla_get_u32 (tb[RTA_OIF]); - if (_check_addr_or_errout (tb, RTA_GATEWAY, addr_len)) + if (_check_addr_or_return_null (tb, RTA_GATEWAY, addr_len)) memcpy (&gateway, nla_data (tb[RTA_GATEWAY]), addr_len); if (!nh.is_present) { @@ -2600,10 +2559,10 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) * verify that it is a duplicate and ignore old-style nexthop. */ if ( nh.ifindex != ifindex || memcmp (&nh.gateway, &gateway, addr_len) != 0) - goto errout; + return NULL; } } else if (!nh.is_present) - goto errout; + return NULL; /*****************************************************************/ @@ -2622,7 +2581,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) err = nla_parse_nested (mtb, RTAX_MAX, tb[RTA_METRICS], rtax_policy); if (err < 0) - goto errout; + return NULL; if (mtb[RTAX_LOCK]) lock = nla_get_u32 (mtb[RTAX_LOCK]); @@ -2650,7 +2609,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) obj->ip_route.ifindex = nh.ifindex; - if (_check_addr_or_errout (tb, RTA_DST, addr_len)) + if (_check_addr_or_return_null (tb, RTA_DST, addr_len)) memcpy (obj->ip_route.network_ptr, nla_data (tb[RTA_DST]), addr_len); obj->ip_route.plen = rtm->rtm_dst_len; @@ -2666,7 +2625,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) if (is_v4) obj->ip4_route.scope_inv = nm_platform_route_scope_inv (rtm->rtm_scope); - if (_check_addr_or_errout (tb, RTA_PREFSRC, addr_len)) { + if (_check_addr_or_return_null (tb, RTA_PREFSRC, addr_len)) { if (is_v4) memcpy (&obj->ip4_route.pref_src, nla_data (tb[RTA_PREFSRC]), addr_len); else @@ -2677,7 +2636,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) obj->ip4_route.tos = rtm->rtm_tos; else { if (tb[RTA_SRC]) { - _check_addr_or_errout (tb, RTA_SRC, addr_len); + _check_addr_or_return_null (tb, RTA_SRC, addr_len); memcpy (&obj->ip6_route.src, nla_data (tb[RTA_SRC]), addr_len); } obj->ip6_route.src_plen = rtm->rtm_src_len; @@ -2707,10 +2666,7 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) obj->ip_route.r_rtm_flags = rtm->rtm_flags; obj->ip_route.rt_source = nmp_utils_ip_config_source_from_rtprot (rtm->rtm_protocol); - obj_result = obj; - obj = NULL; -errout: - return obj_result; + return g_steal_pointer (&obj); } static NMPObject * @@ -6531,20 +6487,17 @@ _wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link .valid_arg = &buf, }; guint i, j; + int wireguard_family_id; - if (!obj->_link.wireguard_family_id) - obj->_link.wireguard_family_id = _support_genl_family (priv->genl, "wireguard"); - - if (!obj->_link.wireguard_family_id) { + wireguard_family_id = genl_ctrl_resolve (priv->genl, "wireguard"); + if (wireguard_family_id < 0) { _LOGD ("wireguard: kernel support not available for wireguard link %s", link->name); goto err; } msg = nlmsg_alloc (); - if (!msg) - goto err; - if (!genlmsg_put (msg, NL_AUTO_PORT, NL_AUTO_SEQ, obj->_link.wireguard_family_id, + if (!genlmsg_put (msg, NL_AUTO_PORT, NL_AUTO_SEQ, wireguard_family_id, 0, NLM_F_DUMP, WG_CMD_GET_DEVICE, 1)) goto err; @@ -6559,6 +6512,7 @@ _wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link /* have each peer point to its own chunk of the allowedips buffer */ for (i = 0, j = 0; i < buf.peers->len; i++) { NMWireGuardPeer *p = &g_array_index (buf.peers, NMWireGuardPeer, i); + p->allowedips = &g_array_index (buf.allowedips, NMWireGuardAllowedIP, j); j += p->allowedips_len; } diff --git a/src/platform/nm-netlink.c b/src/platform/nm-netlink.c index d148b7c855..fa8a812cec 100644 --- a/src/platform/nm-netlink.c +++ b/src/platform/nm-netlink.c @@ -791,7 +791,7 @@ int genl_ctrl_resolve (struct nl_sock *sk, const char *name) { nm_auto_nlmsg struct nl_msg *msg = NULL; - int result = -ENOMEM; + int nlerr; gint32 response_data = -1; const struct nl_cb cb = { .valid_cb = _genl_parse_getfamily, @@ -802,31 +802,29 @@ genl_ctrl_resolve (struct nl_sock *sk, const char *name) if (!genlmsg_put (msg, NL_AUTO_PORT, NL_AUTO_SEQ, GENL_ID_CTRL, 0, 0, CTRL_CMD_GETFAMILY, 1)) - goto out; + return -ENOMEM; - if (nla_put_string (msg, CTRL_ATTR_FAMILY_NAME, name) < 0) - goto out; + nlerr = nla_put_string (msg, CTRL_ATTR_FAMILY_NAME, name); + if (nlerr < 0) + return nlerr; - result = nl_send_auto (sk, msg); - if (result < 0) - goto out; + nlerr = nl_send_auto (sk, msg); + if (nlerr < 0) + return nlerr; - result = nl_recvmsgs (sk, &cb); - if (result < 0) - goto out; + nlerr = nl_recvmsgs (sk, &cb); + if (nlerr < 0) + return nlerr; /* If search was successful, request may be ACKed after data */ - result = nl_wait_for_ack (sk, NULL); - if (result < 0) - goto out; + nlerr = nl_wait_for_ack (sk, NULL); + if (nlerr < 0) + return nlerr; - if (response_data > 0) - result = response_data; - else - result = -ENOENT; + if (response_data < 0) + return -NLE_UNSPEC; -out: - return result; + return response_data; } /*****************************************************************************/ @@ -1312,7 +1310,7 @@ nl_send_iovec (struct nl_sock *sk, struct nl_msg *msg, struct iovec *iov, unsign memcpy(CMSG_DATA(cmsg), creds, sizeof (struct ucred)); } - return nl_sendmsg(sk, msg, &hdr); + return nl_sendmsg (sk, msg, &hdr); } void @@ -1349,9 +1347,9 @@ nl_send (struct nl_sock *sk, struct nl_msg *msg) int nl_send_auto(struct nl_sock *sk, struct nl_msg *msg) { - nl_complete_msg(sk, msg); + nl_complete_msg (sk, msg); - return nl_send(sk, msg); + return nl_send (sk, msg); } int diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c index 1728ad1809..b3bb2bb6e6 100644 --- a/src/platform/wifi/nm-wifi-utils-nl80211.c +++ b/src/platform/wifi/nm-wifi-utils-nl80211.c @@ -929,7 +929,7 @@ nm_wifi_utils_nl80211_class_init (NMWifiUtilsNl80211Class *klass) NMWifiUtils * nm_wifi_utils_nl80211_new (int ifindex, struct nl_sock *genl) { - NMWifiUtilsNl80211 *nl80211; + gs_unref_object NMWifiUtilsNl80211 *nl80211 = NULL; nm_auto_nlmsg struct nl_msg *msg = NULL; struct nl80211_device_info device_info = {}; char ifname[IFNAMSIZ]; @@ -951,7 +951,7 @@ nm_wifi_utils_nl80211_new (int ifindex, struct nl_sock *genl) nl80211->id = genl_ctrl_resolve (nl80211->nl_sock, "nl80211"); if (nl80211->id < 0) { _LOGD (LOGD_WIFI, "genl_ctrl_resolve: failed to resolve \"nl80211\""); - goto error; + return NULL; } nl80211->phy = -1; @@ -963,42 +963,42 @@ nm_wifi_utils_nl80211_new (int ifindex, struct nl_sock *genl) _LOGD (LOGD_PLATFORM | LOGD_WIFI, "(%s): NL80211_CMD_GET_WIPHY request failed", ifname); - goto error; + return NULL; } if (!device_info.success) { _LOGD (LOGD_PLATFORM | LOGD_WIFI, "(%s): NL80211_CMD_GET_WIPHY request indicated failure", ifname); - goto error; + return NULL; } if (!device_info.supported) { _LOGD (LOGD_PLATFORM | LOGD_WIFI, "(%s): driver does not fully support nl80211, falling back to WEXT", ifname); - goto error; + return NULL; } if (!device_info.can_scan_ssid) { _LOGE (LOGD_PLATFORM | LOGD_WIFI, "(%s): driver does not support SSID scans", ifname); - goto error; + return NULL; } if (device_info.num_freqs == 0 || device_info.freqs == NULL) { nm_log_err (LOGD_PLATFORM | LOGD_WIFI, "(%s): driver reports no supported frequencies", ifname); - goto error; + return NULL; } if (device_info.caps == 0) { _LOGE (LOGD_PLATFORM | LOGD_WIFI, "(%s): driver doesn't report support of any encryption", ifname); - goto error; + return NULL; } nl80211->phy = device_info.phy; @@ -1010,9 +1010,5 @@ nm_wifi_utils_nl80211_new (int ifindex, struct nl_sock *genl) _LOGI (LOGD_PLATFORM | LOGD_WIFI, "(%s): using nl80211 for WiFi device control", ifname); - return (NMWifiUtils *) nl80211; - -error: - g_object_unref (nl80211); - return NULL; + return (NMWifiUtils *) g_steal_pointer (&nl80211); } -- cgit v1.2.1 From f99ee135d1145a3cfd6a745efe2670ee1a4e8969 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 4 Sep 2018 14:48:59 +0200 Subject: platform: let _lookup_cached_link() also return cached links that are not in netlink The _lookup_cached_link() function, should not skip over links which are currently in the cache, but not in netlink. Instead, let the callers skip them, as they see fit. No change in behavior, because the few callers now explicitly check for this. --- src/platform/nm-linux-platform.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 7bcd5a6c77..87e3c00216 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -812,19 +812,21 @@ _addrtime_get_lifetimes (guint32 timestamp, /*****************************************************************************/ static const NMPObject * -_lookup_cached_link (const NMPCache *cache, int ifindex, gboolean *completed_from_cache, const NMPObject **link_cached) +_lookup_cached_link (const NMPCache *cache, + int ifindex, + gboolean *completed_from_cache, + const NMPObject **link_cached) { const NMPObject *obj; nm_assert (completed_from_cache && link_cached); if (!*completed_from_cache) { - obj = ifindex > 0 && cache ? nmp_cache_lookup_link (cache, ifindex) : NULL; + obj = ifindex > 0 && cache + ? nmp_cache_lookup_link (cache, ifindex) + : NULL; - if (obj && obj->_link.netlink.is_in_netlink) - *link_cached = obj; - else - *link_cached = NULL; + *link_cached = obj; *completed_from_cache = TRUE; } return *link_cached; @@ -895,6 +897,7 @@ _linktype_get_type (NMPlatform *platform, * when moving interfce to other netns). Thus here there is a tiny potential * of messing stuff up. */ if ( obj + && obj->_link.netlink.is_in_netlink && !NM_IN_SET (obj->link.type, NM_LINK_TYPE_UNKNOWN, NM_LINK_TYPE_NONE) && nm_streq (ifname, obj->link.name) && ( !kind @@ -2240,7 +2243,8 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr || !af_inet6_addr_gen_mode_valid || !tb[IFLA_STATS64])) { _lookup_cached_link (cache, obj->link.ifindex, completed_from_cache, &link_cached); - if (link_cached) { + if ( link_cached + && link_cached->_link.netlink.is_in_netlink) { if ( lnk_data_complete_from_cache && link_cached->link.type == obj->link.type && link_cached->_link.netlink.lnk -- cgit v1.2.1 From 989bdaec6321bc3108850318ffea13ad67ee0987 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 4 Sep 2018 16:43:44 +0200 Subject: platform/trivial: move code in nm-linux-platform.c around Move NMLinuxPlatformPrivate earlier. In the past, I moved the declaration of NMLinuxPlatformPrivate after utility functions which are independent from platform instance. However, parsing netlink messages actually requires NMLinuxPlatformPrivate, because we want to access the "genl" socket. So, move the types to the beginning of the file, like we do for most other source files. --- src/platform/nm-linux-platform.c | 422 +++++++++++++++++++-------------------- 1 file changed, 209 insertions(+), 213 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 87e3c00216..418186b360 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -256,62 +256,6 @@ struct _ifla_vf_vlan_info { /*****************************************************************************/ -#define _NMLOG_PREFIX_NAME "platform-linux" -#define _NMLOG_DOMAIN LOGD_PLATFORM -#define _NMLOG2_DOMAIN LOGD_PLATFORM -#define _NMLOG(level, ...) _LOG ( level, _NMLOG_DOMAIN, platform, __VA_ARGS__) -#define _NMLOG_err(errsv, level, ...) _LOG_err (errsv, level, _NMLOG_DOMAIN, platform, __VA_ARGS__) -#define _NMLOG2(level, ...) _LOG ( level, _NMLOG2_DOMAIN, NULL, __VA_ARGS__) -#define _NMLOG2_err(errsv, level, ...) _LOG_err (errsv, level, _NMLOG2_DOMAIN, NULL, __VA_ARGS__) - -#define _LOG_print(__level, __domain, __errsv, self, ...) \ - G_STMT_START { \ - char __prefix[32]; \ - const char *__p_prefix = _NMLOG_PREFIX_NAME; \ - NMPlatform *const __self = (self); \ - \ - if (__self && nm_platform_get_log_with_ptr (__self)) { \ - g_snprintf (__prefix, sizeof (__prefix), "%s[%p]", _NMLOG_PREFIX_NAME, __self); \ - __p_prefix = __prefix; \ - } \ - _nm_log (__level, __domain, __errsv, NULL, NULL, \ - "%s: " _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ - __p_prefix _NM_UTILS_MACRO_REST (__VA_ARGS__)); \ - } G_STMT_END - -#define _LOG(level, domain, self, ...) \ - G_STMT_START { \ - const NMLogLevel __level = (level); \ - const NMLogDomain __domain = (domain); \ - \ - if (nm_logging_enabled (__level, __domain)) { \ - _LOG_print (__level, __domain, 0, self, __VA_ARGS__); \ - } \ - } G_STMT_END - -#define _LOG_err(errsv, level, domain, self, ...) \ - G_STMT_START { \ - const NMLogLevel __level = (level); \ - const NMLogDomain __domain = (domain); \ - \ - if (nm_logging_enabled (__level, __domain)) { \ - int __errsv = (errsv); \ - \ - /* The %m format specifier (GNU extension) would alread allow you to specify the error - * message conveniently (and nm_log would get that right too). But we don't want to depend - * on that, so instead append the message at the end. - * Currently users are expected not to use %m in the format string. */ \ - _LOG_print (__level, __domain, __errsv, self, \ - _NM_UTILS_MACRO_FIRST (__VA_ARGS__) ": %s (%d)" \ - _NM_UTILS_MACRO_REST (__VA_ARGS__), \ - g_strerror (__errsv), __errsv); \ - } \ - } G_STMT_END - -/****************************************************************** - * Forward declarations and enums - ******************************************************************/ - typedef enum { INFINIBAND_ACTION_CREATE_CHILD, INFINIBAND_ACTION_DELETE_CHILD, @@ -388,6 +332,133 @@ typedef enum { WAIT_FOR_NL_RESPONSE_RESULT_FAILED_SETNS, } WaitForNlResponseResult; +typedef enum { + DELAYED_ACTION_RESPONSE_TYPE_VOID = 0, + DELAYED_ACTION_RESPONSE_TYPE_REFRESH_ALL_IN_PROGRESS = 1, + DELAYED_ACTION_RESPONSE_TYPE_ROUTE_GET = 2, +} DelayedActionWaitForNlResponseType; + +typedef struct { + guint32 seq_number; + WaitForNlResponseResult seq_result; + DelayedActionWaitForNlResponseType response_type; + gint64 timeout_abs_ns; + WaitForNlResponseResult *out_seq_result; + char **out_errmsg; + union { + int *out_refresh_all_in_progress; + NMPObject **out_route_get; + gpointer out_data; + } response; +} DelayedActionWaitForNlResponseData; + +/*****************************************************************************/ + +typedef struct { + struct nl_sock *genl; + + struct nl_sock *nlh; + guint32 nlh_seq_next; +#if NM_MORE_LOGGING + guint32 nlh_seq_last_handled; +#endif + guint32 nlh_seq_last_seen; + GIOChannel *event_channel; + guint event_id; + + bool pruning[_DELAYED_ACTION_IDX_REFRESH_ALL_NUM]; + + bool sysctl_get_warned; + GHashTable *sysctl_get_prev_values; + + NMUdevClient *udev_client; + + struct { + /* which delayed actions are scheduled, as marked in @flags. + * Some types have additional arguments in the fields below. */ + DelayedActionType flags; + + /* counter that a refresh all action is in progress, separated + * by type. */ + int refresh_all_in_progress[_DELAYED_ACTION_IDX_REFRESH_ALL_NUM]; + + GPtrArray *list_master_connected; + GPtrArray *list_refresh_link; + GArray *list_wait_for_nl_response; + + int is_handling; + } delayed_action; +} NMLinuxPlatformPrivate; + +struct _NMLinuxPlatform { + NMPlatform parent; + NMLinuxPlatformPrivate _priv; +}; + +struct _NMLinuxPlatformClass { + NMPlatformClass parent; +}; + +G_DEFINE_TYPE (NMLinuxPlatform, nm_linux_platform, NM_TYPE_PLATFORM) + +#define NM_LINUX_PLATFORM_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMLinuxPlatform, NM_IS_LINUX_PLATFORM, NMPlatform) + +/*****************************************************************************/ + +#define _NMLOG_PREFIX_NAME "platform-linux" +#define _NMLOG_DOMAIN LOGD_PLATFORM +#define _NMLOG2_DOMAIN LOGD_PLATFORM +#define _NMLOG(level, ...) _LOG ( level, _NMLOG_DOMAIN, platform, __VA_ARGS__) +#define _NMLOG_err(errsv, level, ...) _LOG_err (errsv, level, _NMLOG_DOMAIN, platform, __VA_ARGS__) +#define _NMLOG2(level, ...) _LOG ( level, _NMLOG2_DOMAIN, NULL, __VA_ARGS__) +#define _NMLOG2_err(errsv, level, ...) _LOG_err (errsv, level, _NMLOG2_DOMAIN, NULL, __VA_ARGS__) + +#define _LOG_print(__level, __domain, __errsv, self, ...) \ + G_STMT_START { \ + char __prefix[32]; \ + const char *__p_prefix = _NMLOG_PREFIX_NAME; \ + NMPlatform *const __self = (self); \ + \ + if (__self && nm_platform_get_log_with_ptr (__self)) { \ + g_snprintf (__prefix, sizeof (__prefix), "%s[%p]", _NMLOG_PREFIX_NAME, __self); \ + __p_prefix = __prefix; \ + } \ + _nm_log (__level, __domain, __errsv, NULL, NULL, \ + "%s: " _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + __p_prefix _NM_UTILS_MACRO_REST (__VA_ARGS__)); \ + } G_STMT_END + +#define _LOG(level, domain, self, ...) \ + G_STMT_START { \ + const NMLogLevel __level = (level); \ + const NMLogDomain __domain = (domain); \ + \ + if (nm_logging_enabled (__level, __domain)) { \ + _LOG_print (__level, __domain, 0, self, __VA_ARGS__); \ + } \ + } G_STMT_END + +#define _LOG_err(errsv, level, domain, self, ...) \ + G_STMT_START { \ + const NMLogLevel __level = (level); \ + const NMLogDomain __domain = (domain); \ + \ + if (nm_logging_enabled (__level, __domain)) { \ + int __errsv = (errsv); \ + \ + /* The %m format specifier (GNU extension) would alread allow you to specify the error + * message conveniently (and nm_log would get that right too). But we don't want to depend + * on that, so instead append the message at the end. + * Currently users are expected not to use %m in the format string. */ \ + _LOG_print (__level, __domain, __errsv, self, \ + _NM_UTILS_MACRO_FIRST (__VA_ARGS__) ": %s (%d)" \ + _NM_UTILS_MACRO_REST (__VA_ARGS__), \ + g_strerror (__errsv), __errsv); \ + } \ + } G_STMT_END + +/*****************************************************************************/ + static void delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gpointer user_data); static gboolean delayed_action_handle_all (NMPlatform *platform, gboolean read_netlink); static void do_request_link_no_delayed_actions (NMPlatform *platform, int ifindex, const char *name); @@ -2009,7 +2080,64 @@ _wireguard_get_device_cb (struct nl_msg *msg, void *arg) return NL_OK; } -static gboolean _wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link, NMPObject *obj); +static gboolean +_wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link, NMPObject *obj) +{ + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); + nm_auto_nlmsg struct nl_msg *msg = NULL; + struct _wireguard_device_buf buf = { + .obj = obj, + .peers = g_array_new (FALSE, FALSE, sizeof (NMWireGuardPeer)), + .allowedips = g_array_new (FALSE, FALSE, sizeof (NMWireGuardAllowedIP)), + }; + struct nl_cb cb = { + .valid_cb = _wireguard_get_device_cb, + .valid_arg = &buf, + }; + guint i, j; + int wireguard_family_id; + + wireguard_family_id = genl_ctrl_resolve (priv->genl, "wireguard"); + if (wireguard_family_id < 0) { + _LOGD ("wireguard: kernel support not available for wireguard link %s", link->name); + goto err; + } + + msg = nlmsg_alloc (); + + if (!genlmsg_put (msg, NL_AUTO_PORT, NL_AUTO_SEQ, wireguard_family_id, + 0, NLM_F_DUMP, WG_CMD_GET_DEVICE, 1)) + goto err; + + NLA_PUT_U32 (msg, WGDEVICE_A_IFINDEX, link->ifindex); + + if (nl_send_auto (priv->genl, msg) < 0) + goto err; + + if (nl_recvmsgs (priv->genl, &cb) < 0) + goto err; + + /* have each peer point to its own chunk of the allowedips buffer */ + for (i = 0, j = 0; i < buf.peers->len; i++) { + NMWireGuardPeer *p = &g_array_index (buf.peers, NMWireGuardPeer, i); + + p->allowedips = &g_array_index (buf.allowedips, NMWireGuardAllowedIP, j); + j += p->allowedips_len; + } + /* drop the wrapper (but also the buffer if no peer points to it) */ + g_array_free (buf.allowedips, buf.peers->len ? FALSE : TRUE); + + obj->_lnk_wireguard.peers_len = buf.peers->len; + obj->_lnk_wireguard.peers = (NMWireGuardPeer *) g_array_free (buf.peers, FALSE); + + return TRUE; + +err: +nla_put_failure: + g_array_free (buf.peers, TRUE); + g_array_free (buf.allowedips, TRUE); + return FALSE; +} /*****************************************************************************/ @@ -3347,101 +3475,6 @@ nla_put_failure: g_return_val_if_reached (NULL); } -/****************************************************************** - * NMPlatform types and functions - ******************************************************************/ - -typedef enum { - DELAYED_ACTION_RESPONSE_TYPE_VOID = 0, - DELAYED_ACTION_RESPONSE_TYPE_REFRESH_ALL_IN_PROGRESS = 1, - DELAYED_ACTION_RESPONSE_TYPE_ROUTE_GET = 2, -} DelayedActionWaitForNlResponseType; - -typedef struct { - guint32 seq_number; - WaitForNlResponseResult seq_result; - DelayedActionWaitForNlResponseType response_type; - gint64 timeout_abs_ns; - WaitForNlResponseResult *out_seq_result; - char **out_errmsg; - union { - int *out_refresh_all_in_progress; - NMPObject **out_route_get; - gpointer out_data; - } response; -} DelayedActionWaitForNlResponseData; - -typedef struct { - struct nl_sock *genl; - - struct nl_sock *nlh; - guint32 nlh_seq_next; -#if NM_MORE_LOGGING - guint32 nlh_seq_last_handled; -#endif - guint32 nlh_seq_last_seen; - GIOChannel *event_channel; - guint event_id; - - bool pruning[_DELAYED_ACTION_IDX_REFRESH_ALL_NUM]; - - bool sysctl_get_warned; - GHashTable *sysctl_get_prev_values; - - NMUdevClient *udev_client; - - struct { - /* which delayed actions are scheduled, as marked in @flags. - * Some types have additional arguments in the fields below. */ - DelayedActionType flags; - - /* counter that a refresh all action is in progress, separated - * by type. */ - int refresh_all_in_progress[_DELAYED_ACTION_IDX_REFRESH_ALL_NUM]; - - GPtrArray *list_master_connected; - GPtrArray *list_refresh_link; - GArray *list_wait_for_nl_response; - - int is_handling; - } delayed_action; -} NMLinuxPlatformPrivate; - -struct _NMLinuxPlatform { - NMPlatform parent; - NMLinuxPlatformPrivate _priv; -}; - -struct _NMLinuxPlatformClass { - NMPlatformClass parent; -}; - -G_DEFINE_TYPE (NMLinuxPlatform, nm_linux_platform, NM_TYPE_PLATFORM) - -#define NM_LINUX_PLATFORM_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMLinuxPlatform, NM_IS_LINUX_PLATFORM, NMPlatform) - -NMPlatform * -nm_linux_platform_new (gboolean log_with_ptr, gboolean netns_support) -{ - gboolean use_udev = FALSE; - - if ( nmp_netns_is_initial () - && access ("/sys", W_OK) == 0) - use_udev = TRUE; - - return g_object_new (NM_TYPE_LINUX_PLATFORM, - NM_PLATFORM_LOG_WITH_PTR, log_with_ptr, - NM_PLATFORM_USE_UDEV, use_udev, - NM_PLATFORM_NETNS_SUPPORT, netns_support, - NULL); -} - -void -nm_linux_platform_setup (void) -{ - nm_platform_setup (nm_linux_platform_new (FALSE, FALSE)); -} - /*****************************************************************************/ static struct nl_sock * @@ -6476,67 +6509,6 @@ link_release (NMPlatform *platform, int master, int slave) /*****************************************************************************/ -static gboolean -_wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link, NMPObject *obj) -{ - NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); - nm_auto_nlmsg struct nl_msg *msg = NULL; - struct _wireguard_device_buf buf = { - .obj = obj, - .peers = g_array_new (FALSE, FALSE, sizeof (NMWireGuardPeer)), - .allowedips = g_array_new (FALSE, FALSE, sizeof (NMWireGuardAllowedIP)), - }; - struct nl_cb cb = { - .valid_cb = _wireguard_get_device_cb, - .valid_arg = &buf, - }; - guint i, j; - int wireguard_family_id; - - wireguard_family_id = genl_ctrl_resolve (priv->genl, "wireguard"); - if (wireguard_family_id < 0) { - _LOGD ("wireguard: kernel support not available for wireguard link %s", link->name); - goto err; - } - - msg = nlmsg_alloc (); - - if (!genlmsg_put (msg, NL_AUTO_PORT, NL_AUTO_SEQ, wireguard_family_id, - 0, NLM_F_DUMP, WG_CMD_GET_DEVICE, 1)) - goto err; - - NLA_PUT_U32 (msg, WGDEVICE_A_IFINDEX, link->ifindex); - - if (nl_send_auto (priv->genl, msg) < 0) - goto err; - - if (nl_recvmsgs (priv->genl, &cb) < 0) - goto err; - - /* have each peer point to its own chunk of the allowedips buffer */ - for (i = 0, j = 0; i < buf.peers->len; i++) { - NMWireGuardPeer *p = &g_array_index (buf.peers, NMWireGuardPeer, i); - - p->allowedips = &g_array_index (buf.allowedips, NMWireGuardAllowedIP, j); - j += p->allowedips_len; - } - /* drop the wrapper (but also the buffer if no peer points to it) */ - g_array_free (buf.allowedips, buf.peers->len ? FALSE : TRUE); - - obj->_lnk_wireguard.peers_len = buf.peers->len; - obj->_lnk_wireguard.peers = (NMWireGuardPeer *) g_array_free (buf.peers, FALSE); - - return TRUE; - -err: -nla_put_failure: - g_array_free (buf.peers, TRUE); - g_array_free (buf.allowedips, TRUE); - return FALSE; -} - -/*****************************************************************************/ - static gboolean _infiniband_partition_action (NMPlatform *platform, InfinibandAction action, @@ -7643,6 +7615,14 @@ handle_udev_event (NMUdevClient *udev_client, /*****************************************************************************/ +void +nm_linux_platform_setup (void) +{ + nm_platform_setup (nm_linux_platform_new (FALSE, FALSE)); +} + +/*****************************************************************************/ + static void nm_linux_platform_init (NMLinuxPlatform *self) { @@ -7783,6 +7763,22 @@ constructed (GObject *_object) } } +NMPlatform * +nm_linux_platform_new (gboolean log_with_ptr, gboolean netns_support) +{ + gboolean use_udev = FALSE; + + if ( nmp_netns_is_initial () + && access ("/sys", W_OK) == 0) + use_udev = TRUE; + + return g_object_new (NM_TYPE_LINUX_PLATFORM, + NM_PLATFORM_LOG_WITH_PTR, log_with_ptr, + NM_PLATFORM_USE_UDEV, use_udev, + NM_PLATFORM_NETNS_SUPPORT, netns_support, + NULL); +} + static void dispose (GObject *object) { -- cgit v1.2.1 From f18fb7745abaae18f6625d4bf4e92f364d4b6749 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Sep 2018 08:58:26 +0200 Subject: platform: fix resusing ext-data from cache in _new_from_nl_link() --- src/platform/nm-linux-platform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 418186b360..c11c18883b 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -2366,6 +2366,7 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr if ( completed_from_cache && ( lnk_data_complete_from_cache + || need_ext_data || address_complete_from_cache || !af_inet6_token_valid || !af_inet6_addr_gen_mode_valid -- cgit v1.2.1 From cb23779e0acd1b7db3c9f5367ac03f3bc76a562c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Sep 2018 09:53:52 +0200 Subject: platform/trivial: rename local variables for nla_policy/nlattr We have such variables with similar purpose at various places. Name them all the same. --- src/platform/nm-linux-platform.c | 74 ++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index c11c18883b..0de0884497 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1924,7 +1924,7 @@ static gboolean _wireguard_update_from_allowedips_nla (struct _wireguard_device_buf *buf, struct nlattr *allowedip_attr) { - static const struct nla_policy allowedip_policy[WGALLOWEDIP_A_MAX + 1] = { + static const struct nla_policy policy[WGALLOWEDIP_A_MAX + 1] = { [WGALLOWEDIP_A_FAMILY] = { .type = NLA_U16 }, [WGALLOWEDIP_A_IPADDR] = { .minlen = sizeof (struct in_addr) }, [WGALLOWEDIP_A_CIDR_MASK] = { .type = NLA_U8 }, @@ -1936,7 +1936,7 @@ _wireguard_update_from_allowedips_nla (struct _wireguard_device_buf *buf, int addr_len; int nlerr; - nlerr = nla_parse_nested (tba, WGALLOWEDIP_A_MAX, allowedip_attr, allowedip_policy); + nlerr = nla_parse_nested (tba, WGALLOWEDIP_A_MAX, allowedip_attr, policy); if (nlerr < 0) return FALSE; @@ -1967,7 +1967,7 @@ static gboolean _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, struct nlattr *peer_attr) { - static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = { + static const struct nla_policy policy[WGPEER_A_MAX + 1] = { [WGPEER_A_PUBLIC_KEY] = { .minlen = NM_WG_PUBLIC_KEY_LEN }, [WGPEER_A_PRESHARED_KEY] = { }, [WGPEER_A_FLAGS] = { .type = NLA_U32 }, @@ -1978,55 +1978,55 @@ _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, [WGPEER_A_TX_BYTES] = { .type = NLA_U64 }, [WGPEER_A_ALLOWEDIPS] = { .type = NLA_NESTED }, }; - struct nlattr *tbp[WGPEER_A_MAX + 1]; + struct nlattr *tb[WGPEER_A_MAX + 1]; NMWireGuardPeer *const last = buf->peers->len ? &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1) : NULL; NMWireGuardPeer *peer; NMWireGuardPeer new_peer = { 0 }; - if (nla_parse_nested (tbp, WGPEER_A_MAX, peer_attr, peer_policy) < 0) + if (nla_parse_nested (tb, WGPEER_A_MAX, peer_attr, policy) < 0) return FALSE; - if (!tbp[WGPEER_A_PUBLIC_KEY]) + if (!tb[WGPEER_A_PUBLIC_KEY]) return FALSE; /* a peer with the same public key as last peer is just a continuation for extra AllowedIPs */ if ( last - && !memcmp (nla_data (tbp[WGPEER_A_PUBLIC_KEY]), last->public_key, sizeof (last->public_key))) + && !memcmp (nla_data (tb[WGPEER_A_PUBLIC_KEY]), last->public_key, sizeof (last->public_key))) peer = last; else { /* otherwise, start a new peer */ g_array_append_val (buf->peers, new_peer); peer = &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1); - nla_memcpy (&peer->public_key, tbp[WGPEER_A_PUBLIC_KEY], sizeof (peer->public_key)); + nla_memcpy (&peer->public_key, tb[WGPEER_A_PUBLIC_KEY], sizeof (peer->public_key)); - if (tbp[WGPEER_A_PRESHARED_KEY]) - nla_memcpy (&peer->preshared_key, tbp[WGPEER_A_PRESHARED_KEY], sizeof (peer->preshared_key)); - if (tbp[WGPEER_A_ENDPOINT]) { - struct sockaddr *addr = nla_data (tbp[WGPEER_A_ENDPOINT]); + if (tb[WGPEER_A_PRESHARED_KEY]) + nla_memcpy (&peer->preshared_key, tb[WGPEER_A_PRESHARED_KEY], sizeof (peer->preshared_key)); + if (tb[WGPEER_A_ENDPOINT]) { + struct sockaddr *addr = nla_data (tb[WGPEER_A_ENDPOINT]); if (addr->sa_family == AF_INET) - nla_memcpy (&peer->endpoint.addr4, tbp[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr4)); + nla_memcpy (&peer->endpoint.addr4, tb[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr4)); else if (addr->sa_family == AF_INET6) - nla_memcpy (&peer->endpoint.addr6, tbp[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr6)); + nla_memcpy (&peer->endpoint.addr6, tb[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr6)); } - if (tbp[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]) - peer->persistent_keepalive_interval = nla_get_u64 (tbp[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]); - if (tbp[WGPEER_A_LAST_HANDSHAKE_TIME]) - nla_memcpy (&peer->last_handshake_time, tbp[WGPEER_A_LAST_HANDSHAKE_TIME], sizeof (peer->last_handshake_time)); - if (tbp[WGPEER_A_RX_BYTES]) - peer->rx_bytes = nla_get_u64 (tbp[WGPEER_A_RX_BYTES]); - if (tbp[WGPEER_A_TX_BYTES]) - peer->tx_bytes = nla_get_u64 (tbp[WGPEER_A_TX_BYTES]); + if (tb[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]) + peer->persistent_keepalive_interval = nla_get_u64 (tb[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]); + if (tb[WGPEER_A_LAST_HANDSHAKE_TIME]) + nla_memcpy (&peer->last_handshake_time, tb[WGPEER_A_LAST_HANDSHAKE_TIME], sizeof (peer->last_handshake_time)); + if (tb[WGPEER_A_RX_BYTES]) + peer->rx_bytes = nla_get_u64 (tb[WGPEER_A_RX_BYTES]); + if (tb[WGPEER_A_TX_BYTES]) + peer->tx_bytes = nla_get_u64 (tb[WGPEER_A_TX_BYTES]); peer->allowedips = NULL; peer->allowedips_len = 0; } - if (tbp[WGPEER_A_ALLOWEDIPS]) { + if (tb[WGPEER_A_ALLOWEDIPS]) { struct nlattr *attr; int rem; - nla_for_each_nested (attr, tbp[WGPEER_A_ALLOWEDIPS], rem) { + nla_for_each_nested (attr, tb[WGPEER_A_ALLOWEDIPS], rem) { if (!_wireguard_update_from_allowedips_nla (buf, attr)) return FALSE; } @@ -2038,7 +2038,7 @@ _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, static int _wireguard_get_device_cb (struct nl_msg *msg, void *arg) { - static const struct nla_policy device_policy[WGDEVICE_A_MAX + 1] = { + static const struct nla_policy policy[WGDEVICE_A_MAX + 1] = { [WGDEVICE_A_IFINDEX] = { .type = NLA_U32 }, [WGDEVICE_A_IFNAME] = { .type = NLA_NUL_STRING, .maxlen = IFNAMSIZ }, [WGDEVICE_A_PRIVATE_KEY] = { }, @@ -2049,29 +2049,29 @@ _wireguard_get_device_cb (struct nl_msg *msg, void *arg) [WGDEVICE_A_PEERS] = { .type = NLA_NESTED }, }; struct _wireguard_device_buf *buf = arg; - struct nlattr *tbd[WGDEVICE_A_MAX + 1]; + struct nlattr *tb[WGDEVICE_A_MAX + 1]; NMPlatformLnkWireGuard *props = &buf->obj->lnk_wireguard; struct nlmsghdr *nlh = nlmsg_hdr (msg); int nlerr; - nlerr = genlmsg_parse (nlh, 0, tbd, WGDEVICE_A_MAX, device_policy); + nlerr = genlmsg_parse (nlh, 0, tb, WGDEVICE_A_MAX, policy); if (nlerr < 0) return NL_SKIP; - if (tbd[WGDEVICE_A_PRIVATE_KEY]) - nla_memcpy (props->private_key, tbd[WGDEVICE_A_PRIVATE_KEY], sizeof (props->private_key)); - if (tbd[WGDEVICE_A_PUBLIC_KEY]) - nla_memcpy (props->public_key, tbd[WGDEVICE_A_PUBLIC_KEY], sizeof (props->public_key)); - if (tbd[WGDEVICE_A_LISTEN_PORT]) - props->listen_port = nla_get_u16 (tbd[WGDEVICE_A_LISTEN_PORT]); - if (tbd[WGDEVICE_A_FWMARK]) - props->fwmark = nla_get_u32 (tbd[WGDEVICE_A_FWMARK]); + if (tb[WGDEVICE_A_PRIVATE_KEY]) + nla_memcpy (props->private_key, tb[WGDEVICE_A_PRIVATE_KEY], sizeof (props->private_key)); + if (tb[WGDEVICE_A_PUBLIC_KEY]) + nla_memcpy (props->public_key, tb[WGDEVICE_A_PUBLIC_KEY], sizeof (props->public_key)); + if (tb[WGDEVICE_A_LISTEN_PORT]) + props->listen_port = nla_get_u16 (tb[WGDEVICE_A_LISTEN_PORT]); + if (tb[WGDEVICE_A_FWMARK]) + props->fwmark = nla_get_u32 (tb[WGDEVICE_A_FWMARK]); - if (tbd[WGDEVICE_A_PEERS]) { + if (tb[WGDEVICE_A_PEERS]) { struct nlattr *attr; int rem; - nla_for_each_nested (attr, tbd[WGDEVICE_A_PEERS], rem) { + nla_for_each_nested (attr, tb[WGDEVICE_A_PEERS], rem) { if (!_wireguard_update_from_peers_nla (buf, attr)) return NL_SKIP; } -- cgit v1.2.1 From 62d14e188489fab4ea8b20527925b47dc2c15f40 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Sep 2018 09:54:07 +0200 Subject: platform/wireguard: rework parsing wireguard links in platform - previously, parsing wireguard genl data resulted in memory corruption: - _wireguard_update_from_allowedips_nla() takes pointers to allowedip = &g_array_index (buf->allowedips, NMWireGuardAllowedIP, buf->allowedips->len - 1); but resizing the GArray will invalidate this pointer. This happens when there are multiple allowed-ips to parse. - there was some confusion who owned the allowedips pointers. _wireguard_peers_cpy() and _vt_cmd_obj_dispose_lnk_wireguard() assumed each peer owned their own chunk, but _wireguard_get_link_properties() would not duplicate the memory properly. - rework memory handling for allowed_ips. Now, the NMPObjectLnkWireGuard keeps a pointer _allowed_ips_buf. This buffer contains the instances for all peers. The parsing of the netlink message is the complicated part, because we don't know upfront how many peers/allowed-ips we receive. During construction, the tracking of peers/allowed-ips is complicated, via a CList/GArray. At the end of that, we prettify the data representation and put everything into two buffers. That is more efficient and simpler for user afterwards. This moves complexity to the way how the object is created, vs. how it is used later. - ensure that we nm_explicit_bzero() private-key and preshared-key. However, that only works to a certain point, because our netlink library does not ensure that no data is leaked. - don't use a "struct sockaddr" union for the peer's endpoint. Instead, use a combintation of endpoint_family, endpoint_port, and endpoint_addr. - a lot of refactoring. --- libnm-core/nm-core-types-internal.h | 25 --- shared/nm-utils/nm-hash-utils.h | 5 + src/platform/nm-linux-platform.c | 434 +++++++++++++++++++++++++----------- src/platform/nm-platform.c | 105 ++++----- src/platform/nm-platform.h | 12 +- src/platform/nmp-object.c | 301 +++++++++++++------------ src/platform/nmp-object.h | 40 +++- 7 files changed, 555 insertions(+), 367 deletions(-) diff --git a/libnm-core/nm-core-types-internal.h b/libnm-core/nm-core-types-internal.h index 7ab0f59455..4d43aaf45d 100644 --- a/libnm-core/nm-core-types-internal.h +++ b/libnm-core/nm-core-types-internal.h @@ -31,31 +31,6 @@ typedef struct { guint32 to; } NMVlanQosMapping; -typedef struct { - NMIPAddr ip; - guint8 family; - guint8 mask; -} NMWireGuardAllowedIP; - -#define NM_WG_PUBLIC_KEY_LEN 32 -#define NM_WG_SYMMETRIC_KEY_LEN 32 - -typedef struct { - guint8 public_key[NM_WG_PUBLIC_KEY_LEN]; - guint8 preshared_key[NM_WG_SYMMETRIC_KEY_LEN]; - union { - struct sockaddr addr; - struct sockaddr_in addr4; - struct sockaddr_in6 addr6; - } endpoint; - guint16 persistent_keepalive_interval; - struct timespec last_handshake_time; - guint64 rx_bytes, tx_bytes; - - gsize allowedips_len; - NMWireGuardAllowedIP *allowedips; -} NMWireGuardPeer; - #define _NM_IP_TUNNEL_FLAG_ALL_IP6TNL \ ( NM_IP_TUNNEL_FLAG_IP6_IGN_ENCAP_LIMIT \ | NM_IP_TUNNEL_FLAG_IP6_USE_ORIG_TCLASS \ diff --git a/shared/nm-utils/nm-hash-utils.h b/shared/nm-utils/nm-hash-utils.h index 7d9620b96c..b797fb75af 100644 --- a/shared/nm-utils/nm-hash-utils.h +++ b/shared/nm-utils/nm-hash-utils.h @@ -57,6 +57,11 @@ nm_hash_update (NMHashState *state, const void *ptr, gsize n) nm_assert (ptr); nm_assert (n > 0); + /* Note: the data passed in here might be sensitive data (secrets), + * that we should nm_explicty_zero() afterwards. However, since + * we are using siphash24 with a random key, that is not really + * necessary. Something to keep in mind, if we ever move away from + * this hash implementation. */ c_siphash_append (&state->_state, ptr, n); } diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 0de0884497..e73d5d8c3d 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -44,6 +44,7 @@ #include "nm-core-internal.h" #include "nm-setting-vlan.h" +#include "nm-utils/nm-secret-utils.h" #include "nm-netlink.h" #include "nm-core-utils.h" #include "nmp-object.h" @@ -1912,63 +1913,60 @@ _parse_lnk_vxlan (const char *kind, struct nlattr *info_data) /*****************************************************************************/ -/* Context to build a NMPObjectLnkWireGuard instance. - * GArray wrappers are discarded after processing all netlink messages. */ -struct _wireguard_device_buf { - NMPObject *obj; - GArray *peers; - GArray *allowedips; -}; - static gboolean -_wireguard_update_from_allowedips_nla (struct _wireguard_device_buf *buf, - struct nlattr *allowedip_attr) +_wireguard_update_from_allowed_ips_nla (NMPWireGuardAllowedIP *allowed_ip, + struct nlattr *nlattr) { static const struct nla_policy policy[WGALLOWEDIP_A_MAX + 1] = { [WGALLOWEDIP_A_FAMILY] = { .type = NLA_U16 }, [WGALLOWEDIP_A_IPADDR] = { .minlen = sizeof (struct in_addr) }, [WGALLOWEDIP_A_CIDR_MASK] = { .type = NLA_U8 }, }; - struct nlattr *tba[WGALLOWEDIP_A_MAX + 1]; - NMWireGuardPeer *peer = &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1); - NMWireGuardAllowedIP *allowedip; - NMWireGuardAllowedIP new_allowedip = {0}; + struct nlattr *tb[WGALLOWEDIP_A_MAX + 1]; + int family; int addr_len; - int nlerr; - nlerr = nla_parse_nested (tba, WGALLOWEDIP_A_MAX, allowedip_attr, policy); - if (nlerr < 0) + if (nla_parse_nested (tb, WGALLOWEDIP_A_MAX, nlattr, policy) < 0) return FALSE; - g_array_append_val (buf->allowedips, new_allowedip); - allowedip = &g_array_index (buf->allowedips, NMWireGuardAllowedIP, buf->allowedips->len - 1); - peer->allowedips_len++; - - if (tba[WGALLOWEDIP_A_FAMILY]) - allowedip->family = nla_get_u16 (tba[WGALLOWEDIP_A_FAMILY]); + if (!tb[WGALLOWEDIP_A_FAMILY]) + return FALSE; - if (allowedip->family == AF_INET) + family = nla_get_u16 (tb[WGALLOWEDIP_A_FAMILY]); + if (family == AF_INET) addr_len = sizeof (in_addr_t); - else if (allowedip->family == AF_INET6) + else if (family == AF_INET6) addr_len = sizeof (struct in6_addr); else return FALSE; - _check_addr_or_return_val (tba, WGALLOWEDIP_A_IPADDR, addr_len, FALSE); - if (tba[WGALLOWEDIP_A_IPADDR]) - nla_memcpy (&allowedip->ip, tba[WGALLOWEDIP_A_IPADDR], addr_len); - if (tba[WGALLOWEDIP_A_CIDR_MASK]) - allowedip->mask = nla_get_u8 (tba[WGALLOWEDIP_A_CIDR_MASK]); + _check_addr_or_return_val (tb, WGALLOWEDIP_A_IPADDR, addr_len, FALSE); + + memset (allowed_ip, 0, sizeof (NMPWireGuardAllowedIP)); + + allowed_ip->family = family; + nm_assert ((int) allowed_ip->family == family); + + if (tb[WGALLOWEDIP_A_IPADDR]) + nla_memcpy (&allowed_ip->addr, tb[WGALLOWEDIP_A_IPADDR], addr_len); + if (tb[WGALLOWEDIP_A_CIDR_MASK]) + allowed_ip->mask = nla_get_u8 (tb[WGALLOWEDIP_A_CIDR_MASK]); return TRUE; } +typedef struct { + CList lst; + NMPWireGuardPeer data; +} WireGuardPeerConstruct; + static gboolean -_wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, +_wireguard_update_from_peers_nla (CList *peers, + GArray **p_allowed_ips, struct nlattr *peer_attr) { static const struct nla_policy policy[WGPEER_A_MAX + 1] = { - [WGPEER_A_PUBLIC_KEY] = { .minlen = NM_WG_PUBLIC_KEY_LEN }, + [WGPEER_A_PUBLIC_KEY] = { .minlen = NMP_WIREGUARD_PUBLIC_KEY_LEN }, [WGPEER_A_PRESHARED_KEY] = { }, [WGPEER_A_FLAGS] = { .type = NLA_U32 }, [WGPEER_A_ENDPOINT] = { }, @@ -1978,10 +1976,8 @@ _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, [WGPEER_A_TX_BYTES] = { .type = NLA_U64 }, [WGPEER_A_ALLOWEDIPS] = { .type = NLA_NESTED }, }; + WireGuardPeerConstruct *peer_c; struct nlattr *tb[WGPEER_A_MAX + 1]; - NMWireGuardPeer *const last = buf->peers->len ? &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1) : NULL; - NMWireGuardPeer *peer; - NMWireGuardPeer new_peer = { 0 }; if (nla_parse_nested (tb, WGPEER_A_MAX, peer_attr, policy) < 0) return FALSE; @@ -1990,51 +1986,98 @@ _wireguard_update_from_peers_nla (struct _wireguard_device_buf *buf, return FALSE; /* a peer with the same public key as last peer is just a continuation for extra AllowedIPs */ - if ( last - && !memcmp (nla_data (tb[WGPEER_A_PUBLIC_KEY]), last->public_key, sizeof (last->public_key))) - peer = last; + peer_c = c_list_last_entry (peers, WireGuardPeerConstruct, lst); + if ( peer_c + && !memcmp (nla_data (tb[WGPEER_A_PUBLIC_KEY]), peer_c->data.public_key, NMP_WIREGUARD_PUBLIC_KEY_LEN)) { + G_STATIC_ASSERT_EXPR (NMP_WIREGUARD_PUBLIC_KEY_LEN == sizeof (peer_c->data.public_key)); + /* this message is a continuation of the previous peer. + * Only parse WGPEER_A_ALLOWEDIPS below. */ + } else { /* otherwise, start a new peer */ - g_array_append_val (buf->peers, new_peer); - peer = &g_array_index (buf->peers, NMWireGuardPeer, buf->peers->len - 1); + peer_c = g_slice_new0 (WireGuardPeerConstruct); + c_list_link_tail (peers, &peer_c->lst); - nla_memcpy (&peer->public_key, tb[WGPEER_A_PUBLIC_KEY], sizeof (peer->public_key)); + nla_memcpy (&peer_c->data.public_key, tb[WGPEER_A_PUBLIC_KEY], sizeof (peer_c->data.public_key)); - if (tb[WGPEER_A_PRESHARED_KEY]) - nla_memcpy (&peer->preshared_key, tb[WGPEER_A_PRESHARED_KEY], sizeof (peer->preshared_key)); + if (tb[WGPEER_A_PRESHARED_KEY]) { + nla_memcpy (&peer_c->data.preshared_key, tb[WGPEER_A_PRESHARED_KEY], sizeof (peer_c->data.preshared_key)); + /* FIXME(netlink-bzero-secret) */ + nm_explicit_bzero (nla_data (tb[WGPEER_A_PRESHARED_KEY]), + nla_len (tb[WGPEER_A_PRESHARED_KEY])); + } if (tb[WGPEER_A_ENDPOINT]) { - struct sockaddr *addr = nla_data (tb[WGPEER_A_ENDPOINT]); - if (addr->sa_family == AF_INET) - nla_memcpy (&peer->endpoint.addr4, tb[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr4)); - else if (addr->sa_family == AF_INET6) - nla_memcpy (&peer->endpoint.addr6, tb[WGPEER_A_ENDPOINT], sizeof (peer->endpoint.addr6)); + const struct sockaddr *addr = nla_data (tb[WGPEER_A_ENDPOINT]); + unsigned short family; + + G_STATIC_ASSERT (sizeof (addr->sa_family) == sizeof (family)); + memcpy (&family, &addr->sa_family, sizeof (addr->sa_family)); + + if ( family == AF_INET + && nla_len (tb[WGPEER_A_ENDPOINT]) == sizeof (struct sockaddr_in)) { + const struct sockaddr_in *addr4 = (const struct sockaddr_in *) addr; + + peer_c->data.endpoint_family = AF_INET; + peer_c->data.endpoint_port = unaligned_read_be16 (&addr4->sin_port); + peer_c->data.endpoint_addr.addr4 = unaligned_read_ne32 (&addr4->sin_addr.s_addr); + memcpy (&peer_c->data.endpoint_addr.addr4, &addr4->sin_addr.s_addr, 4); + } else if ( family == AF_INET6 + && nla_len (tb[WGPEER_A_ENDPOINT]) == sizeof (struct sockaddr_in6)) { + const struct sockaddr_in6 *addr6 = (const struct sockaddr_in6 *) addr; + + peer_c->data.endpoint_family = AF_INET6; + peer_c->data.endpoint_port = unaligned_read_be16 (&addr6->sin6_port); + memcpy (&peer_c->data.endpoint_addr.addr6, &addr6->sin6_addr, 16); + } } if (tb[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]) - peer->persistent_keepalive_interval = nla_get_u64 (tb[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]); + peer_c->data.persistent_keepalive_interval = nla_get_u64 (tb[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]); if (tb[WGPEER_A_LAST_HANDSHAKE_TIME]) - nla_memcpy (&peer->last_handshake_time, tb[WGPEER_A_LAST_HANDSHAKE_TIME], sizeof (peer->last_handshake_time)); + nla_memcpy (&peer_c->data.last_handshake_time, tb[WGPEER_A_LAST_HANDSHAKE_TIME], sizeof (peer_c->data.last_handshake_time)); if (tb[WGPEER_A_RX_BYTES]) - peer->rx_bytes = nla_get_u64 (tb[WGPEER_A_RX_BYTES]); + peer_c->data.rx_bytes = nla_get_u64 (tb[WGPEER_A_RX_BYTES]); if (tb[WGPEER_A_TX_BYTES]) - peer->tx_bytes = nla_get_u64 (tb[WGPEER_A_TX_BYTES]); - - peer->allowedips = NULL; - peer->allowedips_len = 0; + peer_c->data.tx_bytes = nla_get_u64 (tb[WGPEER_A_TX_BYTES]); } if (tb[WGPEER_A_ALLOWEDIPS]) { struct nlattr *attr; int rem; + GArray *allowed_ips = *p_allowed_ips; nla_for_each_nested (attr, tb[WGPEER_A_ALLOWEDIPS], rem) { - if (!_wireguard_update_from_allowedips_nla (buf, attr)) - return FALSE; + if (!allowed_ips) { + allowed_ips = g_array_new (FALSE, FALSE, sizeof (NMPWireGuardAllowedIP)); + *p_allowed_ips = allowed_ips; + g_array_set_size (allowed_ips, 1); + } else + g_array_set_size (allowed_ips, allowed_ips->len + 1); + + if (!_wireguard_update_from_allowed_ips_nla (&g_array_index (allowed_ips, + NMPWireGuardAllowedIP, + allowed_ips->len - 1), + attr)) { + /* we ignore the error of parsing one allowed-ip. */ + g_array_set_size (allowed_ips, allowed_ips->len - 1); + continue; + } + + if (!peer_c->data._construct_idx_end) + peer_c->data._construct_idx_start = allowed_ips->len - 1; + peer_c->data._construct_idx_end = allowed_ips->len; } } return TRUE; } +typedef struct { + const int ifindex; + NMPObject *obj; + CList peers; + GArray *allowed_ips; +} WireGuardParseData; + static int _wireguard_get_device_cb (struct nl_msg *msg, void *arg) { @@ -2048,95 +2091,196 @@ _wireguard_get_device_cb (struct nl_msg *msg, void *arg) [WGDEVICE_A_FWMARK] = { .type = NLA_U32 }, [WGDEVICE_A_PEERS] = { .type = NLA_NESTED }, }; - struct _wireguard_device_buf *buf = arg; + WireGuardParseData *parse_data = arg; struct nlattr *tb[WGDEVICE_A_MAX + 1]; - NMPlatformLnkWireGuard *props = &buf->obj->lnk_wireguard; - struct nlmsghdr *nlh = nlmsg_hdr (msg); int nlerr; - nlerr = genlmsg_parse (nlh, 0, tb, WGDEVICE_A_MAX, policy); + nlerr = genlmsg_parse (nlmsg_hdr (msg), 0, tb, WGDEVICE_A_MAX, policy); if (nlerr < 0) return NL_SKIP; - if (tb[WGDEVICE_A_PRIVATE_KEY]) - nla_memcpy (props->private_key, tb[WGDEVICE_A_PRIVATE_KEY], sizeof (props->private_key)); - if (tb[WGDEVICE_A_PUBLIC_KEY]) - nla_memcpy (props->public_key, tb[WGDEVICE_A_PUBLIC_KEY], sizeof (props->public_key)); - if (tb[WGDEVICE_A_LISTEN_PORT]) - props->listen_port = nla_get_u16 (tb[WGDEVICE_A_LISTEN_PORT]); - if (tb[WGDEVICE_A_FWMARK]) - props->fwmark = nla_get_u32 (tb[WGDEVICE_A_FWMARK]); + if (tb[WGDEVICE_A_IFINDEX]) { + int ifindex; + + ifindex = (int) nla_get_u32 (tb[WGDEVICE_A_IFINDEX]); + if ( ifindex <= 0 + || parse_data->ifindex != ifindex) + return NL_SKIP; + } else { + if (!parse_data->obj) + return NL_SKIP; + } + + if (parse_data->obj) { + /* we already have an object instance. This means the netlink message + * is a continuation, only providing more WGDEVICE_A_PEERS data below. */ + } else { + NMPObject *obj; + NMPlatformLnkWireGuard *props; + + obj = nmp_object_new (NMP_OBJECT_TYPE_LNK_WIREGUARD, NULL); + props = &obj->lnk_wireguard; + + if (tb[WGDEVICE_A_PRIVATE_KEY]) { + nla_memcpy (props->private_key, tb[WGDEVICE_A_PRIVATE_KEY], sizeof (props->private_key)); + /* FIXME(netlink-bzero-secret): extend netlink library to wipe memory. For now, + * just hack it here (yes, this does not cover all places where the + * private key was copied). */ + nm_explicit_bzero (nla_data (tb[WGDEVICE_A_PRIVATE_KEY]), + nla_len (tb[WGDEVICE_A_PRIVATE_KEY])); + } + if (tb[WGDEVICE_A_PUBLIC_KEY]) + nla_memcpy (props->public_key, tb[WGDEVICE_A_PUBLIC_KEY], sizeof (props->public_key)); + if (tb[WGDEVICE_A_LISTEN_PORT]) + props->listen_port = nla_get_u16 (tb[WGDEVICE_A_LISTEN_PORT]); + if (tb[WGDEVICE_A_FWMARK]) + props->fwmark = nla_get_u32 (tb[WGDEVICE_A_FWMARK]); + + parse_data->obj = obj; + } if (tb[WGDEVICE_A_PEERS]) { struct nlattr *attr; int rem; nla_for_each_nested (attr, tb[WGDEVICE_A_PEERS], rem) { - if (!_wireguard_update_from_peers_nla (buf, attr)) - return NL_SKIP; + if (!_wireguard_update_from_peers_nla (&parse_data->peers, &parse_data->allowed_ips, attr)) { + /* we ignore the error of parsing one peer. + * _wireguard_update_from_peers_nla() leaves the @peers array in the + * desired state. */ + } } } return NL_OK; } -static gboolean -_wireguard_get_link_properties (NMPlatform *platform, const NMPlatformLink *link, NMPObject *obj) +static const NMPObject * +_wireguard_read_info (NMPlatform *platform /* used only as logging context */, + struct nl_sock *genl, + int wireguard_family_id, + int ifindex) { - NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); nm_auto_nlmsg struct nl_msg *msg = NULL; - struct _wireguard_device_buf buf = { - .obj = obj, - .peers = g_array_new (FALSE, FALSE, sizeof (NMWireGuardPeer)), - .allowedips = g_array_new (FALSE, FALSE, sizeof (NMWireGuardAllowedIP)), - }; - struct nl_cb cb = { - .valid_cb = _wireguard_get_device_cb, - .valid_arg = &buf, + NMPObject *obj = NULL; + WireGuardPeerConstruct *peer_c; + WireGuardPeerConstruct *peer_c_safe; + gs_unref_array GArray *allowed_ips = NULL; + WireGuardParseData parse_data = { + .ifindex = ifindex, }; - guint i, j; - int wireguard_family_id; + guint i; - wireguard_family_id = genl_ctrl_resolve (priv->genl, "wireguard"); - if (wireguard_family_id < 0) { - _LOGD ("wireguard: kernel support not available for wireguard link %s", link->name); - goto err; - } + nm_assert (genl); + nm_assert (wireguard_family_id >= 0); + nm_assert (ifindex > 0); msg = nlmsg_alloc (); - if (!genlmsg_put (msg, NL_AUTO_PORT, NL_AUTO_SEQ, wireguard_family_id, - 0, NLM_F_DUMP, WG_CMD_GET_DEVICE, 1)) - goto err; + if (!genlmsg_put (msg, + NL_AUTO_PORT, + NL_AUTO_SEQ, + wireguard_family_id, + 0, + NLM_F_DUMP, + WG_CMD_GET_DEVICE, + 1)) + return NULL; + + NLA_PUT_U32 (msg, WGDEVICE_A_IFINDEX, (guint32) ifindex); - NLA_PUT_U32 (msg, WGDEVICE_A_IFINDEX, link->ifindex); + if (nl_send_auto (genl, msg) < 0) + return NULL; - if (nl_send_auto (priv->genl, msg) < 0) - goto err; + c_list_init (&parse_data.peers); - if (nl_recvmsgs (priv->genl, &cb) < 0) - goto err; + /* we ignore errors, and return whatever we could successfully + * parse. */ + nl_recvmsgs (genl, + &((const struct nl_cb) { + .valid_cb = _wireguard_get_device_cb, + .valid_arg = (gpointer) &parse_data, + })); - /* have each peer point to its own chunk of the allowedips buffer */ - for (i = 0, j = 0; i < buf.peers->len; i++) { - NMWireGuardPeer *p = &g_array_index (buf.peers, NMWireGuardPeer, i); + /* unpack: transfer ownership */ + obj = parse_data.obj; + allowed_ips = parse_data.allowed_ips; - p->allowedips = &g_array_index (buf.allowedips, NMWireGuardAllowedIP, j); - j += p->allowedips_len; + if (!obj) { + while ((peer_c = c_list_first_entry (&parse_data.peers, WireGuardPeerConstruct, lst))) { + c_list_unlink_stale (&peer_c->lst); + nm_explicit_bzero (&peer_c->data.preshared_key, sizeof (peer_c->data.preshared_key)); + g_slice_free (WireGuardPeerConstruct, peer_c); + } + return NULL; } - /* drop the wrapper (but also the buffer if no peer points to it) */ - g_array_free (buf.allowedips, buf.peers->len ? FALSE : TRUE); - obj->_lnk_wireguard.peers_len = buf.peers->len; - obj->_lnk_wireguard.peers = (NMWireGuardPeer *) g_array_free (buf.peers, FALSE); + /* we receive peers/allowed-ips possibly in separate netlink messages. Hence, while + * parsing the dump, we don't know upfront how many peers/allowed-ips we will receive. + * + * We solve that, by collecting all peers with a CList. It's done this way, + * because a GArray would require growing the array, but we want to bzero() + * the preshared-key of each peer while reallocating. The CList apprach avoids + * that. + * + * For allowed-ips, we instead track one GArray, which are all appended + * there. The realloc/resize of the GArray is fine there. However, + * while we build the GArray, we don't yet have the final pointers. + * Hence, while constructing, we track the indexes with peer->_construct_idx_* + * fields. These indexes must be convered to actual pointers blow. + * + * This is all done during parsing. In the final NMPObjectLnkWireGuard we + * don't want the CList anymore and repackage the NMPObject tightly. The + * reason is, that NMPObject instances are immutable and long-living. Spend + * a bit effort below during construction to obtain a most suitable representation + * in this regard. */ + obj->_lnk_wireguard.peers_len = c_list_length (&parse_data.peers); + obj->_lnk_wireguard.peers = obj->_lnk_wireguard.peers_len > 0 + ? g_new (NMPWireGuardPeer, obj->_lnk_wireguard.peers_len) + : NULL; + + /* duplicate allowed_ips instead of using the pointer. The GArray possibly has more + * space allocated then we need, and we want to get rid of this excess buffer. + * Note that NMPObject instance is possibly put into the cache and long-living. */ + obj->_lnk_wireguard._allowed_ips_buf_len = allowed_ips ? allowed_ips->len : 0u; + obj->_lnk_wireguard._allowed_ips_buf = obj->_lnk_wireguard._allowed_ips_buf_len > 0 + ? (NMPWireGuardAllowedIP *) nm_memdup (allowed_ips->data, + sizeof (NMPWireGuardAllowedIP) * allowed_ips->len) + : NULL; + + i = 0; + c_list_for_each_entry_safe (peer_c, peer_c_safe, &parse_data.peers, lst) { + NMPWireGuardPeer *peer = (NMPWireGuardPeer *) &obj->_lnk_wireguard.peers[i++]; + + *peer = peer_c->data; + + c_list_unlink_stale (&peer_c->lst); + nm_explicit_bzero (&peer_c->data.preshared_key, sizeof (peer_c->data.preshared_key)); + g_slice_free (WireGuardPeerConstruct, peer_c); + + if (peer->_construct_idx_end != 0) { + guint len; + + nm_assert (obj->_lnk_wireguard._allowed_ips_buf); + nm_assert (peer->_construct_idx_end > peer->_construct_idx_start); + nm_assert (peer->_construct_idx_start < obj->_lnk_wireguard._allowed_ips_buf_len); + nm_assert (peer->_construct_idx_end <= obj->_lnk_wireguard._allowed_ips_buf_len); + + len = peer->_construct_idx_end - peer->_construct_idx_start; + peer->allowed_ips = &obj->_lnk_wireguard._allowed_ips_buf[peer->_construct_idx_start]; + peer->allowed_ips_len = len; + } else { + nm_assert (!peer->_construct_idx_start); + nm_assert (!peer->_construct_idx_end); + peer->allowed_ips = NULL; + peer->allowed_ips_len = 0; + } + } - return TRUE; + return obj; -err: nla_put_failure: - g_array_free (buf.peers, TRUE); - g_array_free (buf.allowedips, TRUE); - return FALSE; + g_return_val_if_reached (NULL); } /*****************************************************************************/ @@ -2188,7 +2332,7 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr gboolean completed_from_cache_val = FALSE; gboolean *completed_from_cache = cache ? &completed_from_cache_val : NULL; const NMPObject *link_cached = NULL; - NMPObject *lnk_data = NULL; + const NMPObject *lnk_data = NULL; gboolean address_complete_from_cache = TRUE; gboolean lnk_data_complete_from_cache = TRUE; gboolean need_ext_data = FALSE; @@ -2197,10 +2341,12 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr if (!nlmsg_valid_hdr (nlh, sizeof (*ifi))) return NULL; - ifi = nlmsg_data(nlh); + ifi = nlmsg_data (nlh); if (ifi->ifi_family != AF_UNSPEC) return NULL; + if (ifi->ifi_index <= 0) + return NULL; obj = nmp_object_new_link (ifi->ifi_index); @@ -2358,6 +2504,7 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr lnk_data_complete_from_cache = FALSE; break; case NM_LINK_TYPE_WIREGUARD: + lnk_data_complete_from_cache = TRUE; break; default: lnk_data_complete_from_cache = FALSE; @@ -2386,7 +2533,7 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr * Also, sometimes the info-data is missing for updates. In this case * we want to keep the previously received lnk_data. */ nmp_object_unref (lnk_data); - lnk_data = (NMPObject *) nmp_object_ref (link_cached->_link.netlink.lnk); + lnk_data = nmp_object_ref (link_cached->_link.netlink.lnk); } if ( need_ext_data @@ -2411,29 +2558,10 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr } } - if (obj->link.type == NM_LINK_TYPE_WIREGUARD) { - nm_auto_nmpobj NMPObject *lnk_data_now = NULL; - - /* The WireGuard kernel module does not yet send link update - * notifications, so we don't actually update the cache. For - * now, always refetch link data here. */ - lnk_data_now = nmp_object_new (NMP_OBJECT_TYPE_LNK_WIREGUARD, NULL); - if (!_wireguard_get_link_properties (platform, &obj->link, lnk_data_now)) { - _LOGD ("wireguard: %d %s: failed to get properties", - obj->link.ifindex, - obj->link.name ?: ""); - } - - if (lnk_data && nmp_object_cmp (lnk_data, lnk_data_now)) - nmp_object_unref (g_steal_pointer (&lnk_data)); - - if (!lnk_data) - lnk_data = (NMPObject *) nmp_object_ref (lnk_data_now); - } - obj->_link.netlink.lnk = lnk_data; - if (need_ext_data && obj->_link.ext_data == NULL) { + if ( need_ext_data + && obj->_link.ext_data == NULL) { switch (obj->link.type) { case NM_LINK_TYPE_WIFI: obj->_link.ext_data = (GObject *) nm_wifi_utils_new (ifi->ifi_index, @@ -2459,6 +2587,42 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr } } + if (obj->link.type == NM_LINK_TYPE_WIREGUARD) { + const NMPObject *lnk_data_new = NULL; + struct nl_sock *genl = NM_LINUX_PLATFORM_GET_PRIVATE (platform)->genl; + + /* The WireGuard kernel module does not yet send link update + * notifications, so we don't actually update the cache. For + * now, always refetch link data here. */ + + _lookup_cached_link (cache, obj->link.ifindex, completed_from_cache, &link_cached); + if ( link_cached + && link_cached->_link.netlink.is_in_netlink + && link_cached->link.type == NM_LINK_TYPE_WIREGUARD) + obj->_link.wireguard_family_id = link_cached->_link.wireguard_family_id; + else + obj->_link.wireguard_family_id = -1; + + if (obj->_link.wireguard_family_id < 0) + obj->_link.wireguard_family_id = genl_ctrl_resolve (genl, "wireguard"); + + if (obj->_link.wireguard_family_id >= 0) { + lnk_data_new = _wireguard_read_info (platform, + genl, + obj->_link.wireguard_family_id, + obj->link.ifindex); + } + + if ( lnk_data_new + && obj->_link.netlink.lnk + && nmp_object_equal (obj->_link.netlink.lnk, lnk_data_new)) + nmp_object_unref (lnk_data_new); + else { + nmp_object_unref (obj->_link.netlink.lnk); + obj->_link.netlink.lnk = lnk_data_new; + } + } + obj->_link.netlink.is_in_netlink = TRUE; return g_steal_pointer (&obj); } diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 0020acc924..7ddcf41ae5 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -5532,67 +5532,57 @@ nm_platform_lnk_vxlan_to_string (const NMPlatformLnkVxlan *lnk, char *buf, gsize } const char * -nm_platform_wireguard_peer_to_string (const NMWireGuardPeer *peer, char *buf, gsize len) +nm_platform_wireguard_peer_to_string (const NMPWireGuardPeer *peer, char *buf, gsize len) { - gs_free char *public_b64 = NULL; - char s_address[INET6_ADDRSTRLEN] = {0}; - char s_endpoint[INET6_ADDRSTRLEN + NI_MAXSERV + sizeof("endpoint []:") + 1] = {0}; - guint8 nonzero_key = 0; - gsize i; + gs_free char *public_key_b64 = NULL; + char s_endpoint[NM_UTILS_INET_ADDRSTRLEN + 100]; + char s_addr[NM_UTILS_INET_ADDRSTRLEN]; + guint i; nm_utils_to_string_buffer_init (&buf, &len); - if (peer->endpoint.addr.sa_family == AF_INET || peer->endpoint.addr.sa_family == AF_INET6) { - char s_service[NI_MAXSERV]; - socklen_t addr_len = 0; - - if (peer->endpoint.addr.sa_family == AF_INET) - addr_len = sizeof (struct sockaddr_in); - else if (peer->endpoint.addr.sa_family == AF_INET6) - addr_len = sizeof (struct sockaddr_in6); - if (!getnameinfo (&peer->endpoint.addr, addr_len, s_address, sizeof(s_address), s_service, sizeof(s_service), NI_DGRAM | NI_NUMERICSERV | NI_NUMERICHOST)) { - if (peer->endpoint.addr.sa_family == AF_INET6 && strchr (s_address, ':')) - g_snprintf(s_endpoint, sizeof (s_endpoint), "endpoint [%s]:%s ", s_address, s_service); - else - g_snprintf(s_endpoint, sizeof (s_endpoint), "endpoint %s:%s ", s_address, s_service); - } - } - - - for (i = 0; i < sizeof (peer->preshared_key); i++) - nonzero_key |= peer->preshared_key[i]; + if (peer->endpoint_family == AF_INET) { + nm_sprintf_buf (s_endpoint, + " endpoint %s:%u", + nm_utils_inet4_ntop (peer->endpoint_addr.addr4, s_addr), + (guint) peer->endpoint_port); + } else if (peer->endpoint_family == AF_INET6) { + nm_sprintf_buf (s_endpoint, + " endpoint [%s]:%u", + nm_utils_inet6_ntop (&peer->endpoint_addr.addr6, s_addr), + (guint) peer->endpoint_port); + } else + s_endpoint[0] = '\0'; - public_b64 = g_base64_encode (peer->public_key, sizeof (peer->public_key)); + public_key_b64 = g_base64_encode (peer->public_key, sizeof (peer->public_key)); nm_utils_strbuf_append (&buf, &len, - "{ " - "public_key %s " - "%s" /* preshared key indicator */ + "public-key %s" + "%s" /* preshared-key */ "%s" /* endpoint */ - "rx %"G_GUINT64_FORMAT" " - "tx %"G_GUINT64_FORMAT" " - "allowedips (%"G_GSIZE_FORMAT") {", - public_b64, - nonzero_key ? "preshared_key (hidden) " : "", + " rx %"G_GUINT64_FORMAT + " tx %"G_GUINT64_FORMAT + "%s", /* allowed-ips */ + public_key_b64, + nm_utils_mem_all_zero (peer->preshared_key, sizeof (peer->preshared_key)) + ? "" + : " preshared-key (hidden)", s_endpoint, peer->rx_bytes, peer->tx_bytes, - peer->allowedips_len); - - - for (i = 0; i < peer->allowedips_len; i++) { - NMWireGuardAllowedIP *allowedip = &peer->allowedips[i]; - const char *ret; + peer->allowed_ips_len > 0 + ? " allowed-ips" + : ""); - ret = inet_ntop (allowedip->family, &allowedip->ip, s_address, sizeof(s_address)); + for (i = 0; i < peer->allowed_ips_len; i++) { + const NMPWireGuardAllowedIP *allowed_ip = &peer->allowed_ips[i]; nm_utils_strbuf_append (&buf, &len, " %s/%u", - ret ? s_address : "", - allowedip->mask); + nm_utils_inet_ntop (allowed_ip->family, &allowed_ip->addr, s_addr), + allowed_ip->mask); } - nm_utils_strbuf_append_str (&buf, &len, " } }"); return buf; } @@ -5600,25 +5590,26 @@ const char * nm_platform_lnk_wireguard_to_string (const NMPlatformLnkWireGuard *lnk, char *buf, gsize len) { gs_free char *public_b64 = NULL; - guint8 nonzero_key = 0; - gsize i; if (!nm_utils_to_string_buffer_init_null (lnk, &buf, &len)) return buf; - public_b64 = g_base64_encode (lnk->public_key, sizeof (lnk->public_key)); - - for (i = 0; i < sizeof (lnk->private_key); i++) - nonzero_key |= lnk->private_key[i]; + if (!nm_utils_mem_all_zero (lnk->public_key, sizeof (lnk->public_key))) + public_b64 = g_base64_encode (lnk->public_key, sizeof (lnk->public_key)); g_snprintf (buf, len, - "wireguard " - "public_key %s " - "%s" /* private key indicator */ - "listen_port %u " - "fwmark 0x%x", - public_b64, - nonzero_key ? "private_key (hidden) " : "", + "wireguard" + "%s%s" /* public-key */ + "%s" /* private-key */ + " listen-port %u" + " fwmark 0x%x", + public_b64 + ? " public-key " + : "", + public_b64 ?: "", + nm_utils_mem_all_zero (lnk->private_key, sizeof (lnk->private_key)) + ? "" + : " private-key (hidden)", lnk->listen_port, lnk->fwmark); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index a1171342f1..11495aff4f 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -752,11 +752,14 @@ typedef struct { bool l3miss:1; } NMPlatformLnkVxlan; +#define NMP_WIREGUARD_PUBLIC_KEY_LEN 32 +#define NMP_WIREGUARD_SYMMETRIC_KEY_LEN 32 + typedef struct { - guint8 private_key[NM_WG_PUBLIC_KEY_LEN]; - guint8 public_key[NM_WG_PUBLIC_KEY_LEN]; - guint16 listen_port; guint32 fwmark; + guint16 listen_port; + guint8 private_key[NMP_WIREGUARD_PUBLIC_KEY_LEN]; + guint8 public_key[NMP_WIREGUARD_PUBLIC_KEY_LEN]; } NMPlatformLnkWireGuard; typedef enum { @@ -1463,7 +1466,8 @@ const char *nm_platform_vlan_qos_mapping_to_string (const char *name, char *buf, gsize len); -const char *nm_platform_wireguard_peer_to_string (const NMWireGuardPeer *peer, +struct _NMPWireGuardPeer; +const char *nm_platform_wireguard_peer_to_string (const struct _NMPWireGuardPeer *peer, char *buf, gsize len); diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index 4358f8d714..fdc27440cc 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -27,6 +27,7 @@ #include #include "nm-utils.h" +#include "nm-utils/nm-secret-utils.h" #include "nm-core-utils.h" #include "nm-platform-utils.h" @@ -347,117 +348,92 @@ _vlan_xgress_qos_mappings_cpy (guint *dst_n_map, /*****************************************************************************/ static void -_wireguard_peers_hash_update (gsize n_peers, - const NMWireGuardPeer *peers, - NMHashState *h) -{ - gsize i, j; - - nm_hash_update_val (h, n_peers); - for (i = 0; i < n_peers; i++) { - const NMWireGuardPeer *p = &peers[i]; - - nm_hash_update (h, p->public_key, sizeof (p->public_key)); - nm_hash_update (h, p->preshared_key, sizeof (p->preshared_key)); - nm_hash_update_vals (h, - p->persistent_keepalive_interval, - p->allowedips_len, - p->rx_bytes, - p->tx_bytes, - p->last_handshake_time.tv_sec, - p->last_handshake_time.tv_nsec, - p->endpoint.addr.sa_family); - - if (p->endpoint.addr.sa_family == AF_INET) - nm_hash_update_val (h, p->endpoint.addr4); - else if (p->endpoint.addr.sa_family == AF_INET6) - nm_hash_update_val (h, p->endpoint.addr6); - else if (p->endpoint.addr.sa_family != AF_UNSPEC) - g_assert_not_reached (); - - for (j = 0; j < p->allowedips_len; j++) { - const NMWireGuardAllowedIP *ip = &p->allowedips[j]; - - nm_hash_update_vals (h, ip->family, ip->mask); - - if (ip->family == AF_INET) - nm_hash_update_val (h, ip->ip.addr4); - else if (ip->family == AF_INET6) - nm_hash_update_val (h, ip->ip.addr6); - else if (ip->family != AF_UNSPEC) - g_assert_not_reached (); - } - } +_wireguard_allowed_ip_hash_update (const NMPWireGuardAllowedIP *ip, + NMHashState *h) +{ + nm_hash_update_vals (h, ip->family, + ip->mask); + + if (ip->family == AF_INET) + nm_hash_update_val (h, ip->addr.addr4); + else if (ip->family == AF_INET6) + nm_hash_update_val (h, ip->addr.addr6); } static int -_wireguard_peers_cmp (gsize n_peers, - const NMWireGuardPeer *p1, - const NMWireGuardPeer *p2) -{ - gsize i, j; - - for (i = 0; i < n_peers; i++) { - const NMWireGuardPeer *a = &p1[i]; - const NMWireGuardPeer *b = &p2[i]; - - NM_CMP_FIELD (a, b, last_handshake_time.tv_sec); - NM_CMP_FIELD (a, b, last_handshake_time.tv_nsec); - NM_CMP_FIELD (a, b, rx_bytes); - NM_CMP_FIELD (a, b, tx_bytes); - NM_CMP_FIELD (a, b, allowedips_len); - NM_CMP_FIELD (a, b, persistent_keepalive_interval); - NM_CMP_FIELD (a, b, endpoint.addr.sa_family); - NM_CMP_FIELD_MEMCMP (a, b, public_key); - NM_CMP_FIELD_MEMCMP (a, b, preshared_key); - - if (a->endpoint.addr.sa_family == AF_INET) - NM_CMP_FIELD_MEMCMP (a, b, endpoint.addr4); - else if (a->endpoint.addr.sa_family == AF_INET6) - NM_CMP_FIELD_MEMCMP (a, b, endpoint.addr6); - else if (a->endpoint.addr.sa_family != AF_UNSPEC) - g_assert_not_reached (); - - for (j = 0; j < a->allowedips_len; j++) { - const NMWireGuardAllowedIP *aip = &a->allowedips[j]; - const NMWireGuardAllowedIP *bip = &b->allowedips[j]; - - NM_CMP_FIELD (aip, bip, family); - NM_CMP_FIELD (aip, bip, mask); - - if (aip->family == AF_INET) - NM_CMP_FIELD_MEMCMP (&aip->ip, &bip->ip, addr4); - else if (aip->family == AF_INET6) - NM_CMP_FIELD_MEMCMP (&aip->ip, &bip->ip, addr6); - else if (aip->family != AF_UNSPEC) - g_assert_not_reached (); - } - } +_wireguard_allowed_ip_cmp (const NMPWireGuardAllowedIP *a, + const NMPWireGuardAllowedIP *b) +{ + NM_CMP_SELF (a, b); + + NM_CMP_FIELD (a, b, family); + NM_CMP_FIELD (a, b, mask); + + if (a->family == AF_INET) + NM_CMP_FIELD (a, b, addr.addr4); + else if (a->family == AF_INET6) + NM_CMP_FIELD_IN6ADDR (a, b, addr.addr6); return 0; } static void -_wireguard_peers_cpy (gsize *dst_n_peers, - NMWireGuardPeer **dst_peers, - gsize src_n_peers, - const NMWireGuardPeer *src_peers) -{ - if (src_n_peers == 0) { - g_clear_pointer (dst_peers, g_free); - *dst_n_peers = 0; - } else if ( src_n_peers != *dst_n_peers - || _wireguard_peers_cmp (src_n_peers, *dst_peers, src_peers) != 0) { - gsize i; - g_clear_pointer (dst_peers, g_free); - *dst_n_peers = src_n_peers; - if (src_n_peers > 0) - *dst_peers = nm_memdup (src_peers, sizeof (*src_peers) * src_n_peers); - for (i = 0; i < src_n_peers; i++) { - dst_peers[i]->allowedips = nm_memdup (src_peers[i].allowedips, sizeof (src_peers[i].allowedips) * src_peers[i].allowedips_len); - dst_peers[i]->allowedips_len = src_peers[i].allowedips_len; - } +_wireguard_peer_hash_update (const NMPWireGuardPeer *peer, + NMHashState *h) +{ + guint i; + + nm_hash_update (h, peer->public_key, sizeof (peer->public_key)); + nm_hash_update (h, peer->preshared_key, sizeof (peer->preshared_key)); + nm_hash_update_vals (h, + peer->persistent_keepalive_interval, + peer->allowed_ips_len, + peer->rx_bytes, + peer->tx_bytes, + peer->last_handshake_time.tv_sec, + peer->last_handshake_time.tv_nsec, + peer->endpoint_port, + peer->endpoint_family); + + if (peer->endpoint_family == AF_INET) + nm_hash_update_val (h, peer->endpoint_addr.addr4); + else if (peer->endpoint_family == AF_INET6) + nm_hash_update_val (h, peer->endpoint_addr.addr6); + + for (i = 0; i < peer->allowed_ips_len; i++) + _wireguard_allowed_ip_hash_update (&peer->allowed_ips[i], h); +} + +static int +_wireguard_peer_cmp (const NMPWireGuardPeer *a, + const NMPWireGuardPeer *b) +{ + guint i; + + NM_CMP_SELF (a, b); + + NM_CMP_FIELD (a, b, last_handshake_time.tv_sec); + NM_CMP_FIELD (a, b, last_handshake_time.tv_nsec); + NM_CMP_FIELD (a, b, rx_bytes); + NM_CMP_FIELD (a, b, tx_bytes); + NM_CMP_FIELD (a, b, allowed_ips_len); + NM_CMP_FIELD (a, b, persistent_keepalive_interval); + NM_CMP_FIELD (a, b, endpoint_port); + NM_CMP_FIELD (a, b, endpoint_family); + NM_CMP_FIELD_MEMCMP (a, b, public_key); + NM_CMP_FIELD_MEMCMP (a, b, preshared_key); + + if (a->endpoint_family == AF_INET) + NM_CMP_FIELD (a, b, endpoint_addr.addr4); + else if (a->endpoint_family == AF_INET6) + NM_CMP_FIELD_IN6ADDR (a, b, endpoint_addr.addr6); + + for (i = 0; i < a->allowed_ips_len; i++) { + NM_CMP_RETURN (_wireguard_allowed_ip_cmp (&a->allowed_ips[i], + &b->allowed_ips[i])); } + + return 0; } /*****************************************************************************/ @@ -587,12 +563,25 @@ _vt_cmd_obj_dispose_lnk_vlan (NMPObject *obj) } static void -_vt_cmd_obj_dispose_lnk_wireguard (NMPObject *obj) +_wireguard_clear (NMPObjectLnkWireGuard *lnk) { - if (obj->_lnk_wireguard.peers_len) - g_free (obj->_lnk_wireguard.peers[0].allowedips); + guint i; + + nm_explicit_bzero (lnk->_public.private_key, + sizeof (lnk->_public.private_key)); + for (i = 0; i < lnk->peers_len; i++) { + NMPWireGuardPeer *peer = (NMPWireGuardPeer *) &lnk->peers[i]; + + nm_explicit_bzero (peer->preshared_key, sizeof (peer->preshared_key)); + } + g_free ((gpointer) lnk->peers); + g_free ((gpointer) lnk->_allowed_ips_buf); +} - g_free (obj->_lnk_wireguard.peers); +static void +_vt_cmd_obj_dispose_lnk_wireguard (NMPObject *obj) +{ + _wireguard_clear (&obj->_lnk_wireguard); } static NMPObject * @@ -859,7 +848,7 @@ _vt_cmd_obj_to_string_lnk_wireguard (const NMPObject *obj, NMPObjectToStringMode const NMPClass *klass; char buf2[sizeof (_nm_utils_to_string_buffer)]; char *b; - gsize i, l; + guint i; klass = NMP_OBJECT_GET_CLASS (obj); @@ -871,23 +860,26 @@ _vt_cmd_obj_to_string_lnk_wireguard (const NMPObject *obj, NMPObjectToStringMode b = buf; nm_utils_strbuf_append (&b, &buf_size, - "[%s,%p,%u,%calive,%cvisible; %s " - "peers (%" G_GSIZE_FORMAT ") {", + "[%s,%p,%u,%calive,%cvisible; %s" + "%s", klass->obj_type_name, obj, obj->parent._ref_count, nmp_object_is_alive (obj) ? '+' : '-', nmp_object_is_visible (obj) ? '+' : '-', nmp_object_to_string (obj, NMP_OBJECT_TO_STRING_PUBLIC, buf2, sizeof (buf2)), - obj->_lnk_wireguard.peers_len); + obj->_lnk_wireguard.peers_len > 0 + ? " peers {" + : ""); for (i = 0; i < obj->_lnk_wireguard.peers_len; i++) { - const NMWireGuardPeer *peer = &obj->_lnk_wireguard.peers[i]; + const NMPWireGuardPeer *peer = &obj->_lnk_wireguard.peers[i]; + + nm_utils_strbuf_append_str (&b, &buf_size, " { "); nm_platform_wireguard_peer_to_string (peer, b, buf_size); - l = strlen (b); - b += l; - buf_size -= l; + nm_utils_strbuf_seek_end (&b, &buf_size); + nm_utils_strbuf_append_str (&b, &buf_size, " }"); } - - nm_utils_strbuf_append_str (&b, &buf_size, " }"); + if (obj->_lnk_wireguard.peers_len) + nm_utils_strbuf_append_str (&b, &buf_size, " }"); return buf; case NMP_OBJECT_TO_STRING_PUBLIC: @@ -947,6 +939,7 @@ _vt_cmd_obj_hash_update_link (const NMPObject *obj, NMHashState *h) nm_platform_link_hash_update (&obj->link, h); nm_hash_update_vals (h, obj->_link.netlink.is_in_netlink, + obj->_link.wireguard_family_id, obj->_link.udev.device); if (obj->_link.netlink.lnk) nmp_object_hash_update (obj->_link.netlink.lnk, h); @@ -969,10 +962,15 @@ _vt_cmd_obj_hash_update_lnk_vlan (const NMPObject *obj, NMHashState *h) static void _vt_cmd_obj_hash_update_lnk_wireguard (const NMPObject *obj, NMHashState *h) { + guint i; + nm_assert (NMP_OBJECT_GET_TYPE (obj) == NMP_OBJECT_TYPE_LNK_WIREGUARD); nm_platform_lnk_wireguard_hash_update (&obj->lnk_wireguard, h); - _wireguard_peers_hash_update (obj->_lnk_wireguard.peers_len, obj->_lnk_wireguard.peers, h); + + nm_hash_update_val (h, obj->_lnk_wireguard.peers_len); + for (i = 0; i < obj->_lnk_wireguard.peers_len; i++) + _wireguard_peer_hash_update (&obj->_lnk_wireguard.peers[i], h); } int @@ -980,12 +978,7 @@ nmp_object_cmp (const NMPObject *obj1, const NMPObject *obj2) { const NMPClass *klass1, *klass2; - if (obj1 == obj2) - return 0; - if (!obj1) - return -1; - if (!obj2) - return 1; + NM_CMP_SELF (obj1, obj2); g_return_val_if_fail (NMP_OBJECT_IS_VALID (obj1), -1); g_return_val_if_fail (NMP_OBJECT_IS_VALID (obj2), 1); @@ -1006,16 +999,11 @@ nmp_object_cmp (const NMPObject *obj1, const NMPObject *obj2) static int _vt_cmd_obj_cmp_link (const NMPObject *obj1, const NMPObject *obj2) { - int i; + NM_CMP_RETURN (nm_platform_link_cmp (&obj1->link, &obj2->link)); + NM_CMP_DIRECT (obj1->_link.netlink.is_in_netlink, obj2->_link.netlink.is_in_netlink); + NM_CMP_RETURN (nmp_object_cmp (obj1->_link.netlink.lnk, obj2->_link.netlink.lnk)); + NM_CMP_DIRECT (obj1->_link.wireguard_family_id, obj2->_link.wireguard_family_id); - i = nm_platform_link_cmp (&obj1->link, &obj2->link); - if (i) - return i; - if (obj1->_link.netlink.is_in_netlink != obj2->_link.netlink.is_in_netlink) - return obj1->_link.netlink.is_in_netlink ? -1 : 1; - i = nmp_object_cmp (obj1->_link.netlink.lnk, obj2->_link.netlink.lnk); - if (i) - return i; if (obj1->_link.udev.device != obj2->_link.udev.device) { if (!obj1->_link.udev.device) return -1; @@ -1028,6 +1016,7 @@ _vt_cmd_obj_cmp_link (const NMPObject *obj1, const NMPObject *obj2) * Have this check as very last. */ return (obj1->_link.udev.device < obj2->_link.udev.device) ? -1 : 1; } + return 0; } @@ -1056,16 +1045,16 @@ _vt_cmd_obj_cmp_lnk_vlan (const NMPObject *obj1, const NMPObject *obj2) static int _vt_cmd_obj_cmp_lnk_wireguard (const NMPObject *obj1, const NMPObject *obj2) { - int c; + guint i; - c = nm_platform_lnk_wireguard_cmp (&obj1->lnk_wireguard, &obj2->lnk_wireguard); - if (c) - return c; + NM_CMP_RETURN (nm_platform_lnk_wireguard_cmp (&obj1->lnk_wireguard, &obj2->lnk_wireguard)); + + NM_CMP_FIELD (obj1, obj2, _lnk_wireguard.peers_len); - if (obj1->_lnk_wireguard.peers_len != obj2->_lnk_wireguard.peers_len) - return obj1->_lnk_wireguard.peers_len < obj2->_lnk_wireguard.peers_len ? -1 : 1; + for (i = 0; i < obj1->_lnk_wireguard.peers_len; i++) + NM_CMP_RETURN (_wireguard_peer_cmp (&obj1->_lnk_wireguard.peers[i], &obj2->_lnk_wireguard.peers[i])); - return _wireguard_peers_cmp(obj1->_lnk_wireguard.peers_len, obj1->_lnk_wireguard.peers, obj2->_lnk_wireguard.peers); + return 0; } /* @src is a const object, which is not entirely correct for link types, where @@ -1137,9 +1126,36 @@ _vt_cmd_obj_copy_lnk_vlan (NMPObject *dst, const NMPObject *src) static void _vt_cmd_obj_copy_lnk_wireguard (NMPObject *dst, const NMPObject *src) { - dst->lnk_wireguard = src->lnk_wireguard; - _wireguard_peers_cpy (&dst->_lnk_wireguard.peers_len, &dst->_lnk_wireguard.peers, - src->_lnk_wireguard.peers_len, src->_lnk_wireguard.peers); + guint i; + + nm_assert (dst != src); + + _wireguard_clear (&dst->_lnk_wireguard); + + dst->_lnk_wireguard = src->_lnk_wireguard; + + dst->_lnk_wireguard.peers = nm_memdup (dst->_lnk_wireguard.peers, + sizeof (NMPWireGuardPeer) * dst->_lnk_wireguard.peers_len); + dst->_lnk_wireguard._allowed_ips_buf = nm_memdup (dst->_lnk_wireguard._allowed_ips_buf, + sizeof (NMPWireGuardAllowedIP) * dst->_lnk_wireguard._allowed_ips_buf_len); + + /* all the peers' pointers point into the buffer. They need to be readjusted. */ + for (i = 0; i < dst->_lnk_wireguard.peers_len; i++) { + NMPWireGuardPeer *peer = (NMPWireGuardPeer *) &dst->_lnk_wireguard.peers[i]; + + if (peer->allowed_ips_len == 0) { + nm_assert (!peer->allowed_ips); + continue; + } + nm_assert (dst->_lnk_wireguard._allowed_ips_buf_len > 0); + nm_assert (src->_lnk_wireguard._allowed_ips_buf); + nm_assert (peer->allowed_ips >= src->_lnk_wireguard._allowed_ips_buf); + nm_assert (&peer->allowed_ips[peer->allowed_ips_len] <= &src->_lnk_wireguard._allowed_ips_buf[src->_lnk_wireguard._allowed_ips_buf_len]); + + peer->allowed_ips = &dst->_lnk_wireguard._allowed_ips_buf[peer->allowed_ips - src->_lnk_wireguard._allowed_ips_buf]; + } + + nm_assert (nmp_object_equal (src, dst)); } #define _vt_cmd_plobj_id_copy(type, plat_type, cmd) \ @@ -3084,4 +3100,3 @@ const NMPClass _nmp_classes[NMP_OBJECT_TYPE_MAX] = { .cmd_plobj_cmp = (int (*) (const NMPlatformObject *obj1, const NMPlatformObject *obj2)) nm_platform_lnk_vlan_cmp, }, }; - diff --git a/src/platform/nmp-object.h b/src/platform/nmp-object.h index ba63a3a34e..97be832177 100644 --- a/src/platform/nmp-object.h +++ b/src/platform/nmp-object.h @@ -27,6 +27,36 @@ struct udev_device; +/*****************************************************************************/ + +typedef struct { + NMIPAddr addr; + guint8 family; + guint8 mask; +} NMPWireGuardAllowedIP; + +typedef struct _NMPWireGuardPeer { + NMIPAddr endpoint_addr; + struct timespec last_handshake_time; + guint64 rx_bytes; + guint64 tx_bytes; + union { + const NMPWireGuardAllowedIP *allowed_ips; + guint _construct_idx_start; + }; + union { + guint allowed_ips_len; + guint _construct_idx_end; + }; + guint16 persistent_keepalive_interval; + guint16 endpoint_port; + guint8 public_key[NMP_WIREGUARD_PUBLIC_KEY_LEN]; + guint8 preshared_key[NMP_WIREGUARD_SYMMETRIC_KEY_LEN]; + guint8 endpoint_family; +} NMPWireGuardPeer; + +/*****************************************************************************/ + typedef enum { /*< skip >*/ NMP_OBJECT_TO_STRING_ID, NMP_OBJECT_TO_STRING_PUBLIC, @@ -168,6 +198,9 @@ typedef struct { /* Auxiliary data object for Wi-Fi and WPAN */ GObject *ext_data; + /* FIXME: not every NMPObjectLink should pay the price for tracking + * the wireguard family id. This should be tracked via ext_data, which + * would be exactly the right place. */ int wireguard_family_id; } NMPObjectLink; @@ -220,9 +253,10 @@ typedef struct { typedef struct { NMPlatformLnkWireGuard _public; - - gsize peers_len; - NMWireGuardPeer *peers; + const NMPWireGuardPeer *peers; + const NMPWireGuardAllowedIP *_allowed_ips_buf; + guint peers_len; + guint _allowed_ips_buf_len; } NMPObjectLnkWireGuard; typedef struct { -- cgit v1.2.1