[tor-bugs] #23136 [Applications/Tor Launcher]: moat integration (fetch bridges for the user)

[Tor Bug Tracker & Wiki]

#23136: moat integration (fetch bridges for the user)
-------------------------------------------+-------------------------------
 Reporter:  mcs                            |          Owner:  brade
     Type:  defect                         |         Status:
                                           |  needs_revision
 Priority:  Very High                      |      Milestone:
Component:  Applications/Tor Launcher      |        Version:
 Severity:  Normal                         |     Resolution:
 Keywords:  TorBrowserTeam201803, ux-team  |  Actual Points:
Parent ID:  #24689                         |         Points:
 Reviewer:                                 |        Sponsor:  Sponsor4
-------------------------------------------+-------------------------------

Comment (by mcs):

 Replying to [comment:71 gk]:
 > Okay, three final items:
 >
 > 16) "in CMETHOD responses" -> "in CMETHOD response"

 Good catch.

 > 17)
 >
 > Do you mind flipping
 > {{{
 >         version: this.kMoatVersion,
 >         type: this.kMoatCheckRequestType,
 > }}}
 > to
 > {{{
 >         type: this.kMoatCheckRequestType,
 >         version: this.kMoatVersion,
 > }}}
 > in `finishFetch()` so that it conforms to the order outlined in the
 spec?

 Sure, we will make that change.

 > 18)
 >
 > Two points regarding your PT spec compliance:
 >
 > a) Setting `TOR_PT_STATE_LOCATION` is required according to the PT spec
 but is missing in `envAddidions`. Is that intentional? It seems it
 benefits from `meek` not caring about that and I wonder what to do. At
 least we should add a comment explaining why we deviate from the spec.

 Not setting that is an oversight on our part. The meek client doesn't use
 it, but it might in the future or maybe obfs4proxy in meek_lite mode needs
 it. We will add it.

 > b)
 > {{{
 >               if (proxyType == "socks4a")
 >                 this.mMeekClientProxyType = "socks4";
 >               else if (proxyType == "socks5")
 >                 this.mMeekClientProxyType = "socks";
 >               else
 >                 this.mMeekClientProxyType = proxyType;
 >
 > }}}
 > is overly lenient IMO. The spec states that the client talks some sort
 of SOCKS. Thus, we should make sure we get either SOCKS4 or SOCKS5 back
 and error out in case anything else shows up. So, probably something like
 this (taking the SOCKS 5 prioritization in the PT spec into account):
 > ...

 We will make the change you suggested. A new patch is coming soon!

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