[tbb-bugs] #33931 [Applications/Tor Browser]: obfs4 bridges are used instead of meek if meek is selected in Tor Browser for Android alpha

[Tor Bug Tracker & Wiki]

#33931: obfs4 bridges are used instead of meek if meek is selected in Tor Browser
for Android alpha
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:  closed
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:  fixed
 Keywords:  tbb-mobile, tbb-parity, tbb-         |  Actual Points:
  regression, TorBrowserTeam202004               |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by gk):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Replying to [comment:10 sysrqb]:
 > Replying to [comment:8 gk]:
 > > Replying to [comment:7 sysrqb]:
 > > > Replying to [comment:6 acat]:
 > > > > I guess there are no other values that could make `bridgeType=0`
 other than the empty string? If we know all the possible values of
 `userDefinedBridgeList` (when `bridgeType == 0`), would it make sense to
 have cases for all of them, and then have a default that throws an error
 (similar to the switch in https://gitweb.torproject.org/user/sysrqb/tor-
 browser-
 build.git/commit/?h=bug33931_00&id=91e6aec4f60783fc0008d4d3c60c29ddecafac0d)?
 > > >
 > > > The defined cases in this patch are the only pluggable transports we
 currently use (`obfs4` and `meek(_lite)`). In the past, we had `obfs3`. I
 don't think we should expand this list, but (after thinking about this a
 little more) we should fail safe if the type is not recognized instead of
 "all bridges" being the default. I'll add that in a fixup commit. Thanks!
 > >
 > > I am not exactly sure if that's what you meant but I think the case
 where `bridgeType` for whatever reason is still `0` at the end of
 `openBridgeSream()` should be treated in `addBridgesFromResources()` in
 the `throw` block. That is, there should not be a `case 0` check anymore.
 Keeping that smells just like trouble we are currently dealing with. We
 start to be explicit with your patches in this bug. so let's avoid
 ambiguity here where we can.
 >
 > I think I was imagining that we would `throw` at the end of
 `openBridgeStream()`, but letting`addBridgesFromResources()` handle this
 error case seems okay to me, in this case, as well.
 >
 > I pushed a fixup commit onto my tor-android-service `bug33931_00`
 branch, and I pushed a new tor-browser-build branch `bug33931_01`
 containing an updated TOPL patch.
 >
 > https://gitweb.torproject.org/user/sysrqb/tor-android-
 service.git/commit/?h=bug33931_00&id=42d2cdb46542ff488ce0ed4bf9e5b41b3a8356a7
 >
 > https://gitweb.torproject.org/user/sysrqb/tor-browser-
 build.git/commit/?h=bug33931_01&id=04da65342a76f74b3d4a58601f326ded457dc97a

 Looks good to me. Squashed and merged to `tor-android-service`'s `master`
 (commit ed2e1479eeddede01b1d510ef79dc5ec798b39c0) and merged to `tor-
 browser-build`'s `master` + a commit to pick the `tor-android-service`
 changes up (commits 04da65342a76f74b3d4a58601f326ded457dc97a and
 b858dca8745b583afd2660389ccd5a96630154a0).

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