Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fix some error branches in tor controller logic. #14675

Merged
merged 2 commits into from
Jul 10, 2018
Merged

Fix some error branches in tor controller logic. #14675

merged 2 commits into from
Jul 10, 2018

Conversation

riastradh-brave
Copy link
Contributor

fix #14630, q.v. for test plan

(Issue covers two different bugs, both fixed here in separate commits.)

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

#14630

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@riastradh-brave
TorControl.close() n'existe pas. Use TorControl.destroy().
0dba2d1 
fix #14630 (comment)
@riastradh-brave
Assert that we've closed only on the next event loop iteration.
f0519b0 
The event callbacks may not fire immediately.  We may have to wait
for the readable or writable to notice that they are closed.

It may be that process.nextTick is enough for this, but setImmediate
shouldn't hurt and gives us more time for I/O to happen.

fix #14360

Auditors: @diracdeltas @bsclifton

Test Plan:
@diracdeltas
Copy link
Member

i didn't encounter the error when i started a tor tab with wifi turned off, but i did encounter it when i turned wifi back on and clicked 'retry' to retry connecting to tor:

Jul 06 15:37:17.901 [notice] Tor 0.3.3.7 (git-035a35178c92da94) running on Darwin wi$
h Libevent 2.1.8-stable, OpenSSL 1.0.2o, Zlib 1.2.11, Liblzma N/A, and Libzstd N/A.
Jul 06 15:37:17.902 [notice] Tor can't help you if you use it wrong! Learn how to be
safe at https://www.torproject.org/download/download#warning
Jul 06 15:37:17.902 [notice] Configuration file "/nonexistent" not present, using re$
sonable defaults.
Jul 06 15:37:17.905 [notice] Scheduler type KISTLite has been enabled.
Jul 06 15:37:17.905 [notice] Opening Socks listener on 127.0.0.1:9290
Jul 06 15:37:17.905 [notice] Opening Control listener on 127.0.0.1:0
Jul 06 15:37:17.906 [notice] Control listener listening on port 52492.
tor: control socket error: Error: read ECONNRESET
tor: authentication failure: Error: read ECONNRESET
An uncaught exception occurred in the main process Uncaught Exception:
AssertionError: false == true
    at TorControl.destroy (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/browser-
laptop/app/tor.js:884:5)
    at TorDaemon.kill (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/browser-lapt
op/app/tor.js:164:21)
    at _control.cmd1 (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/browser-lapto
p/app/tor.js:512:16)
    at TorControl.destroy (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/browser-
laptop/app/tor.js:897:7)
    at TorDaemon._controlError (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/bro
wser-laptop/app/tor.js:546:19)
    at TorControl._control.on (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/brow
ser-laptop/app/tor.js:494:47)
    at emitOne (events.js:96:13)
    at TorControl.emit (events.js:191:7)
    at TorControl._onError (/Users/yan/repos/tmp/browser-laptop-bootstrap/src/browser
-laptop/app/tor.js:1058:10)
    at emitOne (events.js:96:13)

tor.log shows

Jul 06 15:37:18.000 [notice] Tor 0.3.3.7 (git-035a35178c92da94) opening new log file.
Jul 06 15:37:18.906 [notice] Tor 0.3.3.7 (git-035a35178c92da94) running on Darwin with Libevent 2.1.8-stable, OpenSSL 1.0.2o, Zlib 1.2.11, Liblzma N/A, and Libzstd N/A.
Jul 06 15:37:18.907 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Jul 06 15:37:18.907 [notice] Configuration file "/nonexistent" not present, using reasonable defaults.
Jul 06 15:37:18.910 [notice] Scheduler type KISTLite has been enabled.
Jul 06 15:37:18.910 [notice] Opening Socks listener on 127.0.0.1:9290
Jul 06 15:37:18.910 [notice] Opening Control listener on 127.0.0.1:0
Jul 06 15:37:18.910 [notice] Control listener listening on port 52494.
Jul 06 15:37:18.000 [notice] Parsing GEOIP IPv4 file /usr/local/share/tor/geoip.
Jul 06 15:37:19.000 [notice] Parsing GEOIP IPv6 file /usr/local/share/tor/geoip6.
Jul 06 15:37:19.000 [notice] Bootstrapped 0%: Starting
Jul 06 15:37:19.000 [notice] Starting with guard context "default"
Jul 06 15:37:19.000 [notice] Bootstrapped 80%: Connecting to the Tor network
Jul 06 15:37:23.000 [notice] Bootstrapped 85%: Finishing handshake with first hop
Jul 06 15:37:23.000 [notice] Bootstrapped 90%: Establishing a Tor circuit
Jul 06 15:37:24.000 [notice] Tor has successfully opened a circuit. Looks like client functionality is working.
Jul 06 15:37:24.000 [notice] Bootstrapped 100%: Done
Jul 06 15:37:24.000 [warn] Your Guard Multivac ($1C90D3AEADFF3BCD079810632C8B85637924A58E) is failing a very large amount of circuits. Most likely this means the Tor network is overloaded, but it could also mean an attack against you or potentially the guard itself. Success counts are 97/250. Use counts are 11/11. 245 circuits completed, 0 were unusable, 148 collapsed, and 152 timed out. For reference, your timeout cutoff is 60 seconds.
Jul 06 15:41:24.000 [notice] Catching signal TERM, exiting cleanly.

@riastradh-brave
Copy link
Contributor Author

I see what the problem is, oops. Working on a fix.

bsclifton
bsclifton approved these changes Jul 10, 2018
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested before and after this patch- works great! 😄👍

@bsclifton bsclifton merged commit e680e1c into master Jul 10, 2018
@bsclifton bsclifton mentioned this pull request Jul 10, 2018
Closed
@bsclifton bsclifton deleted the riastradh-tor-controlerrorbranches branch July 10, 2018 16:51
riastradh-brave added a commit that referenced this pull request Jul 10, 2018
@riastradh-brave
Allow TorControl to be idempotent.
59b3cce 
This should address the failure reported in:
#14675 (comment)

fix #14630

Auditors: @diracdeltas

Test Plan: see #14630
riastradh-brave added a commit that referenced this pull request Jul 10, 2018
@riastradh-brave
Allow TorControl.destroy to be idempotent.
2a30a9b 
This should address the failure reported in:
#14675 (comment)

fix #14630

Auditors: @diracdeltas

Test Plan: see #14630
@diracdeltas
Copy link
Member

@bsclifton did you mean to merge this into the release branches?

diracdeltas pushed a commit that referenced this pull request Jul 10, 2018
@bsclifton @diracdeltas
Merge pull request #14675 from brave/riastradh-tor-controlerrorbranches
008f99c 
Fix some error branches in tor controller logic.
@diracdeltas
Copy link
Member

doing it now since it's needed for #14703 to apply cleanly

diracdeltas pushed a commit that referenced this pull request Jul 10, 2018
@bsclifton @diracdeltas
Merge pull request #14675 from brave/riastradh-tor-controlerrorbranches
08e9b49 
Fix some error branches in tor controller logic.
@diracdeltas
Copy link
Member

0.23.x: 08e9b49
0.24.x: 008f99c
master: e680e1c

@bsclifton
Copy link
Member

@diracdeltas yes- I did mean to but forgot; thanks! 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@diracdeltas diracdeltas Awaiting requested review from diracdeltas

Assignees

@riastradh-brave riastradh-brave

Labels
feature/tor
Projects
None yet
Milestone
0.23.x Release 3 (Release Channel)
Development

Successfully merging this pull request may close these issues.

Tor control socket error when network is disconnected
3 participants
@riastradh-brave @diracdeltas @bsclifton