Opened 17 months ago

Last modified 5 months ago

#22689 needs_revision defect

hs: Stop intro points being used as single hop proxies

Reported by: teor Owned by: dgoulet
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: relay-safety, 033-triage-20180320, 033-removed-20180320
Cc: Actual Points:
Parent ID: #17945 Points: 0.5
Reviewer: teor Sponsor:

Description (last modified by teor)

This prevents them knowing both the service and client IP addresses, and therefore being targets for network traffic logging, sybil, or hacking attacks.

We need to implement the following checks:

  • if an introduction point was made using a direct connection (single onion services), refuse direct client connections,
  • for v3 intro points, always refuse direct client connections
  • for v2 intro points, refuse direct client connections based on a consensus parameter
  • if the rend point was made using a direct connection (custom client, no tor2web for HSv3), refuse direct service connections (single onion services).

See #22688 for how this is done for HSDir3s using channel_is_client(). The comments in that patch explain why it works.

We could even refactor the common code out of connection_dir_is_anonymous() into connection_is_anonymous(), and avoid including channel[tls].h into directory.c.

I'm not sure if I will get time to do this, so please feel free to take this ticket.

Child Tickets

Change History (26)

comment:1 Changed 17 months ago by teor

We can do this for HSv2 rend points when we implement this patch, because single onion services retry a 3-hop path on failure.

But we can't fix HSv2 intro points, because tor2web doesn't retry a 3-hop path (#19662).

comment:2 Changed 17 months ago by teor

Description: modified (diff)
Status: newneeds_revision

See cddff59c0 in my branch bug22688-031 for code that might work for OR circuits.
(I accidentally wrote the OR version rather than the directory version.)

comment:3 Changed 17 months ago by teor

Owner: set to teor
Status: needs_revisionassigned

comment:4 Changed 17 months ago by teor

Status: assignedneeds_revision

comment:5 Changed 15 months ago by dgoulet

Status: needs_revisionneeds_review

Hrmm I think that should be in needs_review.

comment:6 Changed 15 months ago by teor

Status: needs_reviewneeds_revision

No, it really does need revision. And someone needs to actually *run* it.

comment:7 Changed 15 months ago by teor

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

I'm not going to get time to do these in 0.3.2.
Moving them to 0.3.3.

comment:8 Changed 10 months ago by teor

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

Moving most of my assigned tickets to 0.3.4

comment:9 Changed 9 months ago by teor

Status: needs_revisionneeds_review

Latest code in #17945

comment:10 Changed 9 months ago by dgoulet

Keywords: prop224 removed
Milestone: Tor: 0.3.4.x-finalTor: 0.3.3.x-final
Owner: changed from teor to dgoulet
Status: needs_reviewassigned
Summary: prop224: Stop rend and intro points being used as single hop proxieshs: Stop rend and intro points being used as single hop proxies

Changing ticket title to reflect that it now applies to all HS version. Then moving it to 033 and I'm taking it, I'll submit a branch soon. We are splitting things out of #17945 so we don't get too confused.

comment:11 Changed 9 months ago by dgoulet

Status: assignedneeds_review

Branch: ticket22689_033_01

It is not that small because I did an HS specific interface but it is as simple as the original patch.

comment:12 Changed 9 months ago by teor

Reviewer: teor

I'll review this later today, after I get some Rust done.

Do we want a consensus parameter to block Tor2web at Intros, like the one at Rendezvous?
I think the answer is "yes, but not on by default, and not on right now, and maybe just in 0.3.4".
I opened #25371 to do it in a separate task.

comment:13 in reply to:  12 Changed 9 months ago by dgoulet

Replying to teor:

I'll review this later today, after I get some Rust done.

Do we want a consensus parameter to block Tor2web at Intros, like the one at Rendezvous?
I think the answer is "yes, but not on by default, and not on right now, and maybe just in 0.3.4".
I opened #25371 to do it in a separate task.

Yes I think ultimately (hopefully 034 imo), we come down to rejecting single hop client for any part of the HS dance (single onion or not). Lets open a ticket for that at the RDV?

I've modified the branch based on asn's comment to merge the suitable functions into one. I've renamed the function also to reflect a bit more what it is doing.

See fixup 216c754e6f.

comment:14 Changed 9 months ago by asn

Patch LGTM.

Are we sure this is not gonna make single onion services (and tor2web clients) go crazy with reconnecting and end up hurting the network?

comment:15 in reply to:  14 Changed 9 months ago by dgoulet

Replying to asn:

Are we sure this is not gonna make single onion services (and tor2web clients) go crazy with reconnecting and end up hurting the network?

In theory, because the circuits are closed at the RP with the "TORPROTOCOL" reason, the client should pick a new rendezvous point. However, there is a good chance the tor2web client will just loop over all RPs it can find and try them all in a loop. That would need to be tested.

comment:16 Changed 9 months ago by teor

v2 Intro:

v2 Tor2web will extend to another intro point, and then succeed because it's no longer a single hop path.

v2 Rendezvous:

v2 single onion services retry failed rendezvous with a 3-hop path, so they should retry once.
But we turned off retries to try to reduce the DDoS load.
So I guess they will just fail.
And then v2 Tor2web will try another rendezvous point, and eventually give up.
But we turned off Tor2web at rendezvous points.

So we should use one of these combination of rendezvous retry and Tor2web settings in the consensus:

  • banning Tor2web works, or
  • allowing Tor2web and at least 1 retry also works.

But allowing Tor2web and with no retries will put extra load on the network from Tor2web / single onion service failures.

v3:

We never implemented any special retry behaviour for v3 single onion services, but that doesn't matter, because there is no v3 Tor2web.

comment:17 in reply to:  16 ; Changed 9 months ago by teor

Replying to teor:

v2 Intro:

v2 Tor2web will extend to another intro point, and then succeed because it's no longer a single hop path.

The client will only extend if it thinks that the service isn't connected to the intro point.
So maybe we shouldn't close intro circuits, but we should force them to extend instead?

And then we can have an option for closing them as a DDoS defence.

comment:18 in reply to:  17 Changed 9 months ago by dgoulet

Replying to teor:

Replying to teor:

v2 Intro:

v2 Tor2web will extend to another intro point, and then succeed because it's no longer a single hop path.

The client will only extend if it thinks that the service isn't connected to the intro point.
So maybe we shouldn't close intro circuits, but we should force them to extend instead?

In theory, just a NACK received by the client will make it reuse the circuit and re-extend. See handle_introduce_ack(). This patch makes the intro return HS_CELL_INTRO_ACK_NORELAY which triggers a re-extend. Same goes for v2 in rend_client_introduction_acked().

comment:19 Changed 9 months ago by teor

Ok, let's put these behind DDoS options that are on by default, so we can turn them off if we get it wrong.

comment:20 Changed 9 months ago by asn

Status: needs_reviewneeds_revision

I feel like we are spending too much time on this ticket, while it's not a priority and we are not sure what's the best way to do it. I'm marking it as needs_revision because of the recent discussion about DoS options.

Perhaps we can revisit this in Rome and figure the best approach for handling and supporting 1-hop everything. I'm fine with delaying this to 034 too.

comment:21 Changed 8 months ago by nickm

Keywords: 033-triage-20180320 added

Marking all tickets reached by current round of 033 triage.

comment:22 Changed 8 months ago by nickm

Keywords: 033-removed-20180320 added

Mark all not-already-included tickets as pending review for removal from 0.3.3 milestone.

comment:23 Changed 8 months ago by nickm

Milestone: Tor: 0.3.3.x-finalTor: unspecified

These tickets were marked as removed, and nobody has said that they can fix them. Let's remember to look at 033-removed-20180320 as we re-evaluate our triage process, to see whether we're triaging out unnecessarily, and to evaluate whether we're deferring anything unnecessarily. But for now, we can't do these: we need to fix the 033-must stuff now.

comment:24 Changed 5 months ago by teor

Summary: hs: Stop rend and intro points being used as single hop proxieshs: Stop intro points being used as single hop proxies

#24902 stops rend points being used as single hop proxies, but we still need to fix intro points (and HSDirs).

comment:25 Changed 5 months ago by teor

Description: modified (diff)

comment:26 Changed 5 months ago by teor

Description: modified (diff)
Note: See TracTickets for help on using tickets.