[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #28655 [Circumvention/BridgeDB]: If a bridge supports obfs4, don't give out its other flavors

#28655: If a bridge supports obfs4, don't give out its other flavors
------------------------------------+-----------------------------
 Reporter:  arma                    |          Owner:  phw
     Type:  defect                  |         Status:  merge_ready
 Priority:  High                    |      Milestone:
Component:  Circumvention/BridgeDB  |        Version:
 Severity:  Normal                  |     Resolution:
 Keywords:  bridgedb                |  Actual Points:
Parent ID:                          |         Points:  2
 Reviewer:                          |        Sponsor:  Sponsor19
------------------------------------+-----------------------------
Changes (by sysrqb):

 * status:  needs_review => merge_ready


Comment:

 Replying to [comment:18 phw]:
 > Replying to [comment:17 sysrqb]:
 > > - I agree extending `SUPPORTED_TRANSPORTS` or creating a new `list`
 config option like `PROBING_RESISTANT_TRANSPORTS` is a good idea. Hard-
 coding the list of probing resistant PTs in one place is not great, but
 hard-coding them in two places is asking for bugs :)
 >
 > I created a separate `PROBING_RESISTANT_TRANSPORTS` in bridgedb.conf,
 right under `SUPPORTED_TRANSPORTS`. I was a bit undecided if this is
 something we should expose in the BridgeDB config because it's not meant
 to be configurable unless you really know what you're doing. That said, I
 agree that a separate config options seems to be the cleanest solution.

 Yeah, I agree with you about this not being a bridgedb-specific attribute.
 But, bridgedb must learn this somehow, so either "hard-coding the list in
 a (constant) variable" or "adding this as a config option" seem like the
 best choices. I don't have a strong opinion on which is better, I simply
 didn't like seeing the list of PTs hard-coded in multiple places :)

 >
 > > For the leekspin patch, I think it looks good. My only concern is in
 the
 [https://gitweb.torproject.org/user/phw/leekspin.git/tree/leekspin/util.py?id=3bc9c660e8df80fe89693c8e4fad38955011bf20#n65
 description] of the new argument. It says `m` out of `n`, but it's not
 immediately obvious what `m` is here. `n` is an actual argument (`-n`,
 `--descriptors`), but `m` is not a valid argument. Replacing `<m>` with
 `<xp>` would make it more readable, or somehow note `m` **is** `xp`: "make
 `<m>` (`xp`) out of all `<n>`".
 >
 > Good point, also fixed.
 >

 Great, looks good. I think the commits can be squashed before merging.

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

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs