Circuit visualizer stops working after some time
So far I haven't been able to isolate this problem, but basically:
-
Use Tor Browser 5.0.2 (current at time of writing), GNU/Linux, 64-bit.
Stock, no additional extensions or addons of any sort; security slider set to high; very strict/restrictive NoScript configuration; some other minor preference tweaks.
-
Have Tor Browser running for a relatively long period, say days.
-
"Eventually," the circuit visualization is no longer there.
The TorButton menu looks and acts normally except for the fact that the right half of it no longer shows the circuit graph, the whole menu is now made only of the 5 buttons (from "New identity" to "Check for Tor Browser update").
This is the case for any web page, regardless of domain, scheme, etc.
-
From this point on, the only thing that seems to restore the circuit \ visualization functionality is restarting the browser. I tried "New \ identity" and "New Tor circuit for this site" with no change.
I'll try to follow up with a comment when/if I manage to identify more precisely "when/how" the issue manifests itself.
BTW, I'm pretty sure I read a comment on the blog about this (may have been months ago), though I cannot find it now.
Activity
I've seen this, too, exactly once while debugging #15493 (moved) (and it is mentioned in that ticket). While I have a debug log it did not go deep enough to find the culprit. :(
Trac:
Cc: N/A to gk-
I think I've tracked that one down. The bug is in
info.stringToValue()intor-control-port.js. More exactlystring.match(/250[ -].+?=(.*?)$/mi)is wrong. There are (or could be) exit relays out there whose nickname ends with "250", e.g. https://atlas.torproject.org/#details/B486925DC901969CCE2B371E93740CF98C30539D. When this is happening and someone is picking this relay as an exit relay the above mentioned code kicks in (although it is not supposed to do that) and something like
NEED_CAPACITY PURPOSE=GENERAL TIME_CREATED=2015-11-04T11:33:11.034987 SOCKS_USERNAME="torproject.org" SOCKS_PASSWORD="0"gets passed to
utils.listMapData()(viainfo.circuitStatusParser()) resulting incircIDbeingNEED_CAPACITY. This in turn letsgetCircuitStatusByID()returnnullleading to the missing circuit display.Trac:
Keywords: tbb-torbutton tbb-circuit-display deleted, tbb-torbutton tbb-circuit-display TorBrowserTeam201511 added
Cc: gk to gk, arthuredelstein Nice find! Here is a patch for review:
https://github.com/arthuredelstein/torbutton/commit/16990
Trac:
Status: new to needs_review
Keywords: tbb-torbutton tbb-circuit-display TorBrowserTeam201511 deleted, tbb-torbutton tbb-circuit-display TorBrowserTeam201511R addedWhy the non-greedy globbing (the "?" modifier added to "+" or "*") when you are matching all the rest of the line anyway?
Compare:
520 - let matchResult = string.match(/^250[ -].+?=(.*?)$/mi) || 520 + let matchResult = string.match(/^250[ -].+?=(.*)$/mi) ||You do this in several other places in this file. Just wondering, I don't think it makes a difference.
Also, based on the regex above, shouldn't the comment in line 515 be like so?
515 - // or `250 circuit-status...` 515 + // or `250 circuit-status=...`To match the rest of the comment.
Also, I do not know what the assumptions/pre-conditions are on the input string, but you do not check that the first line of the multiline case ("!^250+") ends with "=", as exemplified in the commentary for "info.keyValueStringsFromMessage".
Anyway, sorry for wasting your time, I'm not familiar with this code.
Replying to cypherpunks:
Why the non-greedy globbing (the "?" modifier added to "+" or "*") when you are matching all the rest of the line anyway?
Compare: {{{ 520 - let matchResult = string.match(/^250[ -].+?=(.?)$/mi) || 520 + let matchResult = string.match(/^250[ -].+?=(.)$/mi) || }}} You do this in several other places in this file. Just wondering, I don't think it makes a difference.
Also, based on the regex above, shouldn't the comment in line 515 be like so? {{{ 515 - // or
250 circuit-status...515 + // or250 circuit-status=...}}} To match the rest of the comment.Also, I do not know what the assumptions/pre-conditions are on the input string, but you do not check that the first line of the multiline case ("!^250+") ends with "=", as exemplified in the commentary for "info.keyValueStringsFromMessage".
Anyway, sorry for wasting your time, I'm not familiar with this code.
Thanks for the very helpful comments. I added them to #17568 (moved).
-
Replying to cypherpunks:
This problem is still present in 5.0.6. GNU/Linux, x64, English locale.
How do I reproduce your issue?
Trac:
Status: reopened to needs_information -
Replying to gk:
Replying to cypherpunks:
This problem is still present in 5.0.6. GNU/Linux, x64, English locale.
How do I reproduce your issue?
Okay, I found a way to reproduce at least one issue:
- Open a new tab and go to https://bugzilla.mozilla.org/ (after it loaded the circuit should be visible)
- Choose Search -> Advanced Search
- Click on Detailed Bug Information and enter
[tor]in the Whiteboard row - Click on Search and after you got your results checking the circuit display does not show any curcuit anymore
Reloading does not help either. I am not sure if this is caused by our fixup done in this bug yet or whether that is an additional issue we overlooked so far.
Trac:
Keywords: tbb-torbutton tbb-circuit-display TorBrowserTeam201511R deleted, tbb-torbutton tbb-circuit-display TorBrowserTeam201516 tbb-5.5 added
Status: needs_information to assigned Replying to gk:
Replying to gk:
Replying to cypherpunks:
This problem is still present in 5.0.6. GNU/Linux, x64, English locale.
How do I reproduce your issue?
Okay, I found a way to reproduce at least one issue:
- Open a new tab and go to https://bugzilla.mozilla.org/ (after it loaded the circuit should be visible)
- Choose Search -> Advanced Search
- Click on Detailed Bug Information and enter
[tor]in the Whiteboard row - Click on Search and after you got your results checking the circuit display does not show any curcuit anymore
Reloading does not help either. I am not sure if this is caused by our fixup done in this bug yet or whether that is an additional issue we overlooked so far.
I found that browsing to the URL https://bugzilla.mozilla.org/buglist.cgi?a reproduces the error.
For each loaded document, the tor circuit display looks up the SOCKS user name and password. Normally it calls
getSOCKSCredentials(browser)which does the equivalent of:currentDocumentChannel.QueryInterface(Ci.nsIProxiedChannel).proxyInfoIt turns out that that the offending URL causes an nsIMultiPartChannel instead of an nsIHttpChannel to be associated with the document, so that
currentDocumentChannel.QueryInterface(Ci.nsIProxiedChannel)fails.But the main nsIHttpChannel can be found at nsIMultiPartChannel.baseChannel. So in that situation we need to use something like
currentDocumentChannel.QueryInterface(Ci.nsIMultiPartChannel).baseChannel.QueryInterface(Ci.nsIProxiedChannel).proxyInfoHere's a patch to do this: https://github.com/arthuredelstein/torbutton/commit/16990+1
Trac:
Status: assigned to needs_review
Keywords: tbb-torbutton tbb-circuit-display TorBrowserTeam201601 tbb-5.5 deleted, tbb-torbutton tbb-circuit-display TorBrowserTeam201601R, tbb-5.5 addedHi, I'm the [comment:9 punk from comment 9].
Replying to gk:
Okay, I found a way to reproduce at least one issue:
- Open a new tab and go to https://bugzilla.mozilla.org/ (after it loaded the circuit should be visible)
- Choose Search -> Advanced Search
- Click on Detailed Bug Information and enter
[tor]in the Whiteboard row - Click on Search and after you got your results checking the circuit display does not show any curcuit anymore
Reloading does not help either. I am not sure if this is caused by our fixup done in this bug yet or whether that is an additional issue we overlooked so far.
This is interesting, it is 100% reproducible for any site that serves Content-Type "multipart/x-mixed-replace", like bugzilla does for the "loading" animation.
However, note that in this case the circuit display doesn't get screwed up for the whole browser, only for that single document on that tab. But I think what I observed in 5.0.6 was the same behavior described in the OP (though now I'm doubting). Unfortunately I wouldn't know how to reproduce that.
Replying to arthuredelstein:
Here's a patch to do this: https://github.com/arthuredelstein/torbutton/commit/16990+1
Why
getSOCKSCredentialsForBrowserreturns 2 values as a single string, "user:pass" i.e. "host:nonce"? That host can include a port which then results in "domain:port:nonce". Afterwards there's alet domain = credentials.split(":")[0];to get the domain, which may be deliberate because later there's
domain.endsWith(".onion")so whoever wrote that expects not to have the ":port" part in "domain". But before there's
document.getElementById("domain").innerHTML = "(" + domain + "):";which results in the UI only showing the "domain" part and not the ":port" part. But the port is part of the isolation! So shouldn't the user see "domain:port" in the UI?
In any case I think it would be clearer to just return a structure not a compound string from
getSOCKSCredentialsForBrowser.Unrelated to that: the comment for
setupDisplaysays "Returns a function..." but that's not true, it doesn't return anything. I guess that's documentation bitrot.Replying to cypherpunks:
Thanks for the comments.
Why
getSOCKSCredentialsForBrowserreturns 2 values as a single string, "user:pass" i.e. "host:nonce"? That host can include a port which then results in "domain:port:nonce". Afterwards there's aWhile I don't think the host here will include a port, I agree it's safer not to use a colon. So I have changed it to a "|" character.
In any case I think it would be clearer to just return a structure not a compound string from
getSOCKSCredentialsForBrowser.Agreed.
Unrelated to that: the comment for
setupDisplaysays "Returns a function..." but that's not true, it doesn't return anything. I guess that's documentation bitrot.Yes, fixing that.
Here's an additional "code cleanup" patch with these changes, on the same branch:
https://github.com/arthuredelstein/torbutton/commits/16990+1
Replying to arthuredelstein:
While I don't think the host here will include a port, [...] You are right, sorry about the stupid rant.
PS: I now tried it and of course it doesn't include the port. Why was I so sure? I read the C++ definition of
getFirstPartyHostForIsolationand sawGetHostbeing called, this accessor seems to be automatically generated by Mozilla's C++-JS magic glue. So I read the documentation for "host" here: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIURI. Now, I usually prefer to browse with CSS disabled, especially when reading. If you disable CSS in that page you loose the table borders and so I ended up reading the description for "hostPort" instead. -_-"Replying to gk:
ln5 mentions that just using "New Tor Circuit for this Site" is working fine for him to reproduce the problem in the description. I wonder whether we overlooked something in our fix, hrm...
I tried this several times but I couldn't reproduce it. ln5, what platform and TBB version are you using? Any other clues?
Trac:
Cc: gk, arthuredelstein to gk, arthuredelstein, ln5Replying to cypherpunks:
Replying to gk:
[...]
This is interesting, it is 100% reproducible for any site that serves Content-Type "multipart/x-mixed-replace", like bugzilla does for the "loading" animation.
However, note that in this case the circuit display doesn't get screwed up for the whole browser, only for that single document on that tab. But I think what I observed in 5.0.6 was the same behavior described in the OP (though now I'm doubting). Unfortunately I wouldn't know how to reproduce that.
Alright, no more doubts: Right now on 5.0.7 the circuit display is borked exactly like described in the OP: For all tabs, for any document, "New identity" doesn't help, neither does "New circuit for this site".
Edit: I forgot to say, again I have no idea how it happened.
So the same problem exists and the one related to "multipart/x-mixed-replace" discovered by gk is a different one.
I will leave this browser like this in case you want me to try some live experiments. Note, though, that this is not a debug build nor anything like that. It's a normal 5.0.7 release version which (I think) has been auto-updated since 5.0.4 (or maybe even earlier).
