[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 sysrqb):

 Replying to [comment:18 sysrqb]:
 > NOTE: this resolved the proxy address using the system DNS resolver
 [13]. This shouldn't leak anything, but we don't need this.
 > [13] https://gitweb.torproject.org/tor-
 browser.git/tree/mobile/android/thirdparty/ch/boye/httpclientandroidlib/impl/conn/DefaultClientConnectionOperator.java?h
 =tor-browser-60.1.0esr-8.0-1#n159

 This actually uses `java.net.InetAddress.getAllByName()` which resolves a
 host name or parses the a literal IP address:
 {{{
 The host name can either be a machine name, such as "java.sun.com", or a
 textual representation of its IP address. If a literal IP address is
 supplied, only the validity of the address format is checked.
 }}}
 https://developer.android.com/reference/java/net/InetAddress.html#getAllByName(java.lang.String)

 We are currently hard-coding an IP address, so this should not be a
 problem - in theory. The main problem here is we configure the proxy using
 an IP address, but that is stored as a string when it is passed around
 between the different layers of abstraction as a HTTP parameter. The IP
 address-as-a-String is then parsed into a InetAddress when the connection
 is created. I see four options here:
   1. We leave this as it is and assume this the Android API "does the
 right thing"
   2. We hard-code the InetAddress at this point in the code, too
   3. We modify httpclientandroidlib so the `DEFAULT_PROXY` parameter is
 stored as a URI so we never need to reparse the address
   4. We copy another IP address parsing implementation and use that
 instead of relying on the Android implementation (something like Google
 Guava's which provides similar functionality but does not perform name
 resolution - `InetAddresses.forString(String)`)
 {{{
  * <p><b>Important note:</b> Unlike {@code InetAddress.getByName()}, the
 methods of this class never
  * cause DNS services to be accessed. For this reason, you should prefer
 these methods as much as
  * possible over their JDK equivalents whenever you are expecting to
 handle only IP address string
  * literals -- there is no blocking DNS penalty for a malformed string.
 }}}
 https://github.com/google/guava/blob/master/android/guava/src/com/google/common/net/InetAddresses.java#L128

 I'm more in favor or (2) or (4).

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