Opened 18 months ago

Last modified 8 months ago

#30466 needs_information defect

hs: Do not allow more than one control cell on a circuit

Reported by: dgoulet Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-dos, tor-hs, tor-relay
Cc: Actual Points:
Parent ID: Points: 0.1
Reviewer: dgoulet Sponsor: Sponsor27-can


This is the list of HS control cell that is they are all for establishing a circuit or/and "connection" between HS entities (IP, RP, Service, client):


It appears that anyone can send an arbitrary amount of those cells on the same circuit. Even to the point that tor allows a rendezvous circuit to become an intro circuit.

The only special one is INTRODUCE2 which is by-design are sent a lot on the same circuit.

The only cell currently limited to 1 cell is INTRODUCE1 since we do not allow multiple introductions on the same client circuit for DoS reasons.

But the rest should only be seen *once* on a circuit. Lets restrict them and if we see more, then we close the circuit due to a protocol error. This would limit side-channels.

Child Tickets

Change History (9)

comment:1 Changed 18 months ago by neel

Cc: neel added
Owner: set to neel
Status: newassigned

comment:2 Changed 17 months ago by dgoulet

Parent ID: #29999

Doesn't have to do much with DoS after all since the INTRODUCE2 problem we'll be taken care with #15516.

Actually, there is a world where we might want to do this but there is also another world where we could leverage this for instance with prop305 where the service could send multiple ESTABLISH_INTRO cell over the lifetime of the intro circuits to change the parameters.

comment:3 Changed 14 months ago by neel

I have a branch here:

It's incomplete (meaning not ready for review), I need to write tests.

comment:4 Changed 14 months ago by neel

Status: assignedneeds_review


Setting as needs review.

Tests were written where possible/feasible. If more tests are needed feel free to put as needs revision and I can add the extra tests.

comment:5 Changed 14 months ago by dgoulet

Reviewer: dgoulet

comment:6 Changed 14 months ago by dgoulet

Status: needs_reviewneeds_information

This approach is sensible and code seems accurate. But we should put this one on hold for now.

The reason is that there might be some cells, for instance ESTABLISH_INTRO, that we will want to be able to send multiple times during the lifetime of the circuit. (Relevant to #30924).

These control cells could be useful for DoS mitigation systems in order to be able to tell different HS entity on how to behave coming from the service.

comment:7 Changed 14 months ago by neel

I have a PR with the same code sans the ESTABLISH_INTRO part:

I am not setting this as needs review as I don't know if you need this or not (because of #30924). If you do, feel free to set it as needs_review.

comment:8 Changed 8 months ago by neel

Cc: neel removed
Owner: neel deleted
Status: needs_informationassigned

comment:9 Changed 8 months ago by neel

Status: assignedneeds_information
Note: See TracTickets for help on using tickets.