Opened 4 years ago

Closed 4 years ago

#9002 closed defect (fixed)

Clients should discard v2 HS descriptors with more than 10 intro points

Reported by: rransom Owned by: mikeperry
Priority: Very High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: 023-backport tor-hs
Cc: Actual Points:
Parent ID: #9001 Points:
Reviewer: Sponsor:

Description


Child Tickets

Change History (10)

comment:1 Changed 4 years ago by rransom

Parent ID: #9001
Priority: normalcritical

This mitigates a guard-discovery attack far easier to exploit than the ‘critical’ #9072, so adjusting its priority accordingly.

comment:2 Changed 4 years ago by nickm

Keywords: 023-backport added
Milestone: Tor: 0.2.4.x-final

Any reason for HSDirs to not reject them too?

comment:3 Changed 4 years ago by nickm

Keywords: tor-hs added

comment:4 in reply to:  2 Changed 4 years ago by rransom

Replying to nickm:

Any reason for HSDirs to not reject them too?

No. However:

  • Malicious HSDirs could still serve them, so rejecting them at the HSDir end is not sufficient.
  • If an HS requires client authorization, then it encrypts the intro-point list in its descriptors, so HSDirs can't count on being able to parse the intro-point list, so v2 HSDirs don't currently parse the intro-point list ever, so please don't expose any more potentially crashy-buggy parsing code at the relay end over this.

comment:5 Changed 4 years ago by nickm

Status: newneeds_review

See branch "bug9002" on my public repo. It should merge cleanly to 0.2.3 and later. I made the change to the v0 HS descriptor code too, though I suppose that's probably not needed.

comment:6 in reply to:  5 Changed 4 years ago by rransom

Replying to nickm:

See branch "bug9002" on my public repo. It should merge cleanly to 0.2.3 and later.

A hidden service might publish a descriptor containing zero intro points before it shuts down, so n_intro_points might be 0 in practice. (I've seen some piece of design documentation somewhere claiming that they do publish such a descriptor.) But perhaps that's a separate bug.

I made the change to the v0 HS descriptor code too, though I suppose that's probably not needed.

In 0.2.3.x, the v0 HS descriptor code is used only for HS authorities. I don't know whether HS authority support was ever removed.

comment:7 Changed 4 years ago by andrea

This patch looks reasonable to me; only thing to really question is the choice of MAX_INTRO_POINTS. Do we know how hidden service reliability varies as a function of the number of intro points to justify the choice and the phrasing "(misguided) attempt to improve reliability" in the warning?

comment:8 Changed 4 years ago by nickm

I don't think we have solid numbers. The number is equal to NUM_INTRO_POINTS_MAX in rendservice.c, but I don't know whether we picked that through some principled process or what. You'll not see more than 10 in a currently generated descriptor unless somebody's been monkeying with the source.

comment:9 in reply to:  7 Changed 4 years ago by rransom

Replying to andrea:

This patch looks reasonable to me; only thing to really question is the choice of MAX_INTRO_POINTS. Do we know how hidden service reliability varies as a function of the number of intro points to justify the choice and the phrasing "(misguided) attempt to improve reliability" in the warning?

Publishing more intro points in a descriptor should spread the circuit-extension load of client intro circuits over more relays so that clients are less likely to overload the intro-point relays. See #3825 for what happens if clients do overload a relay; in short, if they fail to extend their circuit to the intro point, they used to repeatedly try again, thus keeping the relay overloaded. (I fixed that; now they only pound each intro point a few times before giving up on it.)

The maximum number of intro points which will improve a hidden service's reliability is 6, because each HS publishes its descriptor to at most 6 HSDir relays. I picked 10 as the maximum in #3825 because I wanted to never have to worry about that bottleneck again.

comment:10 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merging to 0.2.3 and beyond.

Note: See TracTickets for help on using tickets.