Fennec does not respect Gecko-like networking configuration (ssl, proxy config, etc..) for favicons, search suggestions, or other background operations

NEW
Unassigned

Status

()

P5
normal
10 years ago
3 months ago

People

(Reporter: mozillazine, Unassigned)

Tracking

(Depends on: 4 bugs, Blocks: 2 bugs)

Trunk
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tor-mobile])

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729)

I am using manual proxy settings but when I visit web sites and check my network traffic logs (that shows only non-proxied traffic) I see that sites I visit are showing up with URL's to load favicon.ico, e.g. www.googlecom/favicon.ico.  No other traffic is showing up.   

Basically the code to load the favicons is not using the FireFox proxy settings.

Reproducible: Always
could be a dupe of bug 507578
Component: General → Networking
Product: Firefox → Core
QA Contact: general → networking
Version: unspecified → 1.9.1 Branch
(Reporter)

Comment 2

10 years ago
Please note that manual proxy configuration is being used, not auto config file

Comment 3

6 years ago
This bug still exists in the Android version, release 18.02.

This bug prevents correct use of anonymizing proxies.
This is WFM - so I will need a specific STR to proceed.

I setup a proxy on remotehost:3127 and began a tcpdump capturing all port 3127 traffic.

I set the proxy in the options screen manunally to be remotehost:3127 and clicked the "use this proxy for all protocol buttons".

I then loaded http://www.favicon.com/ (ha!).

I then stopped the capture and inspected it.

As expected, it contained GET http://favicon.com/favicon.ico

This is with firefox 21 (nightly)

Comment 5

6 years ago
Maybe there should be a new ticket for this -- looking at the history, it appears that this used to be an issue on Windows, but was then fixed.  It does not appear to happen on Windows or Linux in the 18.02, but it appears in Android consistently.
daniel - please confirm how you are configuring the proxy in firefox for android and provide a set of steps that reproduces it there. thanks!

Comment 7

6 years ago
Apologies but I don't know what STR means in this context...  Steps to reproduce?

Here is our setup:

Firefox 18.0.2 for Android (what it says on the "About" screen)
Downloaded from https://ftp.mozilla.org/pub/mozilla.org/mobile/releases/latest/android/multi/fennec-18.0.2.multi.android-arm.apk, dated February 6th.

Android version 2.3.4

We are running on a local testbed, so all the ipaddrs are internal to our testbed.

We set up the proxy by entering in the URL bar about:config, and then searching for proxy, and then configuring:

network.proxy.type = 1
network.proxy.socks_version = 4
network.proxy.socks_port = 5010
network.proxy.socks = 10.0.0.10

Then we run a tcpdump with packets traversing a router between the phone and the rest of the network.

The topology of the part of the network that we're tracing looks like this:

                proxynode
                10.0.0.10
                        |
                        |
phone -X- routernode
                        |
                        |
                    to the
                   internet


X marks where the traces are taken.

What we see in the traces, when visiting an arbitrary web site, is that all of the TCP traffic in and out of the phone is to/from 10.0.0.10, our proxy, except for a few stray connections from the phone to the web site.

Examining the contents of the packets in those connections, we can see that they are all "GET /favicon.ico" requests and responses.

Comment 8

6 years ago
(sorry my attempt at ASCII art did not work properly -- the proxynode is connected to the routernode, and the routernode is connected, via a NAT, to the internet.)

Comment 9

6 years ago
We have also used socks_version = 5; same effect.
Daniel - yes STR is steps to reproduce and that's an excellent STR. Thanks for that! Never in a million years would I have guessed you were going right to about:config and setting up a socks proxy :)

your report is going to get queued for a number of days behind other stuff I am looking at - but I'll get to it.

one last question - does this reproduce with normal http:// urls or does it necessarily involve https:// ?

Comment 11

6 years ago
We have tried this with both http and https URLs.  The https URLs provoke the same pattern (everything goes through the proxy except for one small request/response), but since they are TLS, we can't actually see whether it is a "GET /favicon.ico" or something else.  I'd suspect so.

(If it's useful, we could stand up our own web server, run the experiment against it, and then look at the logs, which will tell us exactly what was requested, but unfortunately it's Friday afternoon and people are starting to disappear for the weekend)
It turns out that favicon handling doesn't use gecko at all on android! doh!

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Favicons.java#294

I'm trying to figure out the drivers behind that.. because I think it causes some serious problems. A quick brainstorm:

1] privacy leaks for proxy users or fails for proxies configured in browser
2] add-ons won't see it - so privacy leaks again with add-ons that do things like clean up request headers or filter requests
3] private browsing rules are probably being broken - I betcha this is in the android disk cache
4] ssl policy mismatches between nss and firefox configs and whatever stock android is using and possibly the cert trust database
5] efficiency things.. this will require new connections (where favicon almost always reuses an idle one inside gecko) which require extra packets and battery.. and probly more significantly a idle reaping strategy out of sync with our own - which is more battery hit.

mxr doesn't show any other uses of AndroidHttpClient so I'm trying to figure out what's driving this one. Mark?
Component: Networking → General
Flags: needinfo?(mark.finkle)
Product: Core → Fennec
Version: 1.9.1 Branch → Trunk
Status: UNCONFIRMED → NEW
Component: General → General
Ever confirmed: true
Product: Fennec → Firefox for Android
Firefox on Android pulls down favicons in the Java part of the application, not just in the Gecko part. Favicons show up in many places of the native Android, and waiting for Gecko to handle the download can cause delays in updating the UI. We use native Java networking libraries to perform the download.

Firefox on Android should be capable of using the system Android proxy settings. Can you try making the proxy changes to Android and not just Firefox.

I am not sure how we could support Gecko overridden proxy settings in Java. We might be able to come up with something.
Flags: needinfo?(mark.finkle)
but this isn't just about proxies right? (ssl configs, private browsing, addons, cookie mgmt, etc..)

it seems wrong to be using a second stack on the side - too surprising for the user.. it took me a long time to figure out the mismatch here and at least I know there is a separate java and gecko layer - the end user doesn't stand a chance.
Note that we also bypass Gecko for search suggestions in the AwesomeScreen, so if we change this, we should change that as well.
(In reply to Mark Finkle (:mfinkle) from comment #13)
> Firefox on Android pulls down favicons in the Java part of the application,
> not just in the Gecko part. Favicons show up in many places of the native
> Android, and waiting for Gecko to handle the download can cause delays in
> updating the UI. We use native Java networking libraries to perform the
> download.

I understand the motivation here. However, it would be helpful to the networking team if you could share more about what kind of performance issues you found with using Necko for this. I personally am surprised that downloading favicons is a performance issue at all, and it definitely seems wasteful to create a whole new network connection (especially on SPDY-enabled sites) just to fetch it. It seems like we may very well be slowing down page load in return for having faster-loading favicons. Is that really the tradeoff we want to make?

> I am not sure how we could support Gecko overridden proxy settings in Java.
> We might be able to come up with something.

IMO, Gecko on Android should *only* use system proxy settings and shouldn't have its own proxy configuration at all.

(In reply to Brian Nicholson (:bnicholson) from comment #15)
> Note that we also bypass Gecko for search suggestions in the AwesomeScreen,
> so if we change this, we should change that as well.

Same thing. Something would have to be very wrong for this to be a net win for performance, regardless of the security/privacy implications. For Desktop Firefox, the networking team implemented special optimizations for the search box, which we could also implement for Android's search box, and which should cause the google search results page connection (which is HTTPS) to be preconnected, which should cause the search results to load faster. By bypassing Gecko, aren't we preventing the prefetching of DNS and/or preconnecting to https://google.com?

To be clear, I think that if Necko really is causing a performance problem then we shouldn't switch this stuff back over to Necko until we can be sure we are improving performance with the switch. I don't think the privacy/security issues with search suggestions and favicons are likely to be so bad as to warrant an hasty replacement of the code. But, whatever badness is in Necko or the Java<->Gecko interface to Necko should be understood and fixed ASAP so that such hacks are not needed.
Brisn - It's not really about Gecko/Necko being slower. It's about the UI being in Java, running in a separate thread and requiring all data to be marshaled back and forth using a string based messaging system.

If we used Necko to pull the favicons, we'd need to convert those to base64 text, pass them to the Java UI side and re-create the bitmaps. We also have some situations where the UI is up and running before Gecko... long before Gecko. This happens at startup and can be a >5 seconds.

Comment 18

6 years ago
(In reply to Brian Smith (:bsmith) from comment #16)
> (In reply to Mark Finkle (:mfinkle) from comment #13)
> ... I don't think the
> privacy/security issues with search suggestions and favicons are likely to
> be so bad as to warrant an hasty replacement of the code. 

Everything is relative, of course.  Many people don't care about privacy,
and quick hacks can lead to long-term maintenance headaches...

But people who depend on anonymity/privacy proxies simply cannot use fennec in its current state.  The non-proxied fetch of the favicon.ico tells any third party exactly what sites these people are accessing and when they access them.
(In reply to Daniel Ellard from comment #18)
> (In reply to Brian Smith (:bsmith) from comment #16)
> > (In reply to Mark Finkle (:mfinkle) from comment #13)
> > ... I don't think the
> > privacy/security issues with search suggestions and favicons are likely to
> > be so bad as to warrant an hasty replacement of the code. 
> 
> Everything is relative, of course.  Many people don't care about privacy,
> and quick hacks can lead to long-term maintenance headaches...
> 
> But people who depend on anonymity/privacy proxies simply cannot use fennec
> in its current state.  The non-proxied fetch of the favicon.ico tells any
> third party exactly what sites these people are accessing and when they
> access them.

Instead of making the proxy changes to Firefox directly, can you make the proxy settings to Android? Firefox will read the Android system settings and use them. That means both Android and Firefox would use the same proxy.
(In reply to Mark Finkle (:mfinkle) from comment #19)

> Instead of making the proxy changes to Firefox directly, can you make the
> proxy settings to Android? Firefox will read the Android system settings and
> use them. That means both Android and Firefox would use the same proxy.

is there an nsISystemProxySettings for Android? Bug 799841 says there isn't and that's a pretty recent bug (12/12).. but then again I thought I had seen a bug report somewhere along the line using one. not sure what the answer is..
(In reply to Patrick McManus [:mcmanus] from comment #20)
.
> 
> is there an nsISystemProxySettings for Android? Bug 799841 says there isn't

experimentally - I do see one. Great! (maybe someone with more surety can update 799841?)
Alternatively, this favicon bug can be fixed by bug 855911 where we do all the favicon loading in Gecko.
Depends on: 855911
OS: Windows XP → Android
Hardware: x86 → All
Morphing the title of this to be more accurate.

The network handling of a huge chunk of Firefox on Android -- including Sync, product announcements, UI favicon fetches, and more -- happen in pure-Android code. Much of this code runs when Gecko is not running.

These requests don't respect Gecko proxy settings, but they should respect OS proxy settings.

Tentatively, I think this bug is a WONTFIX, because proxy users should already be setting the OS proxy settings, and it's awkward to replicate these settings into Java every time Gecko runs. Furthermore, the only users who will have proxies set at all are those running a proxy-setting add-on, or manually futzing with about:config. But I accept that this is confusing and unexpected to people used to desktop Firefox.

When we offer a proxy configuration UI in Firefox on Android (I thought there was a bug filed for this, but I don't see one), it should be explicit in its handling of both sets of proxy settings. As such, let's leave this bug open as a marker for that.
Summary: Firefox does not honor proxy settings when loading favicon.ico files → Firefox does not honor proxy settings for favicons, search suggestions, or other background operations
Depends on: 942652
Depends on: 942653
Depends on: 942654
I'm going to update the title here. This also means fennec, when doing these operations, isn't implementing any networking security policies we have embedded into gecko (cipher suite ordering, the new TLS support, upcoming mandatory OCSP stapling) - there will probably even be some URIs that will work in one of java/gecko but not the other.
Summary: Firefox does not honor proxy settings for favicons, search suggestions, or other background operations → Fennec does not use gecko networking (ssl, proxy config, etc..) for favicons, search suggestions, or other background operations
This very much looks like something that needs doing, but loading favicons on the Gecko side when they're needed in Java for display might be problematic. It's probably worth avoiding use of the bitmaps->base64 encoding scheme that is used elsewhere for that sort of thing.
That said, if we avoid going through JS then that problem goes away - it seems like it would be possible (And really cool) to make JNI bindings from Java to the Gecko networking functions so Java can invoke these directly and get back byte arrays (Or other meaningful Java objects without the need to serialise). 

That would also seem like it would provide a very neat solution to all of those bugs that RNewman just filed, as well as providing a way to avoid creating new ones like it in the future.
(Don't want a repeat of the horros of Bug 896822)
I think it's a given that there are lots of places that loading Gecko and a profile to make a network request isn't feasible in Fennec, so I'm happy to draw a line through that option. 

If we somehow get a super-portable Necko library with Java bindings that we can use instead of our Apache and java.net code, then that might be a possibility, but it still wouldn't be totally equivalent (no profile, for example), so I don't see much point.
I'd also add a requirement:
* We MUST be able to display favicons (and other resources) during startup before Gecko has loaded.
* Responsiveness and memory usage MUST not be regressed.

These requirements could be met many different ways, since there are several scenarios for startup. Clean profile and dirty profile come to mind as general scenarios. The UI must be able to load as fast as possible without blowing memory usage. Whatever solution we come up with for this bug must meet the requirements.
(In reply to Mark Finkle (:mfinkle) from comment #28)

> * We MUST be able to display favicons (and other resources)...

If we add to that list the other stuff (syncing, fetching announcements, etc.), much of which occurs outside of a profile context or is memory-constrained, I'm just going to morph this back into something that's not a WONTFIX.


> These requirements could be met many different ways, since there are several
> scenarios for startup. Clean profile and dirty profile come to mind as
> general scenarios. The UI must be able to load as fast as possible without
> blowing memory usage. Whatever solution we come up with for this bug must
> meet the requirements.

Honestly, there's no feasible way that background services would use Gecko (in the manner that Fennec does now: talking to a Gecko thread) to do network access, so the two options I see are:

* Decide precisely which features we don't have to pass down to libraries on the Java side (proxy config being the trigger for this bug), and implement them. We already have control over cipher suite selection, and on most Android releases we use the system cert keychain. Etc. etc.

* Implement a Java networking library that wraps an optionally profileless Necko without loading the rest of Gecko.
Summary: Fennec does not use gecko networking (ssl, proxy config, etc..) for favicons, search suggestions, or other background operations → Fennec does not respect Gecko-like networking configuration (ssl, proxy config, etc..) for favicons, search suggestions, or other background operations
To reply to some historical comments for context:

(In reply to Patrick McManus [:mcmanus] from comment #14)
> it seems wrong to be using a second stack on the side - too surprising for
> the user.. 

To provide a counter-point, end-users:

* typically are not changing their network configuration within Gecko.
* if anything, typically expect their *Android* system settings to apply, not for there to be a second stack on the side -- the second stack is Necko!
* are way more surprised when a background operation causes OOM than they are by using different network stacks.

The only people I've personally heard complaints from are those people using self-signed certs with custom Sync servers, and wondering why they can load the page in the browser but Sync can't connect.


(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #16)

> To be clear, I think that if Necko really is causing a performance problem
> then we shouldn't switch this stuff back over to Necko until we can be sure
> we are improving performance with the switch.

As Mark mentioned in Comment 17: Necko isn't the problem. Having to load an enormous desktop browser engine, with all of its profile handling, DOM manipulation, yadda yadda, and then having to talk to it essentially using JSON IPC, just to make a network request, is the problem.

I would be happy to use Necko, so long as we can use it 'natively' within Java code without having to start the Gecko runtime and chat with it via JSON messages.

(Broadly speaking, on Android we essentially want Gecko to be a black box that does rendering inside a pure-Java app, because when you're browsing webpages is just about the only time we can be confident that Gecko is alive.)
Blocks: 952799
Depends on: 952970
No longer blocks: 952799
Depends on: 813675
Blocks: 1064837
See Also: → bug 1107569
Comment hidden (me-too)
Duplicate of this bug: 1150899
Tagging this as [tor-mobile] as it relates to Orfox (Tor Browser for Android).  Their patches for this are:

https://github.com/guardianproject/tor-browser/commit/97705b553394f130cd472bdaa7580283bfb3d91f 
    Adds the proxy call to LoadFaviconTask.java

https://github.com/guardianproject/tor-browser/commit/4b68807942c7bce90760ea1f9450f6b4ab9e8db9 
Adds proxy call to CrashReporter (although I thought this was disabled), SuggestClient, Distribution.java, LoadFaviconTask.java (refactoring previous patch), SearchEngineManager, BaseResource.java, and AbstractCommunicator.java.

https://github.com/guardianproject/tor-browser/commit/2866b9a739fba4b8d1bf72cd51e30b2fefa62263 
    DownloadContentHelper.java
Whiteboard: [tor-mobile]
Duplicate of this bug: 1358101
Duplicate of this bug: 1361626
Duplicate of this bug: 1305569
Priority: -- → P3
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
Duplicate of this bug: 1486230
You need to log in before you can comment on or make changes to this bug.