Provide a control port command to query the circuit used for SOCKS u+p
Once we start isolating streams by domain in the browser, it will be useful to have a way to ask Tor what circuit it is currently using for a given SOCKS username+password, so we can provide a tooltip or other indication directly in the browser UI.
Activity
changed milestone to %Tor: 0.2.6.x-final
See my branch isolation-debug-v4 (
https://git.torproject.org/rransom/tor.git isolation-debug-v4).Trac:
Status: new to needs_revision(Some of the reasons that I didn't get that branch merged at the time were (a) its output format was ugly, and I didn't want it carved into stone in control-spec.txt for all time; (b) its output complied with the spec for extended events (see §4.1), but the Python crap can't handle a QuotedString containing spaces; (c) I never got around to dumping isolation values for circuits, only streams; (d) the details of the stream-isolation code should be unspecified and subject to change.)
(Also, that branch predates
smartlist_add_asprintf.)Trac:
Cc: N/A to arthuredelstein@gmail.com
Does this actually work? It will tell you values for those fields (but not the other isolation fields) if a circuit is first created with isolation set... but I don't think it will tell you when an existing circuit gets an isolation field set.
Perhaps it makes more sense to attach ISO_* fields to STREAM events, and then to watch both STREAM events and ATTACHSTREAM events? Would that work for you? If so, I think rransom's approach above is better in that case.
-
Another way to do this might be to provide a query to ask Tor which circuit was used for a given SOCKS source port.
Well, you can get this info now by watching STREAM and ATTACHSTREAM events, I believe. (Would you even need to see isolation info for that?)
Trac:
Status: needs_review to needs_revision Replying to nickm:
Does this actually work? It will tell you values for those fields (but not the other isolation fields) if a circuit is first created with isolation set... but I don't think it will tell you when an existing circuit gets an isolation field set.
Perhaps it makes more sense to attach ISO_* fields to STREAM events, and then to watch both STREAM events and ATTACHSTREAM events? Would that work for you? If so, I think rransom's approach above is better in that case.
OK, I've posted a patch based on rransom's approach. Does this seem like the right thing now? Thanks for your help.
Trac:
Status: needs_revision to needs_reviewReplying to arthuredelstein:
OK, I've posted a patch based on rransom's approach. Does this seem like the right thing now? Thanks for your help.
You must not output the SOCKS4 auth string without escaping it. Either use
esc_for_log_len(and add it if it hasn't already been added to Tor somewhere) like I did or usebase16_encode.Remember that some people will think that a hex-encoded string is encrypted. At the very least, be aware that hexifying strings makes it harder for a human to read the control-port output.
Consider dynamically allocating the hex-encoding buffers for SOCKS5 auth strings, or at least not allocating a full kilobyte on the stack -- you're about to
smartlist_add_asprintfthe contents anyway, so 512 bytes of buffer should be enough.Remember to update
control-spec.txtto document at least what is actually being used by other applications.Trac:
Status: needs_review to needs_revision-
Does this seem like the right thing now?
Before I review this one, I'd like to hear from the TBB team about whether this general approach meets their needs. I don't think I've seen an answer there. I'm happy with an events-based approach, but the whole point of this ticket is to improve TBB usability, so I'd like to know whether we're actually going to do that before we merge.
Replying to rransom:
Thanks for looking this over!
You must not output the SOCKS4 auth string without escaping it.
Stupid mistake. Fixed.
Either use
esc_for_log_len(and add it if it hasn't already been added to Tor somewhere) like I did or usebase16_encode.At the very least, be aware that hexifying strings makes it harder for a human to read the control-port output.
Remember that some people will think that a hex-encoded string is encrypted.
Yes, I originally wanted to use esc_for_log, but it doesn't currently escape all possible dangerous characters. Examples include = and \space. A client with a good parser that correctly recognizes a quoted string likely won't have any problem, but I didn't want to inadvertently break any existing naive parsers. So what do you think is the best option? (1) Use esc_for_log as is and assume good client parsers, (2) make esc_for_log safer, or (3) use base16_encode?
Consider dynamically allocating the hex-encoding buffers for SOCKS5 auth strings, or at least not allocating a full kilobyte on the stack -- you're about to
smartlist_add_asprintfthe contents anyway, so 512 bytes of buffer should be enoFixed.
Remember to update
control-spec.txtto document at least what is actually being used by other applications.Will do, once we settle on a final patch.
I've now posted a new version with the fixes mentioned.
Replying to nickm:
Does this seem like the right thing now?
Before I review this one, I'd like to hear from the TBB team about whether this general approach meets their needs. I don't think I've seen an answer there. I'm happy with an events-based approach, but the whole point of this ticket is to improve TBB usability, so I'd like to know whether we're actually going to do that before we merge.
To summarize: I'm working on #5752 (moved) (isolate streams by URL bar domain) and descendants. I have implemented a patch for #3455 (moved), where the browser is using a unique SOCKS username and password (u+p) for each URL bar domain, and tor accordingly sets up a separate circuit for each unique u+p. The next step (#8641 (closed)) is to implement a UI, attached to the browser's URL bar, that shows the progress of a circuit as it's being built, and the exit IP of that circuit. So we need to be able to query/monitor tor for the circuit ID corresponding to the u+p.
At this point, I don't expect to need any isolation flags apart from SOCKS u+p. So we could remove the other isolation flag stuff from the patch. But, given the work Robert was doing, I imagined you might want all isolation flags covered, for the sake of completeness. I'd be glad to hear your opinion either way.
In the meantime I can start working with the TBB team on a design and prototype for the #8641 (closed) UI, which may help solidify exactly what we need in the patch for this ticket. Any suggestions are much appreciated!
nickm: I've now posted a patch for #8641 (closed) that provides a Tor circuit display UI and requires no patching of Tor. The #8641 (closed) patch works by assuming that (given my patch from #3455 (moved)) the first STREAM connection for each new circuit is to the URL bar domain. So I monitor CIRC and STREAM events and display a mapping from domain to circuit nodes.
But, of course, we only receive the first STREAM event after the circuit is already built. It would be nicer to show the circuit as it is extended node by node. For this purpose, we would need to add USERNAME and PASSWORD parameters to the CIRC events (at least to the original CIRC LANUCHED event. Would you be open to such a patch?
Trac:
Status: needs_revision to needs_information
Trac:
Cc: arthuredelstein@gmail.com to arthuredelstein@gmail.com, mcsIf this is any help, my latest version of the path is posted above. (Please ignore older versions.) This patch prints SOCKS_USERNAME and SOCKS_PASSWORD parameters in circuit status events. If this patch or a similar one lands, I will update my patch for #8641 (closed) to make use of these flags.
At a glance, I think this is useful. I am a little worried that we may end up needing more info, because the STREAM NEW events don't have a circuit id yet, and #8641 (closed) also has the assumption that the first STREAM event on a new circuit is the top-level domain (which is not correct).
So, in short: yes, this is what we want the control port to do with SOCKS u+p. But we may also want it to do more, too.
Most likely though we won't finalize this until October, due to the Firefox 31 rebase work.
Trac:
Keywords: TorBrowserTeam201408, MikePerry201408R deleted, TorBrowserTeam201410 added-
We took this patch for the Tor in TBB 4.5-alpha-1. Will let you know how it works out with the rest of our username+password isolation.
Trac:
Parent: #5752 (moved) to N/A -
16:52 < arthuredelstein> nickm: The #8405 patch is currently being applied to the tor included in TB 4.5-alpha builds: https://gitweb.torproject.org/builders/tor-browser-bundle.git/tree/gitian/descriptors/linux/gitian-tor.yml#n79 16:53 < nickm> arthuredelstein: is it working well? Is it useful? 16:55 < arthuredelstein> I would say it is working well and is useful. We are using it for the Tor circuit display, which shows the Tor circuit for each URL bar domain. 16:55 < nickm> arthuredelstein: ok. May I quote you on the bugtracker? :) mentioned in issue #8641 (closed)
mentioned in issue #13672 (moved)
mentioned in issue #14526 (moved)
mentioned in issue #14555 (moved)
mentioned in issue #15000 (moved)
moved to tpo/core/tor#8405 (closed)
mentioned in issue tpo/core/tor#15000 (closed)
mentioned in issue tpo/applications/tor-browser#14526 (closed)