From e8b86f8445cd621c21ccf87833f4c49c74c325d9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2020 10:35:16 +0200 Subject: [PATCH 1/4] core: add NMIPConfigFlags for NMIPConfig flags This will be useful to set future options on the NMIPConfig. Yes, the code duplication of NMIP[46]Config is horrible. Needs to be unified in the future. --- src/nm-ip4-config.c | 23 +++++++++++++++++++++++ src/nm-ip4-config.h | 19 +++++++++++++++++++ src/nm-ip6-config.c | 23 +++++++++++++++++++++++ src/nm-ip6-config.h | 5 +++++ 4 files changed, 70 insertions(+) diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index ae637aeedc..8c16d0bfae 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -303,6 +303,7 @@ typedef struct { NMIPConfigDedupMultiIdxType idx_ip4_routes_; NMDedupMultiIdxType idx_ip4_routes; }; + NMIPConfigFlags config_flags; } NMIP4ConfigPrivate; struct _NMIP4Config { @@ -2659,6 +2660,28 @@ nm_ip4_config_llmnr_set (NMIP4Config *self, /*****************************************************************************/ +NMIPConfigFlags +nm_ip4_config_get_config_flags (const NMIP4Config *self) +{ + return NM_IP4_CONFIG_GET_PRIVATE (self)->config_flags; +} + +void +nm_ip4_config_set_config_flags (NMIP4Config *self, NMIPConfigFlags flags, NMIPConfigFlags mask) +{ + NMIP4ConfigPrivate *priv = NM_IP4_CONFIG_GET_PRIVATE (self); + + if (mask == 0) { + /* for convenience, accept 0 mask to set any flags. */ + mask = flags; + } + + nm_assert (!NM_FLAGS_ANY (flags, ~mask)); + priv->config_flags = (flags & mask) | (priv->config_flags & ~mask); +} + +/*****************************************************************************/ + void nm_ip4_config_set_dns_priority (NMIP4Config *self, int priority) { diff --git a/src/nm-ip4-config.h b/src/nm-ip4-config.h index 01a42e5464..b2845da3bb 100644 --- a/src/nm-ip4-config.h +++ b/src/nm-ip4-config.h @@ -15,6 +15,10 @@ /*****************************************************************************/ +typedef enum _NMIPConfigFlags { + NM_IP_CONFIG_FLAG_NONE = 0, +} NMIPConfigFlags; + typedef struct { NMDedupMultiIdxType parent; NMPObjectType obj_type; @@ -196,6 +200,9 @@ NMSettingConnectionLlmnr nm_ip4_config_llmnr_get (const NMIP4Config *self); void nm_ip4_config_llmnr_set (NMIP4Config *self, NMSettingConnectionLlmnr llmnr); +void nm_ip4_config_set_config_flags (NMIP4Config *self, NMIPConfigFlags flags, NMIPConfigFlags mask); +NMIPConfigFlags nm_ip4_config_get_config_flags (const NMIP4Config *self); + const NMDedupMultiHeadEntry *nm_ip4_config_lookup_addresses (const NMIP4Config *self); void nm_ip4_config_reset_addresses (NMIP4Config *self); void nm_ip4_config_add_address (NMIP4Config *self, const NMPlatformIP4Address *address); @@ -513,6 +520,18 @@ nm_ip_config_best_default_route_get (const NMIPConfig *self) _NM_IP_CONFIG_DISPATCH (self, nm_ip4_config_best_default_route_get, nm_ip6_config_best_default_route_get); } +static inline NMIPConfigFlags +nm_ip_config_get_config_flags (const NMIPConfig *self) +{ + _NM_IP_CONFIG_DISPATCH (self, nm_ip4_config_get_config_flags, nm_ip6_config_get_config_flags); +} + +static inline void +nm_ip_config_set_config_flags (NMIPConfig *self, NMIPConfigFlags flags, NMIPConfigFlags mask) +{ + _NM_IP_CONFIG_DISPATCH_VOID (self, nm_ip4_config_set_config_flags, nm_ip6_config_set_config_flags, flags, mask); +} + #define _NM_IP_CONFIG_DISPATCH_SET_OP(_return, dst, src, v4_func, v6_func, ...) \ G_STMT_START { \ gpointer _dst = (dst); \ diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 5acb3932d0..3f2c465bda 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -63,6 +63,7 @@ typedef struct { NMIPConfigDedupMultiIdxType idx_ip6_routes_; NMDedupMultiIdxType idx_ip6_routes; }; + NMIPConfigFlags config_flags; bool ipv6_disabled; } NMIP6ConfigPrivate; @@ -2296,6 +2297,28 @@ nm_ip6_config_get_dns_option (const NMIP6Config *self, guint i) /*****************************************************************************/ +NMIPConfigFlags +nm_ip6_config_get_config_flags (const NMIP6Config *self) +{ + return NM_IP6_CONFIG_GET_PRIVATE (self)->config_flags; +} + +void +nm_ip6_config_set_config_flags (NMIP6Config *self, NMIPConfigFlags flags, NMIPConfigFlags mask) +{ + NMIP6ConfigPrivate *priv = NM_IP6_CONFIG_GET_PRIVATE (self); + + if (mask == 0) { + /* for convenience, accept 0 mask to set any flags. */ + mask = flags; + } + + nm_assert (!NM_FLAGS_ANY (flags, ~mask)); + priv->config_flags = (flags & mask) | (priv->config_flags & ~mask); +} + +/*****************************************************************************/ + void nm_ip6_config_set_dns_priority (NMIP6Config *self, int priority) { diff --git a/src/nm-ip6-config.h b/src/nm-ip6-config.h index a9fa8f14a0..36e8518a86 100644 --- a/src/nm-ip6-config.h +++ b/src/nm-ip6-config.h @@ -128,6 +128,11 @@ gboolean nm_ip6_config_replace (NMIP6Config *dst, const NMIP6Config *src, gboole const NMPObject *nm_ip6_config_best_default_route_get (const NMIP6Config *self); const NMPObject *_nm_ip6_config_best_default_route_find (const NMIP6Config *self); +enum _NMIPConfigFlags; + +void nm_ip6_config_set_config_flags (NMIP6Config *self, enum _NMIPConfigFlags flags, enum _NMIPConfigFlags mask); +enum _NMIPConfigFlags nm_ip6_config_get_config_flags (const NMIP6Config *self); + const NMDedupMultiHeadEntry *nm_ip6_config_lookup_addresses (const NMIP6Config *self); void nm_ip6_config_reset_addresses (NMIP6Config *self); void nm_ip6_config_add_address (NMIP6Config *self, const NMPlatformIP6Address *address); From 5da82ee3eab29fc716b4fcf616c2ae89da748c4c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2020 10:57:27 +0200 Subject: [PATCH 2/4] wireguard: suppress automatic "wireguard.peer-routes" for default routes if "ipv[46].never-default" is enabled Enabling both peer-routes and never-default conflicts with having AllowedIPs set to a default route. Let never-default win. --- clients/common/settings-docs.h.in | 2 +- libnm-core/nm-setting-wireguard.c | 6 +++++- src/devices/nm-device-wireguard.c | 8 ++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 9425ef370d..3b8e235963 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -390,7 +390,7 @@ #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_IP6_AUTO_DEFAULT_ROUTE N_("Like ip4-auto-default-route, but for the IPv6 default route.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_LISTEN_PORT N_("The listen-port. If listen-port is not specified, the port will be chosen randomly when the interface comes up.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_MTU N_("If non-zero, only transmit packets of the specified size or smaller, breaking larger packets up into multiple fragments. If zero a default MTU is used. Note that contrary to wg-quick's MTU setting, this does not take into account the current routes at the time of activation.") -#define DESCRIBE_DOC_NM_SETTING_WIREGUARD_PEER_ROUTES N_("Whether to automatically add routes for the AllowedIPs ranges of the peers. If TRUE (the default), NetworkManager will automatically add routes in the routing tables according to ipv4.route-table and ipv6.route-table. If FALSE, no such routes are added automatically. In this case, the user may want to configure static routes in ipv4.routes and ipv6.routes, respectively.") +#define DESCRIBE_DOC_NM_SETTING_WIREGUARD_PEER_ROUTES N_("Whether to automatically add routes for the AllowedIPs ranges of the peers. If TRUE (the default), NetworkManager will automatically add routes in the routing tables according to ipv4.route-table and ipv6.route-table. Usually you want this automatism enabled. If FALSE, no such routes are added automatically. In this case, the user may want to configure static routes in ipv4.routes and ipv6.routes, respectively. Note that if the peer's AllowedIPs is \"0.0.0.0/0\" or \"::/0\" and the profile's ipv4.never-default or ipv6.never-default setting is enabled, the peer route for this peer won't be added automatically.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_PRIVATE_KEY N_("The 256 bit private-key in base64 encoding.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_PRIVATE_KEY_FLAGS N_("Flags indicating how to handle the \"private-key\" property.") #define DESCRIBE_DOC_NM_SETTING_WPAN_CHANNEL N_("IEEE 802.15.4 channel. A positive integer or -1, meaning \"do not set, use whatever the device is already set to\".") diff --git a/libnm-core/nm-setting-wireguard.c b/libnm-core/nm-setting-wireguard.c index ecfc5bb360..1f6940a898 100644 --- a/libnm-core/nm-setting-wireguard.c +++ b/libnm-core/nm-setting-wireguard.c @@ -2493,11 +2493,15 @@ nm_setting_wireguard_class_init (NMSettingWireGuardClass *klass) * Whether to automatically add routes for the AllowedIPs ranges * of the peers. If %TRUE (the default), NetworkManager will automatically * add routes in the routing tables according to ipv4.route-table and - * ipv6.route-table. + * ipv6.route-table. Usually you want this automatism enabled. * If %FALSE, no such routes are added automatically. In this case, the * user may want to configure static routes in ipv4.routes and ipv6.routes, * respectively. * + * Note that if the peer's AllowedIPs is "0.0.0.0/0" or "::/0" and the profile's + * ipv4.never-default or ipv6.never-default setting is enabled, the peer route for + * this peer won't be added automatically. + * * Since: 1.16 **/ obj_properties[PROP_PEER_ROUTES] = diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index bb7a595060..8dffb7f957 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1631,6 +1631,14 @@ _get_dev2_ip_config (NMDeviceWireGuard *self, if (prefix < 0) prefix = (addr_family == AF_INET) ? 32 : 128; + if (prefix == 0) { + NMSettingIPConfig *s_ip; + + s_ip = nm_connection_get_setting_ip_config (connection, addr_family); + if (nm_setting_ip_config_get_never_default (s_ip)) + continue; + } + if (!ip_config) ip_config = nm_device_ip_config_new (NM_DEVICE (self), addr_family); From 115291a46f52ee4adfe85264b9566d216bcb25e8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2020 10:46:17 +0200 Subject: [PATCH 3/4] wireguard: don't let explicit gateway override WireGuard's peer route The profile's "ipv4.gateway" and "ipv6.gateway" has only one real purpose: to define the next hop of a static default route. Usually, when specifying a gateway in this way, the default route from other addressing methods (like DHCPv4 or IPv6 autoconf) gets ignored. If you have a WireGuard peer with "AllowedIPs=0.0.0.0/0" and "wireguard.peer-routes" enabled, NetworkManager would automatically add a route to the peer. Previously, if the user also set a gateway, that route was suppressed. That doesn't feel right. Note that configuring a gateway on a WireGuard profile is likely to be wrong to begin with. At least, unless you take otherwise care to avoid routing loops. If you take care, setting a gateway may work, but it would feel clearer to instead just add an explicit /0 manual route instead. Also, note that usually you don't need a gateway anyway. WireGuard is a Layer 3 (IP) tunnel, where the next hop is alway just the other side of the tunnel. The next hop has little effect on the routes that you configure on a WireGuard interface. What however matters is whether a default route is present or not. Also, an explicit gateway probably works badly with "ipv[46].ip4-auto-default-route", because in that case the automatism should add a /0 peer-route route in a separate routing table. The explicit gateway interferes with that too. Nonetheless, without this patch it's not obvious why the /0 peer route gets suppressed when a gateway is set. Don't allow for that, and always add the peer-route. Probably the profile's gateway setting is still wrong and causes the profile not to work. But at least, you see all routes configured, and it's clearer where the (wrong) default route to the gateway comes from. --- clients/common/settings-docs.h.in | 2 +- libnm-core/nm-setting-wireguard.c | 14 +++++++++----- src/devices/nm-device-wireguard.c | 6 +++++- src/nm-ip4-config.c | 3 ++- src/nm-ip4-config.h | 6 +++++- src/nm-ip6-config.c | 3 ++- src/nm-types.h | 4 +++- 7 files changed, 27 insertions(+), 11 deletions(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 3b8e235963..def9d284c5 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -386,7 +386,7 @@ #define DESCRIBE_DOC_NM_SETTING_WIMAX_MAC_ADDRESS N_("If specified, this connection will only apply to the WiMAX device whose MAC address matches. This property does not change the MAC address of the device (known as MAC spoofing). Deprecated: 1") #define DESCRIBE_DOC_NM_SETTING_WIMAX_NETWORK_NAME N_("Network Service Provider (NSP) name of the WiMAX network this connection should use. Deprecated: 1") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_FWMARK N_("The use of fwmark is optional and is by default off. Setting it to 0 disables it. Otherwise it is a 32-bit fwmark for outgoing packets. Note that \"ip4-auto-default-route\" or \"ip6-auto-default-route\" enabled, implies to automatically choose a fwmark.") -#define DESCRIBE_DOC_NM_SETTING_WIREGUARD_IP4_AUTO_DEFAULT_ROUTE N_("Whether to enable special handling of the IPv4 default route. If enabled, the IPv4 default route will be placed to a dedicated routing-table and two policy routing rules will be added. The fwmark number is also used as routing-table for the default-route, and if fwmark is zero, a unused fwmark/table is chosen automatically. This corresponds to what wg-quick does with Table=auto. Leaving this at the default will enable this option automatically if ipv4.never-default is not set and there are any peers that use a default-route as allowed-ips.") +#define DESCRIBE_DOC_NM_SETTING_WIREGUARD_IP4_AUTO_DEFAULT_ROUTE N_("Whether to enable special handling of the IPv4 default route. If enabled, the IPv4 default route from wireguard.peer-routes will be placed to a dedicated routing-table and two policy routing rules will be added. The fwmark number is also used as routing-table for the default-route, and if fwmark is zero, an unused fwmark/table is chosen automatically. This corresponds to what wg-quick does with Table=auto and what WireGuard calls \"Improved Rule-based Routing\". Note that for this automatism to work, you usually don't want to set ipv4.gateway, because that will result in a conflicting default route. Leaving this at the default will enable this option automatically if ipv4.never-default is not set and there are any peers that use a default-route as allowed-ips.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_IP6_AUTO_DEFAULT_ROUTE N_("Like ip4-auto-default-route, but for the IPv6 default route.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_LISTEN_PORT N_("The listen-port. If listen-port is not specified, the port will be chosen randomly when the interface comes up.") #define DESCRIBE_DOC_NM_SETTING_WIREGUARD_MTU N_("If non-zero, only transmit packets of the specified size or smaller, breaking larger packets up into multiple fragments. If zero a default MTU is used. Note that contrary to wg-quick's MTU setting, this does not take into account the current routes at the time of activation.") diff --git a/libnm-core/nm-setting-wireguard.c b/libnm-core/nm-setting-wireguard.c index 1f6940a898..2bd633cecb 100644 --- a/libnm-core/nm-setting-wireguard.c +++ b/libnm-core/nm-setting-wireguard.c @@ -2534,11 +2534,15 @@ nm_setting_wireguard_class_init (NMSettingWireGuardClass *klass) * NMSettingWireGuard:ip4-auto-default-route: * * Whether to enable special handling of the IPv4 default route. - * If enabled, the IPv4 default route will be placed to a dedicated - * routing-table and two policy routing rules will be added. - * The fwmark number is also used as routing-table for the default-route, - * and if fwmark is zero, a unused fwmark/table is chosen automatically. - * This corresponds to what wg-quick does with Table=auto. + * If enabled, the IPv4 default route from wireguard.peer-routes + * will be placed to a dedicated routing-table and two policy routing rules + * will be added. The fwmark number is also used as routing-table for the default-route, + * and if fwmark is zero, an unused fwmark/table is chosen automatically. + * This corresponds to what wg-quick does with Table=auto and what WireGuard + * calls "Improved Rule-based Routing". + * + * Note that for this automatism to work, you usually don't want to set + * ipv4.gateway, because that will result in a conflicting default route. * * Leaving this at the default will enable this option automatically * if ipv4.never-default is not set and there are any peers that use diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index 8dffb7f957..9f01a76323 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1639,8 +1639,12 @@ _get_dev2_ip_config (NMDeviceWireGuard *self, continue; } - if (!ip_config) + if (!ip_config) { ip_config = nm_device_ip_config_new (NM_DEVICE (self), addr_family); + nm_ip_config_set_config_flags (ip_config, + NM_IP_CONFIG_FLAGS_IGNORE_MERGE_NO_DEFAULT_ROUTES, + 0); + } nm_utils_ipx_address_clear_host_address (addr_family, &addrbin, NULL, prefix); diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 8c16d0bfae..0feb7b2c3c 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -1203,7 +1203,8 @@ nm_ip4_config_merge (NMIP4Config *dst, nm_ip_config_iter_ip4_route_for_each (&ipconf_iter, src, &r_src) { if (NM_PLATFORM_IP_ROUTE_IS_DEFAULT (r_src)) { - if (NM_FLAGS_HAS (merge_flags, NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES)) + if ( NM_FLAGS_HAS (merge_flags, NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES) + && !NM_FLAGS_HAS (src_priv->config_flags, NM_IP_CONFIG_FLAGS_IGNORE_MERGE_NO_DEFAULT_ROUTES)) continue; if (default_route_metric_penalty) { NMPlatformIP4Route r = *r_src; diff --git a/src/nm-ip4-config.h b/src/nm-ip4-config.h index b2845da3bb..d4694d9360 100644 --- a/src/nm-ip4-config.h +++ b/src/nm-ip4-config.h @@ -16,7 +16,11 @@ /*****************************************************************************/ typedef enum _NMIPConfigFlags { - NM_IP_CONFIG_FLAG_NONE = 0, + NM_IP_CONFIG_FLAG_NONE = 0, + + /* if set, then the merge flag NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES gets + * ignored during merge. */ + NM_IP_CONFIG_FLAGS_IGNORE_MERGE_NO_DEFAULT_ROUTES = (1ull << 0), } NMIPConfigFlags; typedef struct { diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 3f2c465bda..b60e8057cb 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -876,7 +876,8 @@ nm_ip6_config_merge (NMIP6Config *dst, nm_ip_config_iter_ip6_route_for_each (&ipconf_iter, src, &r_src) { if (NM_PLATFORM_IP_ROUTE_IS_DEFAULT (r_src)) { - if (NM_FLAGS_HAS (merge_flags, NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES)) + if ( NM_FLAGS_HAS (merge_flags, NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES) + && !NM_FLAGS_HAS (src_priv->config_flags, NM_IP_CONFIG_FLAGS_IGNORE_MERGE_NO_DEFAULT_ROUTES)) continue; if (default_route_metric_penalty) { NMPlatformIP6Route r = *r_src; diff --git a/src/nm-types.h b/src/nm-types.h index 4db9a19b13..d2f6fb4093 100644 --- a/src/nm-types.h +++ b/src/nm-types.h @@ -233,7 +233,9 @@ typedef enum { * NMIPConfigMergeFlags: * @NM_IP_CONFIG_MERGE_DEFAULT: no flags set * @NM_IP_CONFIG_MERGE_NO_ROUTES: don't merge routes - * @NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES: don't merge default routes + * @NM_IP_CONFIG_MERGE_NO_DEFAULT_ROUTES: don't merge default routes. + * Note that if the source IP config has NM_IP_CONFIG_FLAGS_IGNORE_MERGE_NO_DEFAULT_ROUTES + * set, this flag gets ignored during merge. * @NM_IP_CONFIG_MERGE_NO_DNS: don't merge DNS information * @NM_IP_CONFIG_MERGE_EXTERNAL: mark new addresses as external */ From a873c438a8de799e01b710fd5f2179764494d0e7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Apr 2020 11:32:48 +0200 Subject: [PATCH 4/4] NEWS: update --- NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS b/NEWS index 0006ed1b30..d83eeb74c4 100644 --- a/NEWS +++ b/NEWS @@ -46,6 +46,9 @@ Overview of changes since NetworkManager-1.22 * Fixed multiple issues in the internal "nettools" DHCP client. * Export NM_CAPABILITY_OVS capability on D-Bus and in libnm to indicate that the OVS plugin is loaded. +* Fixes for importing WireGuard profiles in nmcli and better handle + configurations that enable ip4-auto-default-route with an explicit + gateway. * Various bug fixes and improvements. =============================================