[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:
                                                 |  accepted
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ff52-esr, tbb-mobile,                |  Actual Points:
  TorBrowserTeam201807                           |
Parent ID:  #21863                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by 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()`.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/background/fxa/FxAccountClient20.java:50:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 All connections are created via `BaseResource`. DefaultHttpClient is
 passed into an `addHeader()` where an `ACCEPT_LANGAUGE` and `ACCEPT`
 header is added.

 Note: FxA uses a unique user agent string in its request.
 https://gitweb.torproject.org/tor-
 browser.git/tree/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/FxAccountConstants.java?h
 =tor-browser-60.1.0esr-8.0-1#n40

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/background/fxa/oauth/FxAccountAbstractClient.java:30:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 DefaultHttpClient is passed into an `addHeader()` where an
 `ACCEPT_LANGAUGE` and `ACCEPT` header is added.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/push/autopush/AutopushClient.java:35:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 {{{
 /**
  * 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.

 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).

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResourceDelegate.java:8:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 This class only provides accessors and mutators, no network calls.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BasicAuthHeaderProvider.java:12:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 No network calls.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/HMACAuthHeaderProvider.java:23:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 `DefaultHttpClient` isn't used. No network calls.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/HawkAuthHeaderProvider.java:29:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 `DefaultHttpClient` isn't used. No network calls.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/ResourceDelegate.java:13: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/SyncStorageCollectionRequest.java:20:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 {{{
 // TODO: this is awful.
 }}}

 Sets `ACCEPT` header. This class mostly handles HTTP responses.

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/SyncStorageRequest.java:20:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 Adds a `x-if-unmodified-since` header. Uses `BaseResource` for creating
 network connections.

 Note: uses another different user agent string.
 https://gitweb.torproject.org/tor-
 browser.git/tree/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SyncConstants.java?h
 =tor-browser-60.1.0esr-8.0-1#n40

 > {{{
 >
 mobile/android/services/src/main/java/org/mozilla/gecko/tokenserver/TokenServerClient.java:37:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}

 Sets `X-Conditions-Accepted` and `X-Client-State` headers. Uses
 `BaseResource` for networking.

 > {{{
 >
 mobile/android/services/src/test/java/org/mozilla/android/sync/test/helpers/MockResourceDelegate.java:9:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 >
 mobile/android/services/src/test/java/org/mozilla/gecko/sync/net/test/TestHawkAuthHeaderProvider.java:12:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 >
 mobile/android/services/src/test/java/org/mozilla/gecko/sync/net/test/TestLiveHawkAuth.java:11:import
 ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient;
 > }}}
 Testing.

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