From f3c2c07e37945b8725af097decf5ee6ffbfc0b9a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 10:08:41 +0200 Subject: [PATCH 1/5] shared: add NM_HASH_SEED_16() macro (cherry picked from commit 72e0b522ff8a7f3e9b4c20aa29d6b179a616c5f4) --- shared/nm-glib-aux/nm-hash-utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index f13e0b6d56..aa0a3d7c0a 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -26,6 +26,9 @@ /*****************************************************************************/ +#define NM_HASH_SEED_16(a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, aa, ab, ac, ad, ae, af) \ + ((const guint8[16]) { a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, aa, ab, ac, ad, ae, af }) + void nm_hash_siphash42_init (CSipHash *h, guint static_seed); /* Siphash24 of binary buffer @arr and @len, using the randomized seed from From 327a92ae2b6c18a9cc438f276012279c63a07899 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 30 Jul 2019 18:38:48 +0200 Subject: [PATCH 2/5] wireguard: fix crash in _auto_default_route_init() #3 0x00007fb0aa9e7d3d in g_return_if_fail_warning (log_domain=log_domain@entry=0x562295fd5ee3 "libnm", pretty_function=pretty_function@entry=0x562295fd71d0 <__func__.35180> "_connection_get_setting_check", expression=expression@entry=0x562295f8edf7 "NM_IS_CONNECTION (connection)") at ../glib/gmessages.c:2767 #4 0x0000562295df151a in _connection_get_setting_check (connection=0x0, setting_type=0x562297b17050 [NMSettingWireGuard/NMSetting]) at libnm-core/nm-connection.c:207 #5 0x0000562295df151a in _connection_get_setting_check (connection=0x0, setting_type=0x562297b17050 [NMSettingWireGuard/NMSetting]) at libnm-core/nm-connection.c:205 #6 0x0000562295ef132a in _nm_connection_get_setting (type=, connection=0x0) at ./libnm-core/nm-core-internal.h:483 #7 0x0000562295ef132a in _auto_default_route_init (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard]) at src/devices/nm-device-wireguard.c:443 #8 0x0000562295ef1b98 in coerce_route_table (device=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2, route_table=0, is_user_config=) at src/devices/nm-device-wireguard.c:565 #9 0x0000562295ea42ae in _get_route_table (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard], addr_family=addr_family@entry=2) at src/devices/nm-device.c:2311 #10 0x0000562295ea4593 in nm_device_get_route_table (self=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2) at src/devices/nm-device.c:2338 #11 0x0000562295eabde0 in ip_config_merge_and_apply (self=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2, commit=1) at src/devices/nm-device.c:7590 #12 0x0000562295ed2f0b in device_link_changed (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard]) at src/devices/nm-device.c:3939 #13 0x00007fb0aa9dc7db in g_idle_dispatch (source=source@entry=0x562297bf0b30, callback=0x562295ed2880 , user_data=0x562297bf82b0) at ../glib/gmain.c:5627 #14 0x00007fb0aa9dfedd in g_main_dispatch (context=0x562297a28090) at ../glib/gmain.c:3189 #15 0x00007fb0aa9dfedd in g_main_context_dispatch (context=context@entry=0x562297a28090) at ../glib/gmain.c:3854 #16 0x00007fb0aa9e0270 in g_main_context_iterate (context=0x562297a28090, block=block@entry=1, dispatch=dispatch@entry=1, self=) at ../glib/gmain.c:3927 #17 0x00007fb0aa9e05a3 in g_main_loop_run (loop=0x562297a0b380) at ../glib/gmain.c:4123 #18 0x0000562295d0b147 in main (argc=, argv=) at src/main.c:465 https://bugzilla.redhat.com/show_bug.cgi?id=1734383 (cherry picked from commit 47fc1a4293437a88adfd247734e32fa1b86ca7a9) --- src/devices/nm-device-wireguard.c | 63 +++++++++++++++++-------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index d36573cf25..e997d2e823 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -423,10 +423,10 @@ _auto_default_route_init (NMDeviceWireGuard *self) { NMDeviceWireGuardPrivate *priv = NM_DEVICE_WIREGUARD_GET_PRIVATE (self); NMConnection *connection; - NMSettingWireGuard *s_wg; - gboolean enabled_v4; - gboolean enabled_v6; + gboolean enabled_v4 = FALSE; + gboolean enabled_v6 = FALSE; gboolean refreshing_only; + guint32 new_fwmark = 0; guint32 old_fwmark; char sbuf1[100]; @@ -436,40 +436,47 @@ _auto_default_route_init (NMDeviceWireGuard *self) refreshing_only = priv->auto_default_route_initialized && priv->auto_default_route_refresh; - priv->auto_default_route_refresh = FALSE; - - connection = nm_device_get_applied_connection (NM_DEVICE (self)); - - s_wg = _nm_connection_get_setting (connection, NM_TYPE_SETTING_WIREGUARD); old_fwmark = priv->auto_default_route_fwmark; - priv->auto_default_route_fwmark = nm_setting_wireguard_get_fwmark (s_wg); + connection = nm_device_get_applied_connection (NM_DEVICE (self)); + if (connection) { + NMSettingWireGuard *s_wg; - _auto_default_route_get_enabled (s_wg, - connection, - &enabled_v4, - &enabled_v6); + s_wg = _nm_connection_get_setting (connection, NM_TYPE_SETTING_WIREGUARD); + + new_fwmark = nm_setting_wireguard_get_fwmark (s_wg); + + _auto_default_route_get_enabled (s_wg, + connection, + &enabled_v4, + &enabled_v6); + } + + if ( ( enabled_v4 + || enabled_v6) + && new_fwmark == 0u) { + if (refreshing_only) + new_fwmark = old_fwmark; + else + new_fwmark = _auto_default_route_find_unused_table (nm_device_get_platform (NM_DEVICE (self))); + } + + priv->auto_default_route_refresh = FALSE; + priv->auto_default_route_fwmark = new_fwmark; priv->auto_default_route_enabled_4 = enabled_v4; priv->auto_default_route_enabled_6 = enabled_v6; priv->auto_default_route_initialized = TRUE; - if ( ( priv->auto_default_route_enabled_4 - || priv->auto_default_route_enabled_6) - && priv->auto_default_route_fwmark == 0u) { - if (refreshing_only) - priv->auto_default_route_fwmark = old_fwmark; - else - priv->auto_default_route_fwmark = _auto_default_route_find_unused_table (nm_device_get_platform (NM_DEVICE (self))); + if (connection) { + _LOGT (LOGD_DEVICE, + "auto-default-route is %s for IPv4 and %s for IPv6%s", + priv->auto_default_route_enabled_4 ? "enabled" : "disabled", + priv->auto_default_route_enabled_6 ? "enabled" : "disabled", + priv->auto_default_route_enabled_4 || priv->auto_default_route_enabled_6 + ? nm_sprintf_buf (sbuf1, " (fwmark 0x%x)", priv->auto_default_route_fwmark) + : ""); } - - _LOGT (LOGD_DEVICE, - "auto-default-route is %s for IPv4 and %s for IPv6%s", - priv->auto_default_route_enabled_4 ? "enabled" : "disabled", - priv->auto_default_route_enabled_6 ? "enabled" : "disabled", - priv->auto_default_route_enabled_4 || priv->auto_default_route_enabled_6 - ? nm_sprintf_buf (sbuf1, " (fwmark 0x%x)", priv->auto_default_route_fwmark) - : ""); } static GPtrArray * From 93300c13201cfea0011aa46d5601ebbe4afe201c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 30 Jul 2019 18:50:15 +0200 Subject: [PATCH 3/5] wireguard: clear cached auto-default-route setting in act_stage1_prepare() We call _auto_default_route_init() at various places, for example during coerce_route_table(). We cannot be sure that we don't call it before activation starts (before stage1) or after device-cleanup. That means, upon activation, we need to clear the state first. Do that in act_stage1_prepare(). (cherry picked from commit dc219662fa31b6bc453c177c10e03442483de21e) --- src/devices/nm-device-wireguard.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index e997d2e823..0f357a1106 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1624,6 +1624,17 @@ link_config_delayed_resolver_cb (gpointer user_data) return G_SOURCE_REMOVE; } +static NMActStageReturn +act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) +{ + NMDeviceWireGuardPrivate *priv = NM_DEVICE_WIREGUARD_GET_PRIVATE (device); + + priv->auto_default_route_initialized = FALSE; + priv->auto_default_route_priority_initialized = FALSE; + + return NM_DEVICE_CLASS (nm_device_wireguard_parent_class)->act_stage1_prepare (device, out_failure_reason); +} + static NMActStageReturn act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) @@ -2064,6 +2075,7 @@ nm_device_wireguard_class_init (NMDeviceWireGuardClass *klass) device_class->connection_type_check_compatible = NM_SETTING_WIREGUARD_SETTING_NAME; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_WIREGUARD); + device_class->act_stage1_prepare = act_stage1_prepare; device_class->state_changed = device_state_changed; device_class->create_and_realize = create_and_realize; device_class->act_stage2_config = act_stage2_config; From ed22b45450a71278587daca3d4833e4133db6270 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 10:01:04 +0200 Subject: [PATCH 4/5] wireguard: use fixed fwmark/rule-priority for auto-default-route With "wireguard.ip4-auto-default-route" and "wireguard.ip6-auto-default-route", NetworkManager automatically adds policy routing rules for the default route. For that, it needs to pick (up to) two numbers: - the fwmark. This is used both for WireGuard's fwmark setting and is also the number of the routing table where the default-route is added. - the rule priority. NetworkManager adds two policy routing rules, and we need to place them somewhere before the default rules (at 32766). Previously, we looked at exiting platform configuration and picked numbers that were not yet used. However, during restart of NetworkManager, we leave the interface up and after restart we will take over the previous configuration. At that point, we need to choose the same fwmark/priority, otherwise the configuration changes. But since we picked numbers that were not yet used, we would always choose different numbers. For routing rules that meant that after restart a second pair of rules was added. We possibly could store this data in the device's state-file. But that is complex. Instead, just pick numbers deterministically based on the connection's UUID. If the picked numbers are not suitable, then the user can still work around that: - for fwmark, the user can explicitly configure wireguard.fwmark setting. - for the policy routes, the user can explicitly add the rules with the desired priorirites (arguably, currently the default-route cannot be added like a regular route, so the table cannot be set. Possibly the user would have to add two /1 routes instead with suppress_prefixlength=1). (cherry picked from commit cfb497e49936d36867ae3175537c7d4f77259655) --- src/devices/nm-device-wireguard.c | 171 ++++++------------------------ 1 file changed, 32 insertions(+), 139 deletions(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index 0f357a1106..f4da539356 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -265,157 +265,51 @@ done: *out_enabled_v6 = (enabled_v6 == TRUE); } -static guint32 -_auto_default_route_find_unused_table (NMPlatform *platform) -{ - guint32 table; - int is_ipv4; - - for (table = 51820; TRUE; table++) { - const NMDedupMultiHeadEntry *head_entry; - const guint32 table_coerced = nm_platform_route_table_coerce (table); - NMDedupMultiIter iter; - const NMPObject *plobj; - - /* find a table/fwmark that is not yet in use. */ - - for (is_ipv4 = 0; is_ipv4 < 2; is_ipv4++) { - head_entry = nm_platform_lookup_object (platform, - is_ipv4 - ? NMP_OBJECT_TYPE_IP4_ROUTE - : NMP_OBJECT_TYPE_IP6_ROUTE, - -1); - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - if (NMP_OBJECT_CAST_IP_ROUTE (plobj)->table_coerced == table_coerced) - goto try_next_table; - } - } - - head_entry = nm_platform_lookup_object_by_addr_family (platform, - NMP_OBJECT_TYPE_ROUTING_RULE, - AF_UNSPEC); - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - const NMPlatformRoutingRule *rr = NMP_OBJECT_CAST_ROUTING_RULE (plobj); - - if (rr->fwmark == table) - goto try_next_table; - } - - head_entry = nm_platform_lookup_obj_type (platform, NMP_OBJECT_TYPE_LINK); - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - const NMPObject *lnk_wg; - - if (plobj->link.type != NM_LINK_TYPE_WIREGUARD) - continue; - - lnk_wg = plobj->_link.netlink.lnk; - - if (!lnk_wg) - continue; - - if (NMP_OBJECT_GET_TYPE (lnk_wg) != NMP_OBJECT_TYPE_LNK_WIREGUARD) - continue; - - if (NMP_OBJECT_CAST_LNK_WIREGUARD (lnk_wg)->fwmark == table) - goto try_next_table; - } - - return table; -try_next_table: - ; - } -} - -#define PRIO_WIDTH ((guint32) 2) - -static gboolean -_auto_default_route_find_priority_exists (const NMDedupMultiHeadEntry *head_entry, - guint32 priority) -{ - NMDedupMultiIter iter; - const NMPObject *plobj; - - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - const NMPlatformRoutingRule *rr = NMP_OBJECT_CAST_ROUTING_RULE (plobj); - - /* we don't differenciate between IPv4 vs. IPv6. There should be no - * conflicting rules with the same priority. */ - if ( rr->priority >= priority - && rr->priority < priority + PRIO_WIDTH) - return TRUE; - } - - return FALSE; -} +#define AUTO_RANDOM_RANGE 500u static guint32 -_auto_default_route_find_priority (NMPlatform *platform, - const char *uuid) +_auto_default_route_get_auto_fwmark (const char *uuid) { - const NMDedupMultiHeadEntry *head_entry; guint64 rnd_seed; - const guint32 PRIME_NUMBER = 1111567573u; - const guint32 RANGE_TOP = ((32766u - 2u * PRIO_WIDTH) / PRIO_WIDTH); - const guint32 RANGE_LEN1 = 200u; - const guint32 RANGE_LEN2 = (RANGE_TOP - 100u) - RANGE_LEN1; - guint32 range_len; - guint32 range_top; - guint32 prio_candidate = 0; - guint32 i_step; - guint32 i; - /* For the auto-default-route policy routing rule we add 4 rules (2 Ipv4 and 2 IPv6). - * Hence, we choose a priority for the first (of the two rules) and the second - * rule gets priority + 1. - * We want a priority that is - * - unused so far. - * - smaller than 32766u (which is the priority of the default rules for IPv4 and IPv6) - * - stable for each connection but different between connections (we hash the UUID - * as a "random" seed) - * - if possible, close to 32766u (RANGE_LEN1). Only otherwise fallback to the entire - * range (RANGE_LEN2). + /* we use the generated number as fwmark but also as routing table for + * the default-route. + * + * We pick a number + * + * - based on the connection's UUID (as stable seed). + * - larger than 51820u (arbitrarily) + * - one out of AUTO_RANDOM_RANGE */ - rnd_seed = c_siphash_hash ((const guint8 [16]) { 0xb9, 0x39, 0x8e, 0xed, 0x15, 0xb3, 0xd1, 0xc4, 0x5f, 0x45, 0x00, 0x4f, 0xec, 0xc2, 0x2b, 0x7e }, + rnd_seed = c_siphash_hash (NM_HASH_SEED_16 (0xb9, 0x39, 0x8e, 0xed, 0x15, 0xb3, 0xd1, 0xc4, 0x5f, 0x45, 0x00, 0x4f, 0xec, 0xc2, 0x2b, 0x7e), (const guint8 *) uuid, uuid ? strlen (uuid) + 1u : 0u); - head_entry = nm_platform_lookup_object_by_addr_family (platform, - NMP_OBJECT_TYPE_ROUTING_RULE, - AF_UNSPEC); + return 51820u + (rnd_seed % AUTO_RANDOM_RANGE); +} - range_len = RANGE_LEN1; - range_top = RANGE_TOP; +#define PRIO_WIDTH 2u -again: - i_step = ((guint32) rnd_seed) % range_len; - for (i = 0; i < range_len; i++) { +static guint32 +_auto_default_route_get_auto_priority (const char *uuid) +{ + const guint32 RANGE_TOP = 32766u - 1000u; + guint64 rnd_seed; - /* we sample the range in a stable, but somewhat arbitrary order to - * find an unused priority. */ - i_step = (i_step + PRIME_NUMBER) % range_len; + /* we pick a priority for the routing rules as follows: + * + * - use the connection's UUID as stable seed for the "random" number. + * - have it smaller than RANGE_TOP (32766u - 1000u), where 32766u is the priority of the default + * rules + * - we add 2 rules (PRIO_WIDTH). Hence only pick even priorites. + * - pick one out of AUTO_RANDOM_RANGE. */ - nm_assert (i_step < range_top); + rnd_seed = c_siphash_hash (NM_HASH_SEED_16 (0x99, 0x22, 0x4d, 0x7c, 0x37, 0xda, 0x8e, 0x7b, 0x2f, 0x55, 0x16, 0x7b, 0x75, 0xda, 0x42, 0xdc), + (const guint8 *) uuid, + uuid ? strlen (uuid) + 1u : 0u); - prio_candidate = (range_top - i_step) * PRIO_WIDTH; - - nm_assert (prio_candidate < 32766u); - - if (!_auto_default_route_find_priority_exists (head_entry, prio_candidate)) - return prio_candidate; - } - - if (range_len == RANGE_LEN1) { - /* within the narrow range close to RANGE_TOP we couldn't find any unused - * priority. Retry with the entire range... */ - range_len = RANGE_LEN2; - range_top -= RANGE_LEN1; - goto again; - } - - /* Couldn't find an unused one? Very odd, this really should not happen unless there - * are thousands of rules already. Just pick the last one we sampled. */ - return prio_candidate; + return RANGE_TOP - (((rnd_seed % (PRIO_WIDTH * AUTO_RANDOM_RANGE)) / PRIO_WIDTH) * PRIO_WIDTH); } static void @@ -459,7 +353,7 @@ _auto_default_route_init (NMDeviceWireGuard *self) if (refreshing_only) new_fwmark = old_fwmark; else - new_fwmark = _auto_default_route_find_unused_table (nm_device_get_platform (NM_DEVICE (self))); + new_fwmark = _auto_default_route_get_auto_fwmark (nm_connection_get_uuid (connection)); } priv->auto_default_route_refresh = FALSE; @@ -513,8 +407,7 @@ get_extra_rules (NMDevice *device) if (priv->auto_default_route_priority_initialized) priority = priv->auto_default_route_priority; else { - priority = _auto_default_route_find_priority (nm_device_get_platform (device), - nm_connection_get_uuid (connection)); + priority = _auto_default_route_get_auto_priority (nm_connection_get_uuid (connection)); priv->auto_default_route_priority = priority; priv->auto_default_route_priority_initialized = TRUE; } From 815acb999649368ba4d4ede29bbfa543230c6bfd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 10:17:59 +0200 Subject: [PATCH 5/5] example: print WireGuard parameters in nm-wg-set example script (cherry picked from commit e966f4b272331fcf99914ee2c1a980074a01ed94) --- examples/python/gi/nm-wg-set | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/examples/python/gi/nm-wg-set b/examples/python/gi/nm-wg-set index 81ee5667e4..5f284c2c25 100755 --- a/examples/python/gi/nm-wg-set +++ b/examples/python/gi/nm-wg-set @@ -234,6 +234,15 @@ def secret_to_string(secret): return '' return secret +def val_to_str(val): + if val == NM.Ternary.DEFAULT: + return 'default' + if val == NM.Ternary.TRUE: + return 'true' + if val == NM.Ternary.FALSE: + return 'false' + return repr(val) + ############################################################################### def wg_read_private_key(privkey_file): @@ -273,6 +282,9 @@ def do_get(nm_client, connection): print('private-key-flags: %s' % (secret_flags_to_string(s_wg.get_private_key_flags()))) print('listen-port: %s' % (s_wg.get_listen_port())) print('fwmark: 0x%x' % (s_wg.get_fwmark())) + print('peer-routes: %s' % (val_to_str(s_wg.get_peer_routes()))) + print('ip4-auto-default-route: %s' % (val_to_str(s_wg.get_ip4_auto_default_route()))) + print('ip6-auto-default-route: %s' % (val_to_str(s_wg.get_ip6_auto_default_route()))) for i in range(s_wg.get_peers_len()): peer = s_wg.get_peer(i) print('peer[%d].public-key: %s' % (i, peer.get_public_key()))