Closed
Bug 924395
Opened 10 years ago
Closed 10 years ago
Gnome proxy settings, address conversion to IPv6 for "ignore hosts" doesn't convert netmask
Categories
(Toolkit :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: cs_gon, Assigned: cs_gon)
References
Details
Attachments
(1 file, 1 obsolete file)
1.27 KB,
patch
|
karlt
:
review+
|
Details | Diff | Splinter Review |
When entering URLs with IP adresses in Firefox, those IP addresses are not matched correctly to the "ignore hosts" setting, when having configured Gnome proxy settings and using "Use system proxy settings" in Firefox, because the IP addresses get converted to IPv6 but not the netmask/prefix. Steps to reproduce: Use a Firefox with Gnome support. Configure proxy settings in Gnome like in the following example (change the proxy host/port to something sensible): gsettings set org.gnome.system.proxy mode 'manual' gsettings set org.gnome.system.proxy.http enabled true gsettings set org.gnome.system.proxy.http host 'example.com' gsettings set org.gnome.system.proxy.http port 8080 gsettings set org.gnome.system.proxy.ftp host 'example.com' gsettings set org.gnome.system.proxy.ftp port 8080 gsettings set org.gnome.system.proxy.https host 'example.com' gsettings set org.gnome.system.proxy.https port 8080 gsettings set org.gnome.system.proxy ignore-hosts "['localhost', '127.0.0.0/8']" After that, choose "Use system proxy settings" in Firefox, so Firefox uses the Gnome proxy settings. When entering URLs containing hostnames, Firefox uses the proxy as intended. But when using any IP addresses in URLs that are not in the 127.0.0.0/8 network, Firefox doesn't use the proxy, but tries to connect directly to those IP addresses. The expected behavior would be, that Firefox ignores the proxy only for addresses in the 127.0.0.0/8 network, not for all addresses. When removing the entry "127.0.0.0/8" from the ignore-hosts, Firefox again uses the proxy for all URLs containing IP addresses, as expected. Looking in the code in toolkit/system/unixproxy/nsUnixSystemProxySettings.cpp in function HostIgnoredByProxy, the IP addresses specified in the ignore-hosts and the URL get converted to IPv6, the netmask gets applied, and the results are then compared. The problem is however, that the conversion to IPv6 doesn't include the netmask. So the wrong netmask (IPv4 netmask) get's applied to the IPv6 addresses, and when comparing the addresses this causes a match for any IPv4 address. This problem can easily be verified when changing the netmask/prefix in the ignore-hosts setting to a IPv6 prefix (IPv4 prefix + 96). So when changing the entry to "127.0.0.0/104" it works as expected, in the sense that only connections for URLs containing IP addresses of the 127.0.0.0/8 network are done directly, connections for URLs containing other IP adresses are done through the proxy. I have attached a patch against mozilla-central to fix the problem. The code converts the IPv4 prefixes to IPv6 prefixes (adds 96) before the netmask get applied. This is along the lines of other proxy code in netwerk/base/src/nsProtocolProxyService.cpp. I have tested the attached patch and it fixes the problem for me. I have read the information about submitting bugs, and patches on developer.mozilla.org, but since this is my first time, and I'm not 100% sure if everything applies to me, or how exactly, it would be nice to point me in the right direction, if I can do something to help get the patch included. Thanks!
Attachment #814387 -
Flags: review?(benjamin)
Updated•10 years ago
|
Attachment #814387 -
Flags: review?(benjamin) → review?(karlt)
Comment 1•10 years ago
|
||
Comment on attachment 814387 [details] [diff] [review] gnome_proxy_netmask_ipv6_conversion.patch Thanks for the nice diagnosis, Carsten. I'm not familiar with details of ipv6 masks, but, looking at proxy_MaskIPv6Addr, it looks like ipv6 masks of <= 32 are meaningful, and so the conversion should be performed only for ipv4 addresses, as in nsProtocolProxyService. http://hg.mozilla.org/mozilla-central/annotate/64b497e6f593/netwerk/base/src/nsProtocolProxyService.cpp#l1362 Perhaps add an optional int32_t* aMask parameter to ConvertToIPV6Addr()?
Attachment #814387 -
Flags: review?(karlt) → review-
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Thanks for the feedback! And you are right, the code in the first patch could pose a problem when IPv6 network are configured in the ignore-hosts list. I should probably have paid more attention to the code in nsProtocolProxyService.cpp. I attached a new patch, which does the conversion in ConvertToIPV6Addr(), and should only be done for IPv4 addresses, as it is done in nsProtocolProxyService.cpp. I wonder though, why the code in nsProtocolProxyService.cpp just skips over the netmask conversion, when an invalid (>32) netmask is found. If a user specifies an invalid IPv4 address/netmask pair, like "127.0.0.1/33", it leads to a similar problem as with the Gnome proxy code, and suddenly every IPv4 Address in an URL produces a match against this (invalid) ignore-hosts entry. I verified this behavior by adding an "127.0.0.1/33" entry in the internal proxy settings in Firefox. I think a better way would be to skip those invalid ignore-hosts entries. So in the new attached patch the conversion will return false if the netmask is invalid, so the entry gets skipped.
Assignee | ||
Updated•10 years ago
|
Attachment #817058 -
Flags: review?(karlt)
Updated•10 years ago
|
Attachment #817058 -
Flags: review?(karlt) → review+
Updated•10 years ago
|
Assignee: nobody → cs_gon
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Attachment #814387 -
Attachment is obsolete: true
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b936f0735ad8
Keywords: checkin-needed
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b936f0735ad8
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
See Also: → https://launchpad.net/bugs/1226537
You need to log in
before you can comment on or make changes to this bug.
Description
•