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

Commit

Permalink
Reverse order in which we read tor control port and auth cookie.
Browse files Browse the repository at this point in the history
The tor daemon writes the control port first, and then the auth
cookie.  Currently we _read_ them in that order too, which means the
following sequence of events might happen:

1. tor writes controlport
2. we read new controlport
3. we read stale control_auth_cookie and give up
4. tor writes control_auth_cookie

Because I inexplicably wasn't thinking about it earlier, I put in two
kludges to work around this:

- We check the mtimes on the control port and control_auth_cookie,
  which is the basis on which we give up in step (2).  Alone, this
  would be harmless, but:

- We rename controlport to controlport.ack so we never read a stale
  version -- but that means if we give up on control_auth_cookie then
  we get wedged.

This changes brave to read the auth cookie first, and then the control
port.  If we successfully read the auth cookie, _then_ we are
guaranteed that the control port has been written, since the control
port write happens before the auth cookie write.

Since the read order is correct now, the kludges should be safely
eliminable and the logic simplified.  However, I will defer that
simplification to a subsequent change.

fix #14517

Auditors: @diracdeltas @bsclifton

Test Plan:
1. Launch Brave.
2. Confirm private tabs with Tor work.
3. Exit Brave.
4. Relaunch Brave.
5. Confirm private tabs with Tor still work.

The symptom that this fixes is that in step (5), Brave thinks that
Tor can't connect to the network, but the tor daemon reports in
$BRAVE/tor/dta/tor.log that it did successfully connect to the
network.

This seems to happen only on Windows.
  • Loading branch information
riastradh-brave authored and bsclifton committed Jun 26, 2018
1 parent 9f12a7f commit 39b4197
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions app/tor.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class TorDaemon extends EventEmitter {
assert(this._control === null)
assert(this._polling)

this._eatControlPort((err, portno, portMtime) => {
this._eatControlCookie((err, cookie, cookieMtime) => {
if (err) {
// If there's an error, don't worry: the file may have been
// written incompletely, and we will, with any luck, be notified
Expand All @@ -248,7 +248,7 @@ class TorDaemon extends EventEmitter {
assert(this._control === null)
assert(this._polling)

this._eatControlCookie((err, cookie, cookieMtime) => {
this._eatControlPort((err, portno, portMtime) => {
if (err) {
// If there's an error, don't worry: the file may not be
// ready yet, and we'll be notified when it is.
Expand Down

0 comments on commit 39b4197

Please sign in to comment.