prop351: Define two variants for the socks extensions
Having these as separate variants makes it easier for C tor to ignore the existence of RPC.
Note that the C implementation merged under tor!828 (merged) is still conformant under this change.
Merge request reports
Activity
requested review from @Diziet
assigned to @nickm
mentioned in merge request arti!2401 (merged)
85 84 86 The Password field is stream isolation parameter. 87 If it is empty, the stream isolation parameter is an empty string. 85 The Password field is stream isolation parameter. 86 If it is empty, the stream isolation parameter is an empty string. 87 88 * When the format type is `[31]` (the ascii encoding of `1`), 89 we interpret the rest of the Username and field and the Password field 90 as follows: 91 92 The remainder of the Username field encodes an RPC Object ID. 93 It must not be empty. 94 95 The Password field is stream isolation parameter. 96 If it is empty, the stream isolation parameter is an empty string. 97 -
This (together with the rest of the document) implies that
- format type 31 and format type 30 SOCKS requests can share circuits
- format type 30/31 cannot share circuits with legacy Tor-style isolation authentication
Is that your design intent?
Does C Tor (or, for that matter, Arti) really implement different isolations for SOCKS4-with-no-user-id vs SOCKS5-with-no-authentication vs SOCKS5-with-username/password ?
This all seems a bit odd?
-
One of the oddest parts of this is that clients that don't supply isolation inforamtion (so presumably don't care and don't want isolation) end up being isolated according to their choice of precise SOCKS protocol, which seems not relevant (and indeed undesirable, since we want to share circuits when we don't have a reason not to).
My apologies in advance for the firehose of response to what probably seemed like a straightforward question. :)
- format type 31 and format type 30 SOCKS requests can share circuits
- format type 30/31 cannot share circuits with legacy Tor-style isolation authentication
Is that your design intent?
Yes and no. :)
It is true that the isolation strings of 31 and 30 would allow them to share circuits. So that's the "yes".
But isolation strings are not the only parameter on which streams are isolated. Since 31 always has an RPC object whereas 30 uses the default TorClient, a 31 stream will always be on a TorClient that is isolated from the default TorClient. Based on that, they won't share streams in practice.
Further, there is no real way for format type 30/31 to share have the same isolation as legacy SOCKS5 isolation: Legacy socks5 isolation has a a username/password tuple. This has only a string. (Now, we could say that the isolation string here corresponds to the concatenation of the username/password, or to a socks5 username with no password set, but that seems like even more complication.)
Does C Tor (or, for that matter, Arti) really implement different isolations for SOCKS4-with-no-user-id vs SOCKS5-with-no-authentication vs SOCKS5-with-username/password ?
Yes. For a needlessly full overview see proposal 171, and the part of the Tor manpage under "SocskPort" describing "isolation flags".
(aside 1: There is technically speaking no such thing as SOCKS4 with no user ID, since the user field is always present, though it can be an empty string.)
(aside 2: we should definitely merge more of this information into the tor specifications proper! Let's open a ticket for that...)
One of the oddest parts of this is that clients that don't supply isolation inforamtion (so presumably don't care and don't want isolation) end up being isolated according to their choice of precise SOCKS protocol, which seems not relevant
FWIW, IIRC SOCKS4 vs SOCKS5 isolation has been on by default since we implemented proposal 171.
(and indeed undesirable, since we want to share circuits when we don't have a reason not to).
Again, yes and no! We do want to share circuits when we don't have a reason not to.
But part of the original job of stream isolation, though, was to try to isolate circuits from different applications, including naive applications that didn't even know about stream isolation. So we didn't, classically, fully trust applications to tell us their isolation preferences, and we looked at other random junk (including socks version) to try to tell different apps apart. And thus we have treated different SOCKS protocols as signalling different applications that should therefore be isolated.
So that's a bit kludgey! Prop 171 has some decade-plus-old thinking here.
A meta-question to hopefully move this forward:
- How much of this do you think we should untangle before we can merge this particular MR? I think I would be satisfied with describing the semantics this particular feature and getting them the way we want.
- Do you think we might need a full re-think of what isolation does and how it works? And if so, do you think that blocks anything else?
- I personally think we should document the current SOCKS isolation behavior in more detail somewhere in the spec, and incorporate more of the information from prop171 and the tor manual. Do you agree? If so, do you think it makes sense to do so before or after we figure out any big re-think?
added Project 119 label
-
Gosh. Thanks for the explanation. I do not think we should revisit prop 171.
It is true that the isolation strings of 31 and 30 would allow them to share circuits. So that's the "yes". [but] a 31 stream will always be on a TorClient that is isolated from the default TorClient. Based on that, they won't share streams in practice
This is very confusing! They fail to share sort of by accident, even though it's our intent that they fail to share.
How about we say here in this spec that 30 and 31 don't share circuits?
(Note that otherwise, it might be valid for Arti to implement a method to get a client-like object whose 31 streams share with general, 30, streams. It seems like you're definitely not intending that, at least for 31.)
And can we add a cross-reference to prop 171 ?
-
I've opened #269 to improve the torspec isolation documentation, and made the other requested changes. Thanks!
mentioned in issue arti#1423
added 1 commit
- 34921a10 - prop351: additional clarifications about isolation
requested review from @Diziet
mentioned in commit ec992b65
mentioned in commit arti@121f43d8
mentioned in commit arti@7217bc80