Opened 7 years ago

Closed 2 months ago

#4700 closed enhancement (implemented)

Tor should provide a mechanism for hidden services to differentiate authorized clients and circuits

Reported by: katmagic Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-control, needs-proposal, tor-hs, needs-design, 035-roadmap-master
Cc: special, ahf, mahrud Actual Points:
Parent ID: Points: 10
Reviewer: dgoulet Sponsor:

Description

Tor allows a hidden service to require that clients provide a secret key prior to connecting. Even though hidden services support the use of multiple keys, information mapping the key specified to the specific connection is never exported.

Child Tickets

Change History (33)

comment:1 Changed 7 years ago by nickm

Keywords: needs-proposal added
Milestone: Tor: unspecified

Good idea; will need a proposal.

comment:2 Changed 6 years ago by nickm

Keywords: tor-hs added

comment:3 Changed 6 years ago by nickm

Component: Tor Hidden ServicesTor

comment:4 Changed 4 years ago by special

Cc: special added
Summary: Tor should provide a mechanism for hidden services to differentiate authorized clients.Tor should provide a mechanism for hidden services to differentiate authorized clients and circuits

Any solution to this should also differentiate between different circuits.

comment:5 Changed 4 years ago by arma

Not quite the same thing, but an intriguing direction anyway:

https://lists.torproject.org/pipermail/tor-dev/2014-March/006576.html

"""
I've written this (ugly, unconfigurable) patch for Tor which is designed to allow hidden services more information about their users, by giving each inbound circuit its own temporary "IP address" in the 127.x range. This technique works on Linux (I've not tried it on anything else) and allows the application server to do some useful things which were previously difficult:

  • Identify TCP connections coming from the same client, in a short space of time, for example, for diagnostic log analysis, identifying traffic trends
  • Rate-limit operations coming from the same client, to defend against some types of DoS attacks
  • Temporarily block abusive clients (at least, until they make a new Tor circuit)

More importantly, it can do this with an unmodified application-server (e.g. web servers typically have these features built-in) because it effectively "spoofs" the client ID as an ip-address, in the 127.x range.
"""

comment:6 Changed 3 years ago by dgoulet

Keywords: hiddenServices removed
Points: medium
Severity: Normal

comment:7 Changed 18 months ago by dgoulet

Keywords: tor-control added; control removed

comment:8 Changed 18 months ago by nickm

Keywords: needs-design added
Points: medium10

comment:9 Changed 5 months ago by ahf

Cc: ahf added

comment:11 Changed 5 months ago by nickm

Keywords: 035-proposed added

comment:12 Changed 5 months ago by arma

ahf, do I remember that you had a patch for this one? Is it nearly-ready-but-sitting-in-a-mailbox-somewhere?

comment:13 Changed 5 months ago by arma

See also #24299 which looks very related.

comment:14 Changed 5 months ago by ahf

arma, yes, there is indeed a very early patch in https://github.com/ahf/tor/commits/ahf/wip-ascii-tcp-proxy - this was shared with our contact at CF to test.

It encodes our 32-bit global identifier on the circuit as two 16-bit in the HA ASCII proxy protocol.

The patch needs some torrc options before being worthwhile to include.

comment:15 Changed 5 months ago by asn

Hmm, yes indeed we want some torrc options. Perhaps we can introduce a string torrc option HSClientIdentifierMethod which takes the proxy value (or haproxy) value for the proxy protocol of comment:10? And then in the future perhaps we can introduce other methods, like client_auth which returns the name of the client visiting, etc.?

Also as you pointed out, the current patch will do the protocol also for regular exit conns. We should check that the edge_connection_t has either hs_ident or rend_data` before doing the protocol.

comment:16 Changed 5 months ago by mahrud

Cc: mahrud added

Sorry I forgot to give any feedback for this. It worked well! At some point in the future it might be a good idea to implement the v2 protocol as well.

Regarding torrc options:
Can you also add an option for encoding the circuit ID in the port or in the source IP?For our specific application using the last 32 bits of a private ipv6 subnet (like fc00::/7) is ideal for two reasons:

  1. This is a large private subnet, so we don't accidentally collide with anyone else's IP.
  2. The rest of the pipeline can simply look at that IP and pretend everything is normal, no need to implement special logic to parse the port numbers.

The only requirement is to implement a proxy protocol server in the normal pipeline, which is already done.
Perhaps HiddenServiceExportCircuitID proxy port for ahf's implementation and proxy srcIP fdXX:XXXX:.../96 for my suggestion?

comment:17 Changed 5 months ago by arma

This is a sufficiently narrow use case that I think we should try to limit ourselves to one torrc option, which turns it on or not, and one behavior that happens when you turn it on.

comment:18 Changed 5 months ago by mahrud

In that case I would prefer HiddenServiceExportCircuitID 1 to choose a random /96 subnet of [fc00::/7] for each hidden service and encode circuit ID in the source IP. I'm working on a Caddy plugin that takes this information and hands it over to the server, so anyone can use this feature. I think returning accurate values for the ports is probably best, but I'm not sure what the destination IP should be.

comment:19 Changed 4 months ago by nickm

Keywords: 035-roadmap-proposed added; 035-proposed removed

comment:20 Changed 4 months ago by ahf

Keywords: 035-roadmap-master added; 035-roadmap-proposed removed

comment:21 Changed 2 months ago by asn

Status: newneeds_review

OK I worked on this. Please see my branch bug4700_rebase:
https://github.com/torproject/tor/pull/327

It does various things:

  • Saves up original virtual port.
  • Adds torrc option.
  • Puts stuff in functions and documentation.
  • Unittest and changes file.

Please review and test if possible. I've done rudimentary basic testing but I don't know how to test this feature more deeply (i.e. actually use the results of the proxy protocol).

comment:22 Changed 2 months ago by dgoulet

Reviewer: dgoulet
Status: needs_reviewneeds_revision

Some questions that *might* require changes.

comment:23 Changed 2 months ago by asn

Status: needs_revisionmerge_ready

OK, after review and revisions from dgoulet and ahf I present the final merge_ready branch:
https://github.com/torproject/tor/pull/343

I'm also gonna send this to mahrud so that he can check it out.

comment:24 Changed 2 months ago by ahf

Found another bug with this code:

$ nc -l -p 6667
CAP LS
NICK user
USER user user rxyw2yu2mxczblqcov5d3m6fqyem7rnamcean46wu7srdh5so7dro7qd.onion :Unknown
PROXY TCP6 fc00:dead:beef:4dad::0:25 ::1 37 6667

This happens when an IRC client is connecting to an OS with this feature enabled. We need to make sure the PROXY string is added to the beginning of the buffer and not the end.

comment:26 in reply to:  25 Changed 2 months ago by asn

Replying to ahf:

https://github.com/ahf/tor/commit/3477a73af99eb72f8374928fdc2fab4858485219 patch for the above problem.

Thanks. Cherry picked it into bug4700 branch.

comment:27 Changed 2 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.6.x-final

Because of the timeframe, calling this "0.3.6", but please let me know if I'm wrong.

comment:28 Changed 2 months ago by asn

Milestone: Tor: 0.3.6.x-finalTor: 0.3.5.x-final

Changing back to 035 after discussion on IRC with Nick.

This is not a huge priority ticket so if the release gets overwhelmed with features we can potentially move this to 036 if needed.

comment:29 Changed 2 months ago by nickm

Status: merge_readyneeds_revision

Hi! This patch looks good.

Three things I think we should do here:

  • I think that the configuration option should accept "none" in addition to "haproxy".
  • We should link to the spec for this protocol, in the code and in the manual, and explain which version we support.
  • Are we exposing the 'global_identifier' field for an important reason, or is it just important that we expose _some_ unique value? If it's the latter case, instead of putting the 'global_identifier' into the IPv6 address and source port directly, I think we should hash them first, possibly with siphash. It's not that these values are very sensitive, but I don't want anybody depending on the actual global_identifier layouts from Tor unless we're exposing them intentionally. (But if we are exposing them intentionally, we should document that.)

One thing for the future, or maybe I don't understand this:

  • Is there some intended way for programs to tell whether a user's circuit is authenticated, and if so to which user?

comment:30 Changed 2 months ago by nickm

After talking with ahf, I think we shouldn't hash the global_identifier, but we should document that we use it.

comment:31 Changed 2 months ago by ahf

Added two patches to https://github.com/ahf/tor/commits/asn/bugs4700_2 to fix documentation and support of the 'none' config option.

comment:32 Changed 2 months ago by asn

Status: needs_revisionmerge_ready

LGTM!

comment:33 Changed 2 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged!

Note: See TracTickets for help on using tickets.