[tor-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, TorBrowserTeam202004R              |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by gk):

 Replying to [comment:7 sysrqb]:
 > Replying to [comment:6 acat]:
 > > I did not have time to fully understand the code, but let's see. My
 understanding is that the code in `CustomTorInstaller.java` cannot do the
 filtering, since it's not responsible of parsing the bridges, and the
 changes are needed so that the code that parses the bridges can filter
 them. I hope that's correct.
 >
 > Correct.
 >
 > >
 > > Ok, so in https://gitweb.torproject.org/user/sysrqb/tor-android-
 service.git/commit/?h=bug33931_00&id=769b3c85de468bb23fbb891266ab6cbb9c662e13
 > >
 > > would it make sense to update the comment:
 > > `For (1), we just pass back all bridges, the filter will occur
 elsewhere in the library.`
 > > to include the fact that we are also encoding the bridge type that has
 to be filtered?
 >
 > Yes, I'll add that comment.
 >
 > >
 > > Besides, given the `userDefinedBridgeList.length() > 5` check, I think
 `userDefinedBridgeList` cannot be `"meek_lite"`, so I assume this can be
 removed from the switch.
 >
 > Yes. I'll remove that.
 >
 > >
 > > 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.

 > >
 > > The rest looks ok to me,. I did not have time to test the patch, but
 I'll start a build for that in case it's needed (it will take a while).
 >
 > Great, thanks for the review!

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