Using private obfs4 bridge does not show circuit display
After providing some private obfs4 bridges, an user reported:
In the Tor Browser (stable and alpha 9.0a8), I cannot view my circuit or click to change my Tor circuit for a website while using a private obfs4.
Activity
I could reproduce this issue with one of the bridges that ggus is talking about. Here is its relay status page: https://metrics.torproject.org/rs.html#details/7234CB14A260A862BA9000004ED1807E6B6B70D6
Ok, here is I think the issue:
In all the bridge lines that work, there is an identity key fingerprint in the bridge line (the 40 hexes after the IP:port).
In the one that doesn't work, that fingerprint is missing.
I believe the fingerprint is technically optional, but I believe bridgedb gives it out always, so probably the tor browser folks figured it would always be there.
Here's a patch that fixes this issue:
With this patch, bridges which have no provided fingerprint will display simply as "Bridge" rather than "Bridge: obfs4 127.0.0.1" (for example). Without a fingerprint, neither type nor ip are determinable from tor-button.
I've dug into this issue a bit and here's a summary of the problem.
Tor Browser builds the circuit display by requesting the circuit (as a list of ids/fingerprints) for the current first-party domain. Then, Tor Browser checks to see if each of nodes are a Bridge by requesting the bridge strings via
getconf bridge. Next, we look for the current id in each of the bridge objects and execute some UI logic that displays each node's info (whether bridge or vanilla relay).When the user provides a bridge without a fingerprint we run into a few problems resulting in the broken behavior in the bug. First of all, requesting the bridge list via getconf bridge (basically) just returns the bridge strings stored in torrc. These strings are either provided by the user, brigedb, or our builtin bridge preferences. The bridge string format is something like
$type $ip:$port $optionalfingerprint $otherstuff....As the fingerprint is optional, the bridge parser in tor-control-port.js ends up storing the $otherstuff data as the id/fingerprint.
Then, getBridge() in tor-circuit-display.js fails to find a bridge by id/fingerprint (because our bridge has $otherstuff in the id field) and returns null.
Next, nodeDataForId() in tor-circuit-display.js skips over the 'bridge' section (as the result of getBridge() is null) and it assumes we are dealing with a relay. The failing
GETINFO ns/id/$fingerprintcall ends up throwing exception because we're passing in a bridge id/fingerprint which is not part of the consensus (according to arma). As a result, the IP of the relay is never set.Since the IP of the relay is not stored, updateCircuitDisplay() in tor-circuit-display() (which is responsible for creating the visuals including the New Circuit button) throws after trying to dereference a null reference, resulting in the behavior described in the description.
Trac:
Keywords: N/A deleted, TorBrowserTeam201910R added
Cc: phw to phw, tbb-team
Actualpoints: N/A to 0.25
Status: needs_information to needs_review-
Replying to pospeselr:
Next, nodeDataForId() in tor-circuit-display.js skips over the 'bridge' section (as the result of getBridge() is null) and it assumes we are dealing with a relay. The failing
GETINFO ns/id/$fingerprintcall ends up throwing exception because we're passing in a bridge id/fingerprint which is not part of the consensus (according to arma). As a result, the IP of the relay is never set.Right -- getinfo ns/id/ calls router_get_consensus_status_by_id() on the digest, which hunts for the relay info among the response from networkstatus_get_latest_consensus(). But since bridges aren't (typically) in the consensus, you won't (typically) have any useful result here.
I think the patch here is the right one in the short term, and maybe even in the long term too because of simplicity. But we should answer the question of "how controllers ought to handle this situation" on the Tor side, in case Tor Browser or somebody else wants to do it in the future. For example, check out "getinfo /ns/purpose/bridge" as a potentially useful building block: it asks Tor to fabricate a status entry like one would find in the consensus, for each of its current bridges. But maybe there is an even better way. I've opened #32157 (moved) to move this question forward on the Tor side.
This looks good to me. Applied to
master(commit 73df82bd36a812496067c8bef948df5130cb991b). We should figure out what follow-up ticket(s) we exactly want to have. I think for one we should fix the TODO you added. But maybe there is more while we are at it and we should put making the circuit display simpler by generally showing only "Bridge" on our radar, too?Trac:
Keywords: TorBrowserTeam201910R deleted, tbb-backport, TorBrowserTeam201910 added
Status: needs_review to needs_information-
We have #32204 (moved) and #32205 (moved) now, I think that's a good start, closing.
Trac:
Status: needs_information to closed
Resolution: N/A to fixed mentioned in issue #32157 (moved)
mentioned in issue #32204 (moved)
mentioned in issue tpo/core/tor#32157
mentioned in issue tpo/core/tor#32204 (closed)
