Re: [PATCH] Improve WPAD proxy handling
On Wed, 2013-06-12 at 15:26 +0100, David Woodhouse wrote:
We already request WPAD information from the DHCP server (bug 368423)
but we don't do anything useful with it at all.
Well, except provide it to dispatcher clients. This was sort of waiting
on more coherent proxy support including connection-specific user
overrides; see TODO.
The same information can also come from a VPN server, but we don't even
*collect* that.
This patch fixes the latter, and exposes the information in a coherent
fashion. There's no need at this point to overcomplicate matters by
pretending to support complicated manual proxy setups (as discussed in
TODO); this is just for what we can glean automatically, which is *only*
PAC files.
Tested with a local DHCP server that tells me where the proxy is, and
with a slightly modified nm-openconnect-service-openconnect-helper[sic].
This is based largely on the existing handling of NIS domainname.
Which isn't a great example, unfortunately.
I see that it's considered acceptable to (ab)use nm-dns-manager to
configure the NIS domain. I'll look at making it prod the PacRunner
Only grudgingly.
dæmon with proxy information too, but this part stands alone.
I think instead of stuffing into the IPv4 config we should probably have
per-interface proxy config instead, like we have ip4_config and
ip6_config. Anyone else have comments on that?
There are any number of proxy options, and I don't think they really are
IPv4 or IPv6 specific, are they? I mean, whether you get a PAC file
from DHCPv4 or DHCPv6 or VPNv4 or VPNv6 doesn't really make a
difference, it's just a URL. And I have no idea how a client would make
a decision on which PAC URL to use if they had more than one?
Dan
diff --git a/include/NetworkManagerVPN.h b/include/NetworkManagerVPN.h
index 6e654b3..9b6e5f7 100644
--- a/include/NetworkManagerVPN.h
+++ b/include/NetworkManagerVPN.h
@@ -176,6 +176,9 @@ typedef enum {
/* boolean: prevent this VPN connection from ever getting the default route
*/
#define NM_VPN_PLUGIN_IP4_CONFIG_NEVER_DEFAULT never-default
+/* string: Location of PAC file for HTTP proxy */
+#define NM_VPN_PLUGIN_IP4_CONFIG_PROXY_PAC proxy-pac
+
/* Deprecated */
#define NM_VPN_PLUGIN_IP4_CONFIG_GATEWAY
NM_VPN_PLUGIN_IP4_CONFIG_EXT_GATEWAY
diff --git a/src/dhcp-manager/nm-dhcp-client.c
b/src/dhcp-manager/nm-dhcp-client.c
index f4e94a6..ebe9a69 100644
--- a/src/dhcp-manager/nm-dhcp-client.c
+++ b/src/dhcp-manager/nm-dhcp-client.c
@@ -1292,6 +1292,12 @@ ip4_options_to_config (NMDHCPClient *self)
nm_ip4_config_set_nis_domain (ip4_config, str);
}
+ str = g_hash_table_lookup (priv-options, new_wpad);
+ if (str) {
+ nm_log_info (LOGD_DHCP4, Proxy PAC '%s', str);
+ nm_ip4_config_set_proxy_pac (ip4_config, str);
+ }
+
str = g_hash_table_lookup (priv-options, new_nis_servers);
if (str) {
char **searches = g_strsplit (str, , 0);
diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c
index 0722981..8f86996 100644
--- a/src/nm-ip4-config.c
+++ b/src/nm-ip4-config.c
@@ -59,6 +59,8 @@ typedef struct {
GArray *nis;
char * nis_domain;
+ char * proxy_pac;
+
GSList *routes;
gboolean never_default;
@@ -569,6 +571,26 @@ nm_ip4_config_get_nis_domain (NMIP4Config *config)
return NM_IP4_CONFIG_GET_PRIVATE (config)-nis_domain;
}
+void
+nm_ip4_config_set_proxy_pac (NMIP4Config *config, const char *uri)
+{
+ NMIP4ConfigPrivate *priv;
+
+ g_return_if_fail (NM_IS_IP4_CONFIG (config));
+
+ priv = NM_IP4_CONFIG_GET_PRIVATE (config);
+ g_free (priv-proxy_pac);
+ priv-proxy_pac = g_strdup (uri);
+}
+
+const char *
+nm_ip4_config_get_proxy_pac (NMIP4Config *config)
+{
+ g_return_val_if_fail (NM_IS_IP4_CONFIG (config), 0);
+
+ return NM_IP4_CONFIG_GET_PRIVATE (config)-proxy_pac;
+}
+
/* libnl convenience/conversion functions */
static int ip4_addr_to_rtnl_local (guint32 ip4_address, struct rtnl_addr
*addr)
@@ -790,6 +812,10 @@ nm_ip4_config_diff (NMIP4Config *a, NMIP4Config *b)
(g_strcmp0 (a_priv-nis_domain, b_priv-nis_domain) != 0))
flags |= NM_IP4_COMPARE_FLAG_NIS_DOMAIN;
+ if ( (a_priv-proxy_pac || b_priv-proxy_pac)
+ (g_strcmp0 (a_priv-proxy_pac, b_priv-proxy_pac) != 0))
+ flags |= NM_IP4_COMPARE_FLAG_PROXY_PAC;
+
if ( !route_slist_compare (a_priv-routes, b_priv-routes)
|| !route_slist_compare (b_priv-routes, a_priv-routes))
flags |= NM_IP4_COMPARE_FLAG_ROUTES;
@@ -856,6 +882,10 @@ nm_ip4_config_hash (NMIP4Config *config, GChecksum *sum,
gboolean dns_only)
s = nm_ip4_config_get_nis_domain (config);
if (s)
g_checksum_update (sum, (const guint8 *) s, strlen (s));
+
+ s = nm_ip4_config_get_proxy_pac (config);
Re: [PATCH] Improve WPAD proxy handling
On Thu, 2013-06-13 at 20:42 +0100, David Woodhouse wrote: On Thu, 2013-06-13 at 13:30 -0500, Dan Williams wrote: On Wed, 2013-06-12 at 15:26 +0100, David Woodhouse wrote: We already request WPAD information from the DHCP server (bug 368423) but we don't do anything useful with it at all. Well, except provide it to dispatcher clients. Yeah, but you provide an irrelevant $DHCP_WPAD to dispatcher clients even when invoking them for a *VPN* connection that doesn't even use DHCP at all, let alone have a proxy configuration of its own. Has anyone ever actually tried to *use* that? Because the parent device config is passed to the dispatcher here, which pulls out the DHCP options. Given that VPN connections don't currently have DHCP capability, I'd expect clients to ignore that information. For proxy configuration, we'd define new PROXY_ env variables that contain the merged proxy configuration from various sources, including DHCP and VPN. I think instead of stuffing into the IPv4 config we should probably have per-interface proxy config instead, like we have ip4_config and ip6_config. Anyone else have comments on that? You are preaching to the choir. The NIS domain doesn't live in the IP4 config, and neither does the DNS search domain (or nameservers). They should *all* live somewhere generic. Yeah, it was basically added because SUSE's netconfig conflates all that crap together and doesn't have a better way of doing it, plus almost nobody (hopefully) uses NIS anymore so it's pretty harmless. It likely shouldn't have been there, but I didn't really want to rearchitect a whole bunch of stuff just for NIS. There are any number of proxy options, and I don't think they really are IPv4 or IPv6 specific, are they? I mean, whether you get a PAC file from DHCPv4 or DHCPv6 or VPNv4 or VPNv6 doesn't really make a difference, it's just a URL. Right. Just like the search domain. And the list of nameservers. In each case you may want to trust or distrust certain information according to whether it comes from DHCPv4 or DHCPv6 or RA or VPN server or wpad.$searchdomain or wherever. But that isn't fundamentally a Legacy IP vs. IPv6 distinction and it's wrong to make it one as we have historically done. Yeah, except that it's *delivered* via those mechanisms, and who knows whether they talk to each other. I know we argued about this on IRC and I think we mostly agree; they are global *after* NM receives them and combines them with user overrides, but their sources are clearly stack-specific and users may want/need to override them that way. But yes they are obviously global when written out to the system, like proxy information is. And I have no idea how a client would make a decision on which PAC URL to use if they had more than one? For now my dispatcher script (bug #701824) just punts that question, on the basis that if you have more than one, it's probably because you have split-tunnel VPN. And if you have split-tunnel VPN, it's probably done *specifically* to avoid having to use the damn corporate proxies for real Internet access, so just having no proxy should be fine. Hmm, not sure we can assume that. I'd rather do use first one. Split tunnel doesn't have anything to do with it though, right? It's just if your corporate VPN stuff gives you a proxy, and your local DHCP stuff gives you a proxy, then you really have no idea what to use. You might be at a location that requires a proxy for access to most resources, but you've got a VPN connection that also requires a proxy for other stuff. So if you do run split tunnel, anything !VPN would require the DHCP-provided proxy, while anything VPN may or may not. Seems pretty likely nothing can handle this situation unless pacrunner can reliably give an answer for how do I get here using a number of domains and proxy settings. But in the fullness of time, the plan is to handle this just like we do dnsmasq configuration for multiple connections. For each proxy configuration (be it PAC script or manual setup), we give PacRunner a list of domains/IP ranges for which that configuration applies. Yeah, that sounds like the right approach. It needs implementing in PacRunner but it's not that hard, as PacRunner already *has* the concept of multiple configs and a 'domains' list for each one. It just doesn't *check* that list yet. But still, step one is actually making the information available in a coherent fashion at all. If you want me to have a go at a patch set which moves NIS and DNS information into the *generic* connection config, and base this patch on top of that, then I can try. But it's probably better for someone who actually knows the codebase to do that, rather than a stray monkey like me. Well, lets start with something like an internal NMProxyConfig just like we have NMIPxConfig; export that over the bus just like the other two, and stuff the PAC URL into it. We can add
Re: [PATCH] Improve WPAD proxy handling
On Thu, 2013-06-13 at 15:27 -0500, Dan Williams wrote: Because the parent device config is passed to the dispatcher here, which pulls out the DHCP options. Given that VPN connections don't currently have DHCP capability, I'd expect clients to ignore that information. For proxy configuration, we'd define new PROXY_ env variables that contain the merged proxy configuration from various sources, including DHCP and VPN. OK. In each case you may want to trust or distrust certain information according to whether it comes from DHCPv4 or DHCPv6 or RA or VPN server or wpad.$searchdomain or wherever. But that isn't fundamentally a Legacy IP vs. IPv6 distinction and it's wrong to make it one as we have historically done. Yeah, except that it's *delivered* via those mechanisms, and who knows whether they talk to each other. I know we argued about this on IRC and I think we mostly agree; they are global *after* NM receives them and combines them with user overrides, but their sources are clearly stack-specific and users may want/need to override them that way. No. Their sources are *not* stack-specific. There is *no* reason to lump DHCPv6 along with RA under IPv6, while DHCPv4 is under IPv4 and what we get given from a VPN server is... well, who the hell knows? That *isn't* a meaningful classification. It *doesn't* make sense to say trust this information if I received it from an IPv6 source vs. trust this information if I received it from an IPv4 source. That *isn't* the interesting distinction between the sources. And it doesn't make sense to say add this search domain or this DNS server to the list if you happen to pick up an IPv6 address somehow. I can't think of any valid use case for that one either. You might say DO *NOT* use this nameserver if you happen to pick an IPv6 address somehow, if you have a known-broken but local and fast nameserver, so when you *are* stuck in the 20th century without IPv6, you want to use it anyway. But we don't support that use case anyway. We can only *add* nameservers when there's IPv6, not remove them. But yes they are obviously global when written out to the system, like proxy information is. And I have no idea how a client would make a decision on which PAC URL to use if they had more than one? For now my dispatcher script (bug #701824) just punts that question, on the basis that if you have more than one, it's probably because you have split-tunnel VPN. And if you have split-tunnel VPN, it's probably done *specifically* to avoid having to use the damn corporate proxies for real Internet access, so just having no proxy should be fine. Hmm, not sure we can assume that. I'd rather do use first one. Split tunnel doesn't have anything to do with it though, right? In practice, use first one is more likely to be wrong, for now, than bail and use none. It's not as if do the right thing with all available configs simultaneously is far off. But one thing at a time. Well, lets start with something like an internal NMProxyConfig just like we have NMIPxConfig; export that over the bus just like the other two, and stuff the PAC URL into it. We can add the other manual settings later if/when we get NMSettingProxy. Then you can populate ProxyConfig with the info from VPN and info from DNS, and eventually the dispatcher can use it too. How does that sound? I'll take a look. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] Improve WPAD proxy handling
On Thu, 2013-06-13 at 13:30 -0500, Dan Williams wrote: On Wed, 2013-06-12 at 15:26 +0100, David Woodhouse wrote: We already request WPAD information from the DHCP server (bug 368423) but we don't do anything useful with it at all. Well, except provide it to dispatcher clients. Yeah, but you provide an irrelevant $DHCP_WPAD to dispatcher clients even when invoking them for a *VPN* connection that doesn't even use DHCP at all, let alone have a proxy configuration of its own. Has anyone ever actually tried to *use* that? I think instead of stuffing into the IPv4 config we should probably have per-interface proxy config instead, like we have ip4_config and ip6_config. Anyone else have comments on that? You are preaching to the choir. The NIS domain doesn't live in the IP4 config, and neither does the DNS search domain (or nameservers). They should *all* live somewhere generic. There are any number of proxy options, and I don't think they really are IPv4 or IPv6 specific, are they? I mean, whether you get a PAC file from DHCPv4 or DHCPv6 or VPNv4 or VPNv6 doesn't really make a difference, it's just a URL. Right. Just like the search domain. And the list of nameservers. In each case you may want to trust or distrust certain information according to whether it comes from DHCPv4 or DHCPv6 or RA or VPN server or wpad.$searchdomain or wherever. But that isn't fundamentally a Legacy IP vs. IPv6 distinction and it's wrong to make it one as we have historically done. And I have no idea how a client would make a decision on which PAC URL to use if they had more than one? For now my dispatcher script (bug #701824) just punts that question, on the basis that if you have more than one, it's probably because you have split-tunnel VPN. And if you have split-tunnel VPN, it's probably done *specifically* to avoid having to use the damn corporate proxies for real Internet access, so just having no proxy should be fine. But in the fullness of time, the plan is to handle this just like we do dnsmasq configuration for multiple connections. For each proxy configuration (be it PAC script or manual setup), we give PacRunner a list of domains/IP ranges for which that configuration applies. It needs implementing in PacRunner but it's not that hard, as PacRunner already *has* the concept of multiple configs and a 'domains' list for each one. It just doesn't *check* that list yet. But still, step one is actually making the information available in a coherent fashion at all. If you want me to have a go at a patch set which moves NIS and DNS information into the *generic* connection config, and base this patch on top of that, then I can try. But it's probably better for someone who actually knows the codebase to do that, rather than a stray monkey like me. -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] Improve WPAD proxy handling
We already request WPAD information from the DHCP server (bug 368423)
but we don't do anything useful with it at all.
The same information can also come from a VPN server, but we don't even
*collect* that.
This patch fixes the latter, and exposes the information in a coherent
fashion. There's no need at this point to overcomplicate matters by
pretending to support complicated manual proxy setups (as discussed in
TODO); this is just for what we can glean automatically, which is *only*
PAC files.
Tested with a local DHCP server that tells me where the proxy is, and
with a slightly modified nm-openconnect-service-openconnect-helper[sic].
This is based largely on the existing handling of NIS domainname.
I see that it's considered acceptable to (ab)use nm-dns-manager to
configure the NIS domain. I'll look at making it prod the PacRunner
dæmon with proxy information too, but this part stands alone.
diff --git a/include/NetworkManagerVPN.h b/include/NetworkManagerVPN.h
index 6e654b3..9b6e5f7 100644
--- a/include/NetworkManagerVPN.h
+++ b/include/NetworkManagerVPN.h
@@ -176,6 +176,9 @@ typedef enum {
/* boolean: prevent this VPN connection from ever getting the default route */
#define NM_VPN_PLUGIN_IP4_CONFIG_NEVER_DEFAULT never-default
+/* string: Location of PAC file for HTTP proxy */
+#define NM_VPN_PLUGIN_IP4_CONFIG_PROXY_PAC proxy-pac
+
/* Deprecated */
#define NM_VPN_PLUGIN_IP4_CONFIG_GATEWAY NM_VPN_PLUGIN_IP4_CONFIG_EXT_GATEWAY
diff --git a/src/dhcp-manager/nm-dhcp-client.c
b/src/dhcp-manager/nm-dhcp-client.c
index f4e94a6..ebe9a69 100644
--- a/src/dhcp-manager/nm-dhcp-client.c
+++ b/src/dhcp-manager/nm-dhcp-client.c
@@ -1292,6 +1292,12 @@ ip4_options_to_config (NMDHCPClient *self)
nm_ip4_config_set_nis_domain (ip4_config, str);
}
+ str = g_hash_table_lookup (priv-options, new_wpad);
+ if (str) {
+ nm_log_info (LOGD_DHCP4, Proxy PAC '%s', str);
+ nm_ip4_config_set_proxy_pac (ip4_config, str);
+ }
+
str = g_hash_table_lookup (priv-options, new_nis_servers);
if (str) {
char **searches = g_strsplit (str, , 0);
diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c
index 0722981..8f86996 100644
--- a/src/nm-ip4-config.c
+++ b/src/nm-ip4-config.c
@@ -59,6 +59,8 @@ typedef struct {
GArray *nis;
char * nis_domain;
+ char * proxy_pac;
+
GSList *routes;
gboolean never_default;
@@ -569,6 +571,26 @@ nm_ip4_config_get_nis_domain (NMIP4Config *config)
return NM_IP4_CONFIG_GET_PRIVATE (config)-nis_domain;
}
+void
+nm_ip4_config_set_proxy_pac (NMIP4Config *config, const char *uri)
+{
+ NMIP4ConfigPrivate *priv;
+
+ g_return_if_fail (NM_IS_IP4_CONFIG (config));
+
+ priv = NM_IP4_CONFIG_GET_PRIVATE (config);
+ g_free (priv-proxy_pac);
+ priv-proxy_pac = g_strdup (uri);
+}
+
+const char *
+nm_ip4_config_get_proxy_pac (NMIP4Config *config)
+{
+ g_return_val_if_fail (NM_IS_IP4_CONFIG (config), 0);
+
+ return NM_IP4_CONFIG_GET_PRIVATE (config)-proxy_pac;
+}
+
/* libnl convenience/conversion functions */
static int ip4_addr_to_rtnl_local (guint32 ip4_address, struct rtnl_addr *addr)
@@ -790,6 +812,10 @@ nm_ip4_config_diff (NMIP4Config *a, NMIP4Config *b)
(g_strcmp0 (a_priv-nis_domain, b_priv-nis_domain) != 0))
flags |= NM_IP4_COMPARE_FLAG_NIS_DOMAIN;
+ if ( (a_priv-proxy_pac || b_priv-proxy_pac)
+(g_strcmp0 (a_priv-proxy_pac, b_priv-proxy_pac) != 0))
+ flags |= NM_IP4_COMPARE_FLAG_PROXY_PAC;
+
if ( !route_slist_compare (a_priv-routes, b_priv-routes)
|| !route_slist_compare (b_priv-routes, a_priv-routes))
flags |= NM_IP4_COMPARE_FLAG_ROUTES;
@@ -856,6 +882,10 @@ nm_ip4_config_hash (NMIP4Config *config, GChecksum *sum,
gboolean dns_only)
s = nm_ip4_config_get_nis_domain (config);
if (s)
g_checksum_update (sum, (const guint8 *) s, strlen (s));
+
+ s = nm_ip4_config_get_proxy_pac (config);
+ if (s)
+ g_checksum_update (sum, (const guint8 *) s, strlen (s));
}
for (i = 0; i nm_ip4_config_get_num_nameservers (config); i++)
@@ -900,6 +930,7 @@ finalize (GObject *object)
g_ptr_array_free (priv-searches, TRUE);
g_array_free (priv-nis, TRUE);
g_free (priv-nis_domain);
+ g_free (priv-proxy_pac);
G_OBJECT_CLASS (nm_ip4_config_parent_class)-finalize (object);
}
diff --git a/src/nm-ip4-config.h b/src/nm-ip4-config.h
index 5433768..c66aece 100644
--- a/src/nm-ip4-config.h
+++ b/src/nm-ip4-config.h
@@ -107,6 +107,9 @@ void nm_ip4_config_reset_nis_servers
(NMIP4Config *config);
void nm_ip4_config_set_nis_domain (NMIP4Config *config, const
char *domain);
const char * nm_ip4_config_get_nis_domain (NMIP4Config *config);
