[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:
                                                 |  needs_review
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-mobile, tbb-parity, tbb-         |  Actual Points:
  regression, TorBrowserTeam202004               |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by sysrqb):

 * status:  needs_revision => needs_review


Comment:

 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

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