From 0160834ea644f8ab42a8b04e238e53ea26276d6c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 8 Mar 2017 14:55:30 +0100 Subject: [PATCH 1/6] pacrunner: remove @domains from private struct The domain list is not a property of the global pacrunner instance and can be stored in a local variable. (cherry picked from commit 10f68543690b7b7e67304763ca71ce7534af1391) --- src/nm-pacrunner-manager.c | 46 +++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/src/nm-pacrunner-manager.c b/src/nm-pacrunner-manager.c index 450cf0e3ee..b9e0a2c308 100644 --- a/src/nm-pacrunner-manager.c +++ b/src/nm-pacrunner-manager.c @@ -41,7 +41,6 @@ struct remove_data { typedef struct { char *iface; - GPtrArray *domains; GDBusProxy *pacrunner; GCancellable *pacrunner_cancellable; GList *args; @@ -84,7 +83,7 @@ remove_data_destroy (struct remove_data *data) } static void -add_proxy_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, const NMProxyConfig *proxy_config) +add_proxy_config (GVariantBuilder *proxy_data, const NMProxyConfig *proxy_config) { const char *pac_url, *pac_script; NMProxyConfigMethod method; @@ -113,19 +112,18 @@ add_proxy_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, const N } static void -add_ip4_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, NMIP4Config *ip4) +get_ip4_domains (GPtrArray *domains, NMIP4Config *ip4) { - NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self); + char *cidr; int i; - char *cidr = NULL; /* Extract searches */ for (i = 0; i < nm_ip4_config_get_num_searches (ip4); i++) - g_ptr_array_add (priv->domains, g_strdup (nm_ip4_config_get_search (ip4, i))); + g_ptr_array_add (domains, g_strdup (nm_ip4_config_get_search (ip4, i))); /* Extract domains */ for (i = 0; i < nm_ip4_config_get_num_domains (ip4); i++) - g_ptr_array_add (priv->domains, g_strdup (nm_ip4_config_get_domain (ip4, i))); + g_ptr_array_add (domains, g_strdup (nm_ip4_config_get_domain (ip4, i))); /* Add addresses and routes in CIDR form */ for (i = 0; i < nm_ip4_config_get_num_addresses (ip4); i++) { @@ -134,8 +132,7 @@ add_ip4_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, NMIP4Conf cidr = g_strdup_printf ("%s/%u", nm_utils_inet4_ntop (address->address, NULL), address->plen); - g_ptr_array_add (priv->domains, g_strdup (cidr)); - g_free (cidr); + g_ptr_array_add (domains, cidr); } for (i = 0; i < nm_ip4_config_get_num_routes (ip4); i++) { @@ -144,25 +141,23 @@ add_ip4_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, NMIP4Conf cidr = g_strdup_printf ("%s/%u", nm_utils_inet4_ntop (routes->network, NULL), routes->plen); - g_ptr_array_add (priv->domains, g_strdup (cidr)); - g_free (cidr); + g_ptr_array_add (domains, cidr); } } static void -add_ip6_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, NMIP6Config *ip6) +get_ip6_domains (GPtrArray *domains, NMIP6Config *ip6) { - NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self); + char *cidr; int i; - char *cidr = NULL; /* Extract searches */ for (i = 0; i < nm_ip6_config_get_num_searches (ip6); i++) - g_ptr_array_add (priv->domains, g_strdup (nm_ip6_config_get_search (ip6, i))); + g_ptr_array_add (domains, g_strdup (nm_ip6_config_get_search (ip6, i))); /* Extract domains */ for (i = 0; i < nm_ip6_config_get_num_domains (ip6); i++) - g_ptr_array_add (priv->domains, g_strdup (nm_ip6_config_get_domain (ip6, i))); + g_ptr_array_add (domains, g_strdup (nm_ip6_config_get_domain (ip6, i))); /* Add addresses and routes in CIDR form */ for (i = 0; i < nm_ip6_config_get_num_addresses (ip6); i++) { @@ -171,8 +166,7 @@ add_ip6_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, NMIP6Conf cidr = g_strdup_printf ("%s/%u", nm_utils_inet6_ntop (&address->address, NULL), address->plen); - g_ptr_array_add (priv->domains, g_strdup (cidr)); - g_free (cidr); + g_ptr_array_add (domains, cidr); } for (i = 0; i < nm_ip6_config_get_num_routes (ip6); i++) { @@ -181,8 +175,7 @@ add_ip6_config (NMPacrunnerManager *self, GVariantBuilder *proxy_data, NMIP6Conf cidr = g_strdup_printf ("%s/%u", nm_utils_inet6_ntop (&routes->network, NULL), routes->plen); - g_ptr_array_add (priv->domains, g_strdup (cidr)); - g_free (cidr); + g_ptr_array_add (domains, cidr); } } @@ -312,6 +305,7 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self, NMPacrunnerManagerPrivate *priv; GVariantBuilder proxy_data; GVariant *pacrunner_manager_args; + GPtrArray *domains; g_return_if_fail (NM_IS_PACRUNNER_MANAGER (self)); g_return_if_fail (proxy_config); @@ -343,18 +337,18 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self, g_variant_new_string ("direct")); } - priv->domains = g_ptr_array_new_with_free_func (g_free); + domains = g_ptr_array_new_with_free_func (g_free); /* Extract stuff from configs */ - add_proxy_config (self, &proxy_data, proxy_config); + add_proxy_config (&proxy_data, proxy_config); if (ip4_config) - add_ip4_config (self, &proxy_data, ip4_config); + get_ip4_domains (domains, ip4_config); if (ip6_config) - add_ip6_config (self, &proxy_data, ip6_config); + get_ip6_domains (domains, ip6_config); - g_ptr_array_add (priv->domains, NULL); - strv = (char **) g_ptr_array_free (priv->domains, (priv->domains->len == 1)); + g_ptr_array_add (domains, NULL); + strv = (char **) g_ptr_array_free (domains, (domains->len == 1)); if (strv) { g_variant_builder_add (&proxy_data, "{sv}", From e895beb0da38fc87ce93fe7403a6b50e92f0dd82 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 8 Mar 2017 14:56:17 +0100 Subject: [PATCH 2/6] pacrunner: rework processing of configuration entries Fix some issues in nm-pacrunner-manager.c: - when adding a configuration through nm_pacrunner_manager_send(), we kept an association between the interface name and the pacrunner configuration object path, so that the configuration for that interface could be removed later. Unfortunately not all configurations have an interface associated, so we need a more generic way to identify configurations. Introduce a new @tag argument that serves as key to match configurations - the interface name of the last pushed configuration was stored in the manager private config and reused later; this could cause issues when there are multiple outstanding D-Bus calls. The interface is not needed anymore after the previous point. - remove() didn't actually remove the configuration from the list (cherry picked from commit 3ad89223d0ea9a772b650842d15583d92cf9a904) --- src/devices/nm-device.c | 2 + src/nm-pacrunner-manager.c | 224 +++++++++++++++++++++++------------- src/nm-pacrunner-manager.h | 3 +- src/vpn/nm-vpn-connection.c | 13 ++- 4 files changed, 156 insertions(+), 86 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index cf3d7525ac..26bd632bca 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8844,6 +8844,7 @@ reactivate_proxy_config (NMDevice *self) nm_device_set_proxy_config (self, priv->dhcp4.pac_url); nm_pacrunner_manager_send (priv->pacrunner_manager, + nm_device_get_ip_iface (self), nm_device_get_ip_iface (self), priv->proxy_config, priv->ip4_config, @@ -12536,6 +12537,7 @@ _set_state_full (NMDevice *self, if (priv->proxy_config) { nm_pacrunner_manager_send (priv->pacrunner_manager, + nm_device_get_ip_iface (self), nm_device_get_ip_iface (self), priv->proxy_config, priv->ip4_config, diff --git a/src/nm-pacrunner-manager.c b/src/nm-pacrunner-manager.c index b9e0a2c308..d3e89c6369 100644 --- a/src/nm-pacrunner-manager.c +++ b/src/nm-pacrunner-manager.c @@ -28,23 +28,28 @@ #include "nm-ip4-config.h" #include "nm-ip6-config.h" +static void pacrunner_remove_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data); + #define PACRUNNER_DBUS_SERVICE "org.pacrunner" #define PACRUNNER_DBUS_INTERFACE "org.pacrunner.Manager" #define PACRUNNER_DBUS_PATH "/org/pacrunner/manager" /*****************************************************************************/ -struct remove_data { - char *iface; +typedef struct { + char *tag; + NMPacrunnerManager *manager; + GVariant *args; char *path; -}; + guint refcount; + bool removed; +} Config; typedef struct { char *iface; GDBusProxy *pacrunner; GCancellable *pacrunner_cancellable; - GList *args; - GList *remove; + GList *configs; } NMPacrunnerManagerPrivate; struct _NMPacrunnerManager { @@ -71,15 +76,42 @@ NM_DEFINE_SINGLETON_GETTER (NMPacrunnerManager, nm_pacrunner_manager_get, NM_TYP /*****************************************************************************/ -static void -remove_data_destroy (struct remove_data *data) +static Config * +config_new (NMPacrunnerManager *manager, char *tag, GVariant *args) { - g_return_if_fail (data != NULL); + Config *config; - g_free (data->iface); - g_free (data->path); - memset (data, 0, sizeof (struct remove_data)); - g_free (data); + config = g_slice_new0 (Config); + config->manager = manager; + config->tag = tag; + config->args = g_variant_ref_sink (args); + config->refcount = 1; + + return config; +} + +static void +config_ref (Config *config) +{ + g_assert (config); + g_assert (config->refcount > 0); + + config->refcount++; +} + +static void +config_unref (Config *config) +{ + g_assert (config); + g_assert (config->refcount > 0); + + if (config->refcount == 1) { + g_free (config->tag); + g_variant_unref (config->args); + g_free (config->path); + g_slice_free (Config, config); + } else + config->refcount--; } static void @@ -180,63 +212,71 @@ get_ip6_domains (GPtrArray *domains, NMIP6Config *ip6) } static void -pacrunner_send_done (GObject *source, GAsyncResult *res, gpointer user_data) +pacrunner_send_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data) { - NMPacrunnerManager *self = NM_PACRUNNER_MANAGER (user_data); - NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self); + Config *config = user_data; + NMPacrunnerManager *self; + NMPacrunnerManagerPrivate *priv; gs_free_error GError *error = NULL; gs_unref_variant GVariant *variant = NULL; const char *path = NULL; - GList *iter = NULL; - gboolean found = FALSE; - variant = g_dbus_proxy_call_finish (priv->pacrunner, res, &error); + g_return_if_fail (!config->path); + + variant = g_dbus_proxy_call_finish (proxy, res, &error); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { + config_unref (config); + return; + } + + self = NM_PACRUNNER_MANAGER (config->manager); + priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self); + if (!variant) { - _LOGD ("sending proxy config to pacrunner failed: %s", error->message); + _LOGD ("send config for '%s' failed: %s", config->tag, error->message); } else { - struct remove_data *data; g_variant_get (variant, "(&o)", &path); - /* Replace the old path (if any) of proxy config with the new one returned - * from CreateProxyConfiguration() DBus method on pacrunner. - */ - for (iter = g_list_first (priv->remove); iter; iter = g_list_next (iter)) { - struct remove_data *r = iter->data; - if (g_strcmp0 (priv->iface, r->iface) == 0) { - g_free (r->path); - r->path = g_strdup (path); - found = TRUE; - break; - } - } + config->path = g_strdup (path); + _LOGD ("successfully sent config for '%s'", config->tag); - if (!found) { - data = g_malloc0 (sizeof (struct remove_data)); - data->iface = g_strdup (priv->iface); - data->path = g_strdup (path); - priv->remove = g_list_append (priv->remove, data); - _LOGD ("proxy config sent to pacrunner"); + if (config->removed) { + g_dbus_proxy_call (priv->pacrunner, + "DestroyProxyConfiguration", + g_variant_new ("(o)", config->path), + G_DBUS_CALL_FLAGS_NO_AUTO_START, + -1, + priv->pacrunner_cancellable, + (GAsyncReadyCallback) pacrunner_remove_done, + config); } } + config_unref (config); } static void -send_pacrunner_proxy_data (NMPacrunnerManager *self, GVariant *pacrunner_manager_args) +pacrunner_send_config (NMPacrunnerManager *self, Config *config) { NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self); - if (!pacrunner_manager_args) - return; + if (priv->pacrunner) { + gs_free char *args_str = NULL; + + _LOGT ("sending proxy config for '%s': %s", config->tag, + (args_str = g_variant_print (config->args, FALSE))); + + config_ref (config); + g_clear_pointer (&config->path, g_free); - if (priv->pacrunner) g_dbus_proxy_call (priv->pacrunner, "CreateProxyConfiguration", - pacrunner_manager_args, - G_DBUS_CALL_FLAGS_NONE, + config->args, + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, - NULL, - (GAsyncReadyCallback) pacrunner_send_done, - self); + priv->pacrunner_cancellable, + (GAsyncReadyCallback) pacrunner_send_done, + config); + } } static void @@ -252,9 +292,8 @@ name_owner_changed (GObject *object, owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (object)); if (owner) { _LOGD ("pacrunner appeared as %s", owner); - for (iter = g_list_first(priv->args); iter; iter = g_list_next(iter)) { - send_pacrunner_proxy_data (self, iter->data); - } + for (iter = g_list_first (priv->configs); iter; iter = g_list_next (iter)) + pacrunner_send_config (self, iter->data); } else { _LOGD ("pacrunner disappeared"); } @@ -289,6 +328,7 @@ pacrunner_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data) * nm_pacrunner_manager_send: * @self: the #NMPacrunnerManager * @iface: the iface for the connection or %NULL + * @tag: unique configuration identifier * @proxy_config: proxy config of the connection * @ip4_config: IP4 config of the connection * @ip6_config: IP6 config of the connection @@ -296,6 +336,7 @@ pacrunner_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data) void nm_pacrunner_manager_send (NMPacrunnerManager *self, const char *iface, + const char *tag, NMProxyConfig *proxy_config, NMIP4Config *ip4_config, NMIP6Config *ip6_config) @@ -304,8 +345,8 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self, NMProxyConfigMethod method; NMPacrunnerManagerPrivate *priv; GVariantBuilder proxy_data; - GVariant *pacrunner_manager_args; GPtrArray *domains; + Config *config; g_return_if_fail (NM_IS_PACRUNNER_MANAGER (self)); g_return_if_fail (proxy_config); @@ -357,31 +398,39 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self, g_strfreev (strv); } - pacrunner_manager_args = g_variant_ref_sink (g_variant_new ("(a{sv})", &proxy_data)); - priv->args = g_list_append (priv->args, pacrunner_manager_args); + config = config_new (self, g_strdup (tag), + g_variant_new ("(a{sv})", &proxy_data)); + priv->configs = g_list_append (priv->configs, config); - /* Send if pacrunner is available on Bus, otherwise - * argument has already been appended above to be + /* Send if pacrunner is available on bus, otherwise + * config has already been appended above to be * sent when pacrunner appears. */ - send_pacrunner_proxy_data (self, pacrunner_manager_args); + pacrunner_send_config (self, config); } static void -pacrunner_remove_done (GObject *source, GAsyncResult *res, gpointer user_data) +pacrunner_remove_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data) { - /* @self may be a dangling pointer. However, we don't use it as the - * logging macro below does not dereference @self. */ - NMPacrunnerManager *self = user_data; + Config *config = user_data; + NMPacrunnerManager *self; gs_free_error GError *error = NULL; gs_unref_variant GVariant *ret = NULL; - ret = g_dbus_proxy_call_finish ((GDBusProxy *) source, res, &error); + ret = g_dbus_proxy_call_finish (proxy, res, &error); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { + config_unref (config); + return; + } + + self = NM_PACRUNNER_MANAGER (config->manager); if (!ret) - _LOGD ("Couldn't remove proxy config from pacrunner: %s", error->message); + _LOGD ("couldn't remove config for '%s': %s", config->tag, error->message); else - _LOGD ("Successfully removed proxy config from pacrunner"); + _LOGD ("successfully removed config for '%s'", config->tag); + + config_unref (config); } /** @@ -391,26 +440,44 @@ pacrunner_remove_done (GObject *source, GAsyncResult *res, gpointer user_data) * from pacrunner */ void -nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *iface) +nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *tag) { NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self); GList *list; - for (list = g_list_first(priv->remove); list; list = g_list_next(list)) { - struct remove_data *data = list->data; - if (g_strcmp0 (data->iface, iface) == 0) { - if (priv->pacrunner && data->path) + g_return_if_fail (tag); + + _LOGT ("removing config for '%s'", tag); + + for (list = g_list_first (priv->configs); list; list = g_list_next (list)) { + Config *config = list->data; + + if (nm_streq (config->tag, tag)) { + if (priv->pacrunner) { + if (!config->path) { + /* send() is pending: mark the config as removed + * so that the send() callback will remove it when + * the D-Bus path is known. */ + config->removed = TRUE; + config_unref (config); + return; + } g_dbus_proxy_call (priv->pacrunner, "DestroyProxyConfiguration", - g_variant_new ("(o)", data->path), - G_DBUS_CALL_FLAGS_NONE, + g_variant_new ("(o)", config->path), + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, - NULL, - (GAsyncReadyCallback) pacrunner_remove_done, - self); - break; + priv->pacrunner_cancellable, + (GAsyncReadyCallback) pacrunner_remove_done, + config); + } else + config_unref (config); + priv->configs = g_list_delete_link (priv->configs, list); + return; } } + /* bug, remove() should always match a previous send() for a given tag */ + g_return_if_reached (); } /*****************************************************************************/ @@ -439,16 +506,11 @@ dispose (GObject *object) NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE ((NMPacrunnerManager *) object); g_clear_pointer (&priv->iface, g_free); - nm_clear_g_cancellable (&priv->pacrunner_cancellable); - g_clear_object (&priv->pacrunner); - g_list_free_full (priv->args, (GDestroyNotify) g_variant_unref); - priv->args = NULL; - - g_list_free_full (priv->remove, (GDestroyNotify) remove_data_destroy); - priv->remove = NULL; + g_list_free_full (priv->configs, (GDestroyNotify) config_unref); + priv->configs = NULL; G_OBJECT_CLASS (nm_pacrunner_manager_parent_class)->dispose (object); } diff --git a/src/nm-pacrunner-manager.h b/src/nm-pacrunner-manager.h index 99e8511580..4f6ad15857 100644 --- a/src/nm-pacrunner-manager.h +++ b/src/nm-pacrunner-manager.h @@ -36,10 +36,11 @@ NMPacrunnerManager *nm_pacrunner_manager_get (void); void nm_pacrunner_manager_send (NMPacrunnerManager *self, const char *iface, + const char *tag, NMProxyConfig *proxy_config, NMIP4Config *ip4_config, NMIP6Config *ip6_config); -void nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *iface); +void nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *tag); #endif /* __NETWORKMANAGER_PACRUNNER_MANAGER_H__ */ diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index add26830a6..17dd9f4a2e 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -481,6 +481,7 @@ _set_vpn_state (NMVpnConnection *self, VpnState old_vpn_state; NMVpnConnectionState new_external_state, old_external_state; NMDevice *parent_dev = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (self)); + NMConnection *applied; g_return_if_fail (NM_IS_VPN_CONNECTION (self)); @@ -552,13 +553,15 @@ _set_vpn_state (NMVpnConnection *self, } break; case STATE_ACTIVATED: + applied = _get_applied_connection (self); + /* Secrets no longer needed now that we're connected */ nm_active_connection_clear_secrets (NM_ACTIVE_CONNECTION (self)); /* Let dispatcher scripts know we're up and running */ nm_dispatcher_call_vpn (NM_DISPATCHER_ACTION_VPN_UP, _get_settings_connection (self, FALSE), - _get_applied_connection (self), + applied, parent_dev, priv->ip_iface, priv->proxy_config, @@ -571,16 +574,18 @@ _set_vpn_state (NMVpnConnection *self, if (priv->proxy_config) { nm_pacrunner_manager_send (nm_pacrunner_manager_get (), priv->ip_iface, + nm_connection_get_uuid (applied), priv->proxy_config, priv->ip4_config, priv->ip6_config); } break; case STATE_DEACTIVATING: + applied = _get_applied_connection (self); if (quitting) { nm_dispatcher_call_vpn_sync (NM_DISPATCHER_ACTION_VPN_PRE_DOWN, _get_settings_connection (self, FALSE), - _get_applied_connection (self), + applied, parent_dev, priv->ip_iface, priv->proxy_config, @@ -589,7 +594,7 @@ _set_vpn_state (NMVpnConnection *self, } else { if (!nm_dispatcher_call_vpn (NM_DISPATCHER_ACTION_VPN_PRE_DOWN, _get_settings_connection (self, FALSE), - _get_applied_connection (self), + applied, parent_dev, priv->ip_iface, priv->proxy_config, @@ -604,7 +609,7 @@ _set_vpn_state (NMVpnConnection *self, } /* Remove config from PacRunner */ - nm_pacrunner_manager_remove (nm_pacrunner_manager_get(), priv->ip_iface); + nm_pacrunner_manager_remove (nm_pacrunner_manager_get(), nm_connection_get_uuid (applied)); break; case STATE_FAILED: case STATE_DISCONNECTED: From e4b323100e8736a034fa04290ee1125e6a897352 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 9 Mar 2017 14:45:45 +0100 Subject: [PATCH 3/6] pacrunner: don't log pacrunner-manager address It's a singleton, the address is not meaningful. (cherry picked from commit 752e906aa46c3d3317947f093fa66b00144e1ff6) --- src/nm-pacrunner-manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nm-pacrunner-manager.c b/src/nm-pacrunner-manager.c index d3e89c6369..a9114bab20 100644 --- a/src/nm-pacrunner-manager.c +++ b/src/nm-pacrunner-manager.c @@ -72,7 +72,7 @@ NM_DEFINE_SINGLETON_GETTER (NMPacrunnerManager, nm_pacrunner_manager_get, NM_TYP /*****************************************************************************/ #define _NMLOG_DOMAIN LOGD_PROXY -#define _NMLOG(level, ...) __NMLOG_DEFAULT_WITH_ADDR (level, _NMLOG_DOMAIN, "pacrunner", __VA_ARGS__) +#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "pacrunner", __VA_ARGS__) /*****************************************************************************/ @@ -291,11 +291,11 @@ name_owner_changed (GObject *object, owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (object)); if (owner) { - _LOGD ("pacrunner appeared as %s", owner); + _LOGD ("name owner appeared (%s)", owner); for (iter = g_list_first (priv->configs); iter; iter = g_list_next (iter)) pacrunner_send_config (self, iter->data); } else { - _LOGD ("pacrunner disappeared"); + _LOGD ("name owner disappeared"); } } From 1fe0b781fd7acbdc56b54ba66c82c9659846a22d Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 4 Apr 2017 11:12:53 +0200 Subject: [PATCH 4/6] pacrunner: specify domains only for VPNs If a VPN provides a proxy, we want to restrict the usage of that proxy to URLs in the VPN domain. For all other connections, the proxy should be used for all domains. (cherry picked from commit b1395522558cde434868a47b43ae67b73a455db7) --- src/devices/nm-device.c | 8 ++++---- src/nm-pacrunner-manager.c | 31 +++++++++++++++++-------------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 26bd632bca..9168907b12 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8847,8 +8847,8 @@ reactivate_proxy_config (NMDevice *self) nm_device_get_ip_iface (self), nm_device_get_ip_iface (self), priv->proxy_config, - priv->ip4_config, - priv->ip6_config); + NULL, + NULL); } static gboolean @@ -12540,8 +12540,8 @@ _set_state_full (NMDevice *self, nm_device_get_ip_iface (self), nm_device_get_ip_iface (self), priv->proxy_config, - priv->ip4_config, - priv->ip6_config); + NULL, + NULL); } break; case NM_DEVICE_STATE_FAILED: diff --git a/src/nm-pacrunner-manager.c b/src/nm-pacrunner-manager.c index a9114bab20..88d82602ef 100644 --- a/src/nm-pacrunner-manager.c +++ b/src/nm-pacrunner-manager.c @@ -330,8 +330,8 @@ pacrunner_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data) * @iface: the iface for the connection or %NULL * @tag: unique configuration identifier * @proxy_config: proxy config of the connection - * @ip4_config: IP4 config of the connection - * @ip6_config: IP6 config of the connection + * @ip4_config: IP4 config of the connection to extract domain info from + * @ip6_config: IP6 config of the connection to extract domain info from */ void nm_pacrunner_manager_send (NMPacrunnerManager *self, @@ -378,24 +378,27 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self, g_variant_new_string ("direct")); } - domains = g_ptr_array_new_with_free_func (g_free); /* Extract stuff from configs */ add_proxy_config (&proxy_data, proxy_config); - if (ip4_config) - get_ip4_domains (domains, ip4_config); - if (ip6_config) - get_ip6_domains (domains, ip6_config); + if (ip4_config || ip6_config) { + domains = g_ptr_array_new_with_free_func (g_free); - g_ptr_array_add (domains, NULL); - strv = (char **) g_ptr_array_free (domains, (domains->len == 1)); + if (ip4_config) + get_ip4_domains (domains, ip4_config); + if (ip6_config) + get_ip6_domains (domains, ip6_config); - if (strv) { - g_variant_builder_add (&proxy_data, "{sv}", - "Domains", - g_variant_new_strv ((const char *const *) strv, -1)); - g_strfreev (strv); + g_ptr_array_add (domains, NULL); + strv = (char **) g_ptr_array_free (domains, (domains->len == 1)); + + if (strv) { + g_variant_builder_add (&proxy_data, "{sv}", + "Domains", + g_variant_new_strv ((const char *const *) strv, -1)); + g_strfreev (strv); + } } config = config_new (self, g_strdup (tag), From 0dead63886c49a7ab62e69cf08e4d7521dac9a02 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 7 Apr 2017 10:23:22 +0200 Subject: [PATCH 5/6] device: fix removal of pacrunner configurations Don't try to remove the configuration if we haven't added it in the first place, for example when the connection gets deactivated before it completes or for slave connections without IP configuration. Fixes: 3ad89223d0ea9a772b650842d15583d92cf9a904 (cherry picked from commit 3cada7722d41551ff24f3b03e991eb6cfa2d953d) --- src/devices/nm-device.c | 12 ++++++++++-- src/vpn/nm-vpn-connection.c | 9 +++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9168907b12..0f08f7ce60 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -322,6 +322,7 @@ typedef struct _NMDevicePrivate { /* Proxy Configuration */ NMProxyConfig *proxy_config; NMPacrunnerManager *pacrunner_manager; + bool proxy_config_sent; /* IP4 configuration info */ NMIP4Config * ip4_config; /* Combined config from VPN, settings, and device */ @@ -8839,6 +8840,9 @@ reactivate_proxy_config (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + if (!priv->proxy_config_sent) + return; + nm_pacrunner_manager_remove (priv->pacrunner_manager, nm_device_get_ip_iface (self)); @@ -12509,8 +12513,11 @@ _set_state_full (NMDevice *self, } } - /* Remove config from PacRunner */ - nm_pacrunner_manager_remove (priv->pacrunner_manager, nm_device_get_ip_iface (self)); + if (priv->proxy_config_sent) { + nm_pacrunner_manager_remove (priv->pacrunner_manager, + nm_device_get_ip_iface (self)); + priv->proxy_config_sent = FALSE; + } break; case NM_DEVICE_STATE_DISCONNECTED: if ( priv->queued_act_request @@ -12542,6 +12549,7 @@ _set_state_full (NMDevice *self, priv->proxy_config, NULL, NULL); + priv->proxy_config_sent = TRUE; } break; case NM_DEVICE_STATE_FAILED: diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index 17dd9f4a2e..464395dffa 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -127,6 +127,7 @@ typedef struct { GVariant *connect_hash; guint connect_timeout; NMProxyConfig *proxy_config; + gboolean proxy_config_sent; gboolean has_ip4; NMIP4Config *ip4_config; guint32 ip4_internal_gw; @@ -578,6 +579,7 @@ _set_vpn_state (NMVpnConnection *self, priv->proxy_config, priv->ip4_config, priv->ip6_config); + priv->proxy_config_sent = TRUE; } break; case STATE_DEACTIVATING: @@ -608,8 +610,11 @@ _set_vpn_state (NMVpnConnection *self, } } - /* Remove config from PacRunner */ - nm_pacrunner_manager_remove (nm_pacrunner_manager_get(), nm_connection_get_uuid (applied)); + if (priv->proxy_config_sent) { + nm_pacrunner_manager_remove (nm_pacrunner_manager_get(), + nm_connection_get_uuid (applied)); + priv->proxy_config_sent = FALSE; + } break; case STATE_FAILED: case STATE_DISCONNECTED: From c6f2173f107403aa22fd0ce898a5ea15bc4c6158 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 7 Apr 2017 14:12:56 +0200 Subject: [PATCH 6/6] pacrunner: remove failed and pending items from configuration list If a configuration does not have a path it is because we are still sending it to pacrunner or because we failed to do so. In both cases, we have to remove the configuration from the list. Fixes: 3ad89223d0ea9a772b650842d15583d92cf9a904 (cherry picked from commit fad2cf072143762d9f51c9d79c05f5ec995b099f) --- src/nm-pacrunner-manager.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/nm-pacrunner-manager.c b/src/nm-pacrunner-manager.c index 88d82602ef..d4a5b5ab9a 100644 --- a/src/nm-pacrunner-manager.c +++ b/src/nm-pacrunner-manager.c @@ -458,21 +458,22 @@ nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *tag) if (nm_streq (config->tag, tag)) { if (priv->pacrunner) { if (!config->path) { - /* send() is pending: mark the config as removed - * so that the send() callback will remove it when - * the D-Bus path is known. */ + /* send() failed or is still pending. Mark the item as + * removed, so that we ask pacrunner to drop it when the + * send() completes. + */ config->removed = TRUE; config_unref (config); - return; + } else { + g_dbus_proxy_call (priv->pacrunner, + "DestroyProxyConfiguration", + g_variant_new ("(o)", config->path), + G_DBUS_CALL_FLAGS_NO_AUTO_START, + -1, + priv->pacrunner_cancellable, + (GAsyncReadyCallback) pacrunner_remove_done, + config); } - g_dbus_proxy_call (priv->pacrunner, - "DestroyProxyConfiguration", - g_variant_new ("(o)", config->path), - G_DBUS_CALL_FLAGS_NO_AUTO_START, - -1, - priv->pacrunner_cancellable, - (GAsyncReadyCallback) pacrunner_remove_done, - config); } else config_unref (config); priv->configs = g_list_delete_link (priv->configs, list);