add ClientConnectPolicy config option
There should be a config option called ClientExitPolicy which specifies which destinations clients are allowed to build circuits to. The default value should be "accept :".
This will deprecate the RejectPlaintextPorts option as it will be able to provide the same functionality.
I am beginning to work on a patch for this.
Activity
changed milestone to %Tor: unspecified
Sounds like a good idea. We can talk about the name some more once the patch is done.
The code could probably reuse the existing policies.c logic. If this is going to be port-based only, it should use the shortpolicy logic from that module.
(I'm not sure how you could reasonably make this cover address and port, since we don't know what the IP of a hostname will be before we have made a connection.)
Trac:
Keywords: N/A deleted, tor-client added
Milestone: N/A to Tor: unspecifiedThe patch above is my very first for tor, and the most C I've written in a while, so I'm looking forward to hearing what I've done wrong :)
One thing I don't understand is what the expression
!conn->use_begindir && !conn->chosen_exit_name && !circinconnection_edge.cis for - I just copied that from the consider_plaintext_ports code directly above the code I inserted. (Under which circumstances would that expression be expected to evaluate to false?)I uploaded a second patch for review. I think this version is good, but I still don't understand the purpose or effect of the
!conn->use_begindir && !conn->chosen_exit_name && !circexpression I copy+pasted fromconsider_plaintext_ports.(Btw, the problem with the first patch was that I wasn't checking the result of
tor_addr_parse, so when a hostname was parsed an uninitializedaddrwas being considered. The confusing thing was that, on my system, the uninitializedaddractually did match thereject *:*policy, which caused me to believe the patch worked. But when someone else tested it, on their system tor would log a warningtor_addr_is_null(): Bug: Called with unknown address familyand would then fail open because the uninitialized addr didn't match any policy.)-
Hm. After looking at this, I don't think I understand why you're doing this with full addresses, and not just ports.
In other words, if the user allows "1.2.3.4/80", and then Tor receives a SOCKS connection for "www.example.com:80", should the code allow the request to be made or not? Keep in mind that a BEGIN cell does a lookup and a connect in one step: Tor won't know whether www.example.com resolved to 1.2.3.4 until the connection is made. With this patch, I think the answer will depend on whether the user said to allow 0.0.0.0, which can't really be the right behavior.
Given that address-based rules don't work the way that users might expect here, are we losing anything important by having this be address-and-port based rather than port-based alone?
Replying to nickm:
Hm. After looking at this, I don't think I understand why you're doing this with full addresses, and not just ports.
I mostly want this feature for port-based policies, but I included IP address support too because the policy parser does it already (so it was easy) and also because I think address-based policies might actually be useful in some cases. For instance, say I've got an system that I expect should only connect to 1.2.3.4:22 without using a hostname, I could have a ClientConnectPolicy of "accept 1.2.3.4:22, reject :". Attempts to connect to hostnames that resolve to 1.2.3.4 will be rejected, but connections to the IP will be allowed.
I'd rather have hostname support too, but this would be complicated because (a) the policy parser would need to support hostnames, and (b) applying IP-based policies to hostname-based connections isn't really possible due to what you say below. So, I figured that documenting Connections using hostnames will only be matched by policies with "*" as their IP address was a reasonable middle ground. Perhaps that could be said in a clearer way?
In other words, if the user allows "1.2.3.4/80", and then Tor receives a SOCKS connection for "www.example.com:80", should the code allow the request to be made or not? Keep in mind that a BEGIN cell does a lookup and a connect in one step: Tor won't know whether www.example.com resolved to 1.2.3.4 until the connection is made. With this patch, I think the answer will depend on whether the user said to allow 0.0.0.0, which can't really be the right behavior.
Correct, but I did think that would be OK behavior if sufficiently documented (which perhaps it isn't).
Given that address-based rules don't work the way that users might expect here, are we losing anything important by having this be address-and-port based rather than port-based alone?
I assume you mean the opposite, would we losing anything by having this be port based alone? We'd just be losing the ability to have IP-based policies which don't apply to hostname-based connections. That functionality was not my primary objective, but I do think it could be useful in some cases.
Let me know if you think the docs should/could be clarified about this, or if you want me to make a solely port-based version of the patch.
-
Sorry, I missed that in the documentation. That's what I get for focusing on the code.
If we're okay with saying that connection-by-hostname is only matched by *, that's plausible. But your code doesn't do that, I think. In the patch you wrote, if I read it right, connection-by-hostname is matched by *, and by any other pattern that matches 0.0.0.0, which seems like a weird interface. Can we make it so that only * matches?
moved to tpo/core/tor#11010 (closed)