Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9665 closed defect (implemented)

if no bridges are usable, tor should report a bootstrap error

Reported by: mcs Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version: Tor: 0.2.4.14-alpha
Severity: Keywords: tor-client tor-pt easy 025-triaged, tbb-tor-backported-3.6b2
Cc: brade, mikeperry, nickm, arma, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In testing Tor Launcher changes while working on #9445, Kathy Brade and I found that when tor is only configured with bridges it cannot use due to a lack of PT plugins, the bootstrap process just stops making progress. It would be much better if an error was reported via the control port. Here is our torrc:

AvoidDiskWrites 1
bridge obfs2 91.143.91.97:34770
ControlPort 9151
CookieAuthentication 1
DataDirectory /Users/brade/dev/tor/tbb-for-tl-testing/TBB-3.0a2.app/Contents/Resources/Data/Tor
DirReqStatistics 0
GeoIPFile ../../Contents/Resources/Data/Tor/geoip
Log notice stdout
SocksListenAddress 127.0.0.1
SocksPort 9150
UseBridges 1

Here is the tor log:

Sep 04 11:37:33.689 [notice] Tor v0.2.4.14-alpha (git-f5729b8c1d45933f) running on Darwin with Libevent 2.0.21-stable and OpenSSL 1.0.1e.
Sep 04 11:37:33.689 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Sep 04 11:37:33.689 [notice] This version is not a stable Tor release. Expect more bugs than usual.
Sep 04 11:37:33.710 [notice] Read configuration file "/Users/brade/dev/tor/tbb-for-tl-testing/TBB-3.0a2.app/Library/Vidalia/torrc".
Sep 04 11:37:33.714 [notice] Opening Socks listener on 127.0.0.1:9150
Sep 04 11:37:33.714 [notice] Opening Control listener on 127.0.0.1:9151
Sep 04 11:37:33.000 [notice] New control connection opened.
Sep 04 11:37:33.000 [notice] New control connection opened.
Sep 04 11:37:34.000 [notice] Bootstrapped 5%: Connecting to directory server.
Sep 04 11:37:34.000 [warn] We were supposed to connect to bridge '91.143.91.97:34770' using pluggable transport 'obfs2', but we can't find a pluggable transport proxy supporting 'obfs2'. This can happen if you haven't provided a ClientTransportPlugin line, or if your pluggable transport proxy stopped running.

Child Tickets

Attachments (2)

0001-Fix-bug9665.patch (1.0 KB) - added by nickm 6 years ago.
Patch from Fábio Bertinatto
0002-Fix-bug9665.patch (1.7 KB) - added by fabio 6 years ago.
Reworked patch

Download all attachments as: .zip

Change History (22)

comment:1 Changed 6 years ago by mcs

Cc: nickm arma added

comment:2 Changed 6 years ago by nickm

Keywords: tor-client tor-pt easy added
Milestone: Tor: 0.2.5.x-final

Hm. This should be as simple as adding a control_event_bootstrap_problem() call to whatever part of the code generates that message, I think...

Though we should also have a controller message and a log if *all* bridges are unuseable.

Changed 6 years ago by nickm

Attachment: 0001-Fix-bug9665.patch added

Patch from Fábio Bertinatto

comment:3 Changed 6 years ago by nickm

Status: newneeds_review

Fábio Bertinatto has written a patch on tor-dev (subject "Bug 9665"). Putting it here for review.

comment:4 in reply to:  2 ; Changed 6 years ago by nickm

At first glance, the attached patch appears to solve this part:

Hm. This should be as simple as adding a control_event_bootstrap_problem() call to whatever part of the code generates that message, I think...

But not this part, yet:

Though we should also have a controller message and a log if *all* bridges are unuseable.

Changed 6 years ago by fabio

Attachment: 0002-Fix-bug9665.patch added

Reworked patch

comment:5 in reply to:  4 Changed 6 years ago by fabio

From my understanding it seems that the only place I can know whether all bridges are usable is the loop inside the "fetch_bridge_descriptors" function. However, there are cases when this functions is called 2 times during the tor startup.

Moreover, the variable "bridge->fetch_status.n_download_failures", where I suppose I can check if the bridge is ok, is always set to 1 even when tor can establish a connection.

Please let me know if I'm misunderstanding the code.

Last edited 6 years ago by fabio (previous) (diff)

comment:6 Changed 6 years ago by andrea

Triage for 0.2.5.x: looks fairly straightforward, keep in 0.2.5.x.

comment:7 Changed 6 years ago by nickm

Keywords: 025-triaged added

comment:8 Changed 6 years ago by nickm

So, the first one seems to work as advertised, and if it does what Mike and Kathy need, I say we should merge that. The second one doesn't actually work: I tried starting Tor with 3 perfectly fine bridges, and it generated its warning anyway. If we need a version that logs a problem when _all_ bridges have their plugins missing, we need to do more work here.

comment:9 Changed 6 years ago by mcs

Is the issue that if three bridges are configured without a corresponding ClientTransportPlugin, then three bootstrap errors will be generated? For TBB, with Tor Launcher, I think this will be OK because Tor Launcher suppresses repeated errors (that is, it does not display an error alert to the user) if the TAG and REASON match the the last bootstrap error that was reported. So the user should only see one error message if this case.

comment:10 Changed 6 years ago by nickm

So, the problem I thought there was with patch 1 is that if three broken bridges are configured, three bootstrap errors are generated, but we don't tell you whether that's a blocker or not. (We don't generate a final "All bridges are broken!" message.)

The problem with patch 2 is that it tries to generate an "all bridges are broken!" message but it doesn't actually do it right.

If the behavior I described for patch 1 is good enough for TBB/ TorLauncher, please let me know and I'll forward-port the patch and merge it.

comment:11 Changed 6 years ago by nickm

Cc: mcs added

comment:12 in reply to:  10 Changed 6 years ago by mcs

Replying to nickm:

If the behavior I described for patch 1 is good enough for TBB/ TorLauncher, please let me know and I'll forward-port the patch and merge it.

brade and I tested a tor with patch 1 applied with TBB/Tor Launcher. I think the behavior is OK. Users will see an error alert that reads "Connecting to a relay directory failed (no route to host)." From there they can get to the tor log to figure out why they got this error. It would be nice if the error was something more specific than END_OR_CONN_REASON_NO_ROUTE but I am not sure what the cost is to add a new error code.

comment:13 Changed 6 years ago by nickm

Any suggestions for the new error code? It should be pretty easy to add one.

comment:14 Changed 6 years ago by nickm

Also, is it okay with you that we'll get these warnings when only *some* of the bridges have a missing PT?

comment:15 in reply to:  14 Changed 6 years ago by mcs

Replying to nickm:

Also, is it okay with you that we'll get these warnings when only *some* of the bridges have a missing PT?

I just tested that scenario. What currently happens is that Tor Launcher puts up an error alert (which is OK, although users may think all hope is lost at that point). Unfortunately, Tor Launcher also removes the progress bar from its network status window, even though progress is still being made due to the presence of another PT bridge. We should be able to fix Tor Launcher to restore the progress bar when more progress is made, but it would be better to have the more complete fix inside tor.

comment:16 Changed 6 years ago by nickm

Actually, I feel like George's #11069 fix might have improved this to work more like how we'd like.

(I'm assuming you didn't run with George's #11069 fix, since the patch above isn't compatible with it.)

I've made a new branch "bug9665_redox" in my public repository, starting with Fábio's first patch, but forward-porting it and tweaking the error code not to be the always-warning NO_ROUTE. Is this a bit better?

(If not, is it still worth doing this in 0.2.5? To do this in the maximally smart way, we might need to refactor the code that tracks what we know about bridge status.)

comment:17 in reply to:  16 Changed 6 years ago by brade

Replying to nickm:

I've made a new branch "bug9665_redox" in my public repository... Is this a bit better?

Yes, the behavior seems good. We will go ahead and add knowledge of the new PT_MISSING error (the error messages are localizable).

comment:18 Changed 6 years ago by brade

comment:19 Changed 6 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged to master; documented in control-spec.txt with 358e8ac97811ff2ad6de34f84fb59a92612b4a08 .

comment:20 Changed 6 years ago by mikeperry

Keywords: tbb-tor-backported-3.6b2 added
Note: See TracTickets for help on using tickets.