[tbb-bugs] #22170 [Applications/Tor Browser]: Check uses of ch.boye.httpclientandroidlib.impl.client.* for proxy safety on Android

[Tor Bug Tracker & Wiki]

#22170: Check uses of ch.boye.httpclientandroidlib.impl.client.* for proxy safety
on Android
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  sysrqb
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ff52-esr, tbb-mobile,                |  Actual Points:
  TorBrowserTeam201808R                          |
Parent ID:  #21863                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by gk):

 Replying to [comment:21 sysrqb]:
 > Replying to [comment:20 sysrqb]:
 > > All files where Fennec uses `impl.client`
 > >
 > > {{{
 > > $ git grep -n ch.boye.httpclientandroidlib.impl.client
 mobile/android/[bs]*
 > >
 mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java:15:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > > }}}
 >
 > We should never get here because its telemetry, but it's worth checking.
 The DefaultHttpClient is passed in, but not created. The `DATE` headers is
 set. A `BaseResource` is created and `BaseResource.postBlocking()` is
 called. The proxy will be set within `BaseResource.execute()`.

 You mean `BaseResource.prepareClient()`?

 [snip]

 > {{{
 > /**
 >  * Interact with the autopush endpoint HTTP API.
 >  * <p/>
 >  * The API is a Mozilla-proprietary interface, and not even specified to
 Mozilla's usual ad-hoc standards.
 >  * This client is written against a work-in-progress, un-deployed
 upstream commit.
 >  */
 > }}}
 >
 > That's reassuring.

 Indeed.

 > All connections are created via `BaseResource`. DefaultHttpClient is
 passed into an `addHeader()` where an `ACCEPT_LANGAUGE` and `ACCEPT`
 header is added.
 >
 > > {{{
 > >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/AbstractBearerTokenAuthHeaderProvider.java:9:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > > }}}
 >
 > `DefaultHttpClient` isn't used. No network calls in this class.
 >
 > > {{{
 > >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/AuthHeaderProvider.java:11:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > > }}}
 >
 > This is an `interface`, no logic here.
 >
 > > {{{
 > >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResource.java:51:import
 ch.boye.httpclientandroidlib.impl.client.BasicAuthCache;
 > >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResource.java:52:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > > }}}
 >
 > This class is probably proxy-safe. I'll need to look at this again (and
 a second pair of eyes would be welcome).

 Looks good to me.

 [snip]

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22170#comment:29>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online