Tor SOCKS handshake makes SOCKS circuit isolation non-functional for many apps
Tor 0.2.3 is supposed to have SOCKS username+password isolation on by default. But with Pidgin and other apps, vidalia still shows circuits being shared between multiple apps using different SOCKS usernames and passwords.
I dug in with Wireshark, and it looks like the problem for Pidgin is that its SOCKS client handshake lists 2 "Client Authorization Methods": "No authentication" and "Username/password". Tor's SOCKS port replies that it only supports "No Authentication", so Pidgin doesn't send the username and password at all!
Tor should reply that it supports "Username/password" in this case if the SOCKS isolation feature is enabled.
Activity
changed milestone to %Tor: 0.2.4.x-final
Agreed wrt priority and backportability.
It looks easy enough to fix at first glance: answer "username/password" if the client offers it; otherwise answer "no auth". I'm attaching a patch to do that.
I'm a little worried that there could be a failure mode here where a user's application offers username/password authentication even though it doesn't know a username/password combination, and then responds to Tor's selecting username/password authentication by asking the user for a username and password. If there are many apps like that, we'll need another fix here.
This patch needs testing: first to ensure that username/password isolation is working with programs that behave like pidgin. And second, to make sure that the failure mode above doesn't occur when no username and password are configured.
Trac:
Status: new to needs_review
Keywords: N/A deleted, tor-client isolation added-
Trac:
8117_023_v2.patch The patch works properly on Tor's side. Wireshark reports Tor is now sending username/password in the response, and Pidgin sends the username and password, and then Tor says "success". Unfortunately, Pidgin immediately closes the SOCKS connection and then times out, seemingly forgetting what happened.
Other apps have similar issues. At least one began blasting data at the SOCKS port as soon as Tor said "username/password", which causes a warn about an improper SOCKS username/password version from Tor. Apparently it forgot that it still needed to actually send one as configured! What a mess.
Nothing has yet asked me for a password if I didn't provide one, though.
-
So, it sounds like the bug here is in pidgin and the other app (what was it?), not necessarily in Tor now. Or maybe Tor is screwing up here somehow, but not in a way wireshark can notice?
What would happen if Tor just stops saying that NO_AUTH is okay? That's not something we could do by default, but it could be a socksport-specific option.
-
So, I really want to do this change (if it makes a difference) but so far it sounds like that no tested applications are actually fixed by this fix so far. We should figure out:
- what apps we need to test
- whether this fix actually fixes any of them
- whether this fix actually breaks any of them.
Any takers there?
My opinion here is that it's actually good to break apps that are doing SOCKS u+p wrong, so long as we don't also break those apps when SOCKS u+p is not set. We really shouldn't tell users "Hey, use SOCKS u+p to isolate your apps!" and then not actually isolate anything because our handshake silently tells the app not to use u+p.
Also, if there are actually bugs in Pidgin/whatever's SOCKS u+p support, it's unlikely those will ever get fixed if our handshake always tells Pidgin not to use SOCKS u+p.
Adding ioerror to Cc because I think he's worked improving on Pidgin's SOCKS implementation for Tor in the past.
Trac:
Cc: N/A to ioerrorI would say that we should ensure that torsocks has isolated circuits/streams at all times. Pidgin should also have a bug opened about this issue, has anyone opened an issue or can someone detail their test case? I'll repro and file a bug if someone who best understands it details their experience...
I was working on #8053 (moved) and noticed Tor was preferring NoAuth over u+p, luckily asn pointed me here. Both Tor and Torsocks perform as expected with patched Tor and the byte stream doesn't appear to have any artifacts or oddities, so I think this patch is good to go.
-
(Let's try this in 0.2.4 first, then backport. Anything else we can test it on? Right now, I am seeing exactly two test reports above. I have no idea what it might break.)
Trac:
Keywords: tor-client isolation deleted, tor-client isolation 023-backport added
Milestone: Tor: 0.2.3.x-final to Tor: 0.2.4.x-final Replying to nickm:
(Let's try this in 0.2.4 first, then backport. Anything else we can test it on? Right now, I am seeing exactly two test reports above. I have no idea what it might break.)
I agree. It's sad that this may break some apps but it is a reality, so a testing phase will be good. I'll also check a few other popular apps and get patches rolling if necessary. If someone can provide steps to reproduce the pidgin failure for ioerror that'd be awesome, too.
-
To be clear: I don't think we should merge this if it will break things that work today for a substantial number of users. I've tried the "break the nonconformant apps, and write patches as needed" approach to making only-nonfonformant-apps-will-break changes in the past, and it just mades for a huge pile of pain.
If this patch breaks something significant, the right fix is IMO to have it be enabled or disabled on a per-SOCKS-listener basis, so that if you need to run any applications that can't handle this fix, you can point them at a SOCKS port that doesn't have this fix turned on. It'll be more code, but potentially far less effort than upstreaming a big pile of fixes and/or upsetting a big mass of users.
This works well, at least for a non-broken app. I was initially uncertain about Tor only respecting socks_prefer_no_auth if NoAuth was send by the client. I'm wonder if there are any commonly used and Socks5-broken apps that say they only support u+p if they are configured with a username and password. If this was the case, then Tor would still reply to use u+p because NoAuth was not a client-supported method. There's not much Tor can do in this case, though, if the app doesn't know how to handle the response.
On a different note, how do you feel about creating isolated circuits for connections from apps that said they support u+p but Tor responded with NoAuth because PreferSOCKSNoAuth was set?
The backport here looks pretty big, and pretty feature-like (even if we thought the feature was already present). So I'm inclined to say we should skip it for 0.2.3.
That said, this is a good one to get weasel's opinion on to confirm: weasel, what would you think of trying to talk your Debian people into taking this fix in whatever debian ships 0.2.3 currently?
Trac:
Cc: ioerror to ioerror, weasel
Debian wheezy will, at least initially, ship with Tor 0.2.3.25 unless something really huge shows up yesterday.
The patch isn't actually that long, so I'm less owrried about patch size. What I don't like about this is that it's changing Tor's default behavior in what is then the stable tree (of both Tor and Debian). If this was forced onto us as part of a 0.2.3.26 that had lots of things we wanted we'd probably take it however.
-
If we're going to leave this broken in 0.2.3.x, please remove IsolateSOCKSAuth from the manpage, and deprecate the torrc option in config.c, so we tell users who try to use it that it's not supported and Tor exits if they try to use it.
It's really bad form to let people think they're getting a security bonus by using an option we decided to let remain silently broken..
Trac:
Status: needs_review to needs_revision -
Replying to mikeperry:
My opinion here is that it's actually good to break apps that are doing SOCKS u+p wrong, so long as we don't also break those apps when SOCKS u+p is not set. We really shouldn't tell users "Hey, use SOCKS u+p to isolate your apps!" and then not actually isolate anything because our handshake silently tells the app not to use u+p.
Turns out pidgin offers the username of "" and password of "", when Tor takes pidgin up on its offer to provide u+p. That is, pidgin says it knows how to provide u+p auth, and Tor says yes please, even when the Username and Password boxes in the pidgin UI are blank.
So, pidgin doesn't work at all with Tor 0.2.4.12-alpha. Filed as #8879 (moved).
-
Replying to arma:
So, pidgin doesn't work at all with Tor 0.2.4.12-alpha. Filed as #8879 (moved).
I now believe this (bug #8879 (moved)) is Tor's fault, not pidgin's fault.
-
The commits in nickm/bug8117_023 were a264c4fedab87ce7c8cbb94632657a90e95e7a4e and fa3c23773944788125db2f67bcb048376c17fec9. They got merged in Tor 0.2.4.12-alpha. Their changes entry was:
+ + - Many SOCKS5 clients, when configured to offer a username/password, + offer both username/password authentication and "no authentication". + Tor had previously preferred no authentication, but this was + problematic when trying to make applications get proper stream + isolation with IsolateSOCKSAuth. Now, on any SOCKS port with + IsolateSOCKSAuth turned on (which is the default), Tor selects + username/password authentication if it's offered. If this confuses your + application, you can disable it on a per-SOCKSPort basis via + PreferSOCKSNoAuth. Fixes bug 8117; bugfix on 0.2.3.3-alpha.I would be glad to take a documentation fix in 0.2.3. Anybody want to write one?
mentioned in issue #8879 (moved)
moved to tpo/core/tor#8117 (closed)
mentioned in issue tpo/core/tor#8879 (closed)