From c8e6f3e5fb9cd8dd80b8a7278338d0e9064be97f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 7 Sep 2017 10:22:10 +0200 Subject: [PATCH] vpn: fix ifindex of parent IP config in apply_parent_device_config() When creating the NMIP4Config/NMIP6Config instance, we must always use the right ifindex. That is the ifindex, on which we want to apply the config. It also means, that for device-based VPNs (those with priv->ip_ifindex set, like OpenVPN), the parent's config must have the ip-ifindex of the parent device. Not of the VPN's device. One effect of this bug is that in add_ip4_vpn_gateway_route() we resolve the route to the external gateway and only accept it if it's on the parent device. But since the ifindex of the config was wrong, we would accept route on the wrong interface. https://bugzilla.gnome.org/show_bug.cgi?id=787370 --- src/devices/nm-device.c | 10 +++++++++ src/vpn/nm-vpn-connection.c | 41 +++++++++++++++---------------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 8001e74b08..55ad6c3237 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -10038,6 +10038,11 @@ nm_device_replace_vpn4_config (NMDevice *self, NMIP4Config *old, NMIP4Config *co { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + nm_assert (!old || NM_IS_IP4_CONFIG (old)); + nm_assert (!config || NM_IS_IP4_CONFIG (config)); + nm_assert (!old || nm_ip4_config_get_ifindex (old) == nm_device_get_ip_ifindex (self)); + nm_assert (!config || nm_ip4_config_get_ifindex (config) == nm_device_get_ip_ifindex (self)); + if (!_replace_vpn_config_in_list (&priv->vpn4_configs, (GObject *) old, (GObject *) config)) return; @@ -10167,6 +10172,11 @@ nm_device_replace_vpn6_config (NMDevice *self, NMIP6Config *old, NMIP6Config *co { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + nm_assert (!old || NM_IS_IP6_CONFIG (old)); + nm_assert (!config || NM_IS_IP6_CONFIG (config)); + nm_assert (!old || nm_ip6_config_get_ifindex (old) == nm_device_get_ip_ifindex (self)); + nm_assert (!config || nm_ip6_config_get_ifindex (config) == nm_device_get_ip_ifindex (self)); + if (!_replace_vpn_config_in_list (&priv->vpn6_configs, (GObject *) old, (GObject *) config)) return; diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index 11eb9a4952..1a858060f0 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -1075,40 +1075,31 @@ apply_parent_device_config (NMVpnConnection *self) { NMVpnConnectionPrivate *priv = NM_VPN_CONNECTION_GET_PRIVATE (self); NMDevice *parent_dev = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (self)); + int ifindex; NMIP4Config *vpn4_parent_config = NULL; NMIP6Config *vpn6_parent_config = NULL; - if (priv->ip_ifindex > 0) { - if (priv->ip4_config) { - vpn4_parent_config = nm_ip4_config_new (nm_netns_get_multi_idx (priv->netns), - priv->ip_ifindex); - } - if (priv->ip6_config) { - vpn6_parent_config = nm_ip6_config_new (nm_netns_get_multi_idx (priv->netns), - priv->ip_ifindex); - } - } else { - int ifindex; - + ifindex = nm_device_get_ip_ifindex (parent_dev); + if (ifindex > 0) { /* If the VPN didn't return a network interface, it is a route-based * VPN (like kernel IPSec) and all IP addressing and routing should * be done on the parent interface instead. */ - - /* Also clear the gateway. We don't configure the gateway as part of the - * vpn-config. Instead we tell NMDefaultRouteManager directly about the - * default route. */ - ifindex = nm_device_get_ip_ifindex (parent_dev); - if (ifindex > 0) { - if (priv->ip4_config) { - vpn4_parent_config = nm_ip4_config_new (nm_netns_get_multi_idx (priv->netns), - ifindex); + if (priv->ip4_config) { + vpn4_parent_config = nm_ip4_config_new (nm_netns_get_multi_idx (priv->netns), + ifindex); + if (priv->ip_ifindex <= 0) nm_ip4_config_merge (vpn4_parent_config, priv->ip4_config, NM_IP_CONFIG_MERGE_NO_DNS); - } - if (priv->ip6_config) { - vpn6_parent_config = nm_ip6_config_new (nm_netns_get_multi_idx (priv->netns), - ifindex); + } + if (priv->ip6_config) { + vpn6_parent_config = nm_ip6_config_new (nm_netns_get_multi_idx (priv->netns), + ifindex); + if (priv->ip_ifindex <= 0) { nm_ip6_config_merge (vpn6_parent_config, priv->ip6_config, NM_IP_CONFIG_MERGE_NO_DNS); + + /* Also clear the gateway. We don't configure the gateway as part of the + * vpn-config. Instead we tell NMDefaultRouteManager directly about the + * default route. */ nm_ip6_config_set_gateway (vpn6_parent_config, NULL); } }