#31548 closed defect (fixed)

hs-v3: Service can pick more than HiddenServiceNumIntroductionPoints intro points

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor:
Severity: Normal Keywords: consider-backport-after-0424, tor-hs, service, hs-v3, 035-backport, 040-backport, 041-backport, 042-should
Cc: Actual Points: 0.2
Parent ID: #29995 Points: 0.2
Reviewer: asn Sponsor: Sponsor27-must


During my testing of #30200, I ended up with service descriptor with 4 intro points even though HiddenServiceNumIntroductionPoints is set to 3 (default).

Further investigation confirmed this by adding a log in the decode_intro_points() function which showed me 4 intro points.

I haven't found out why but one feature of HS is that we launch HiddenServiceNumIntroductionPoints + 2 intro circuits in parallel and the first one to finish are picked.

It appears that more than the defined value can finish at the same time and will be picked.

Child Tickets

Change History (10)

comment:1 Changed 14 months ago by dgoulet

Cc: asn removed
Keywords: 035-backport 040-backport 041-backport added
Points: 0.2
Reviewer: asn
Status: newneeds_review

I recommend backport up to 035. This is major bugfix in my opinion affecting HS reachability quite a bit. This fix also mitigates #31561 that makes a service possibly put in the descriptor intro points for which it has *no* circuits, never.

Following is based on maint-0.3.5:

PR: https://github.com/torproject/tor/pull/1274
Branch: ticket31548_035_01

comment:2 Changed 14 months ago by asn

Status: needs_reviewneeds_revision

Great find. LGTM. Just a tiny comment nitpick.

BTW, do we do this check in v2?

comment:3 Changed 14 months ago by nickm

Keywords: 042-should added

Mark some needs_revision tickets as 042-should

comment:4 Changed 14 months ago by dgoulet

Owner: set to dgoulet
Status: needs_revisionaccepted

comment:5 Changed 14 months ago by dgoulet

Status: acceptedneeds_revision

comment:6 Changed 13 months ago by dgoulet

Status: needs_revisionneeds_review

Done, removed the XXX in the fixup commit: afcca119989a5b078cbfee8641024997af133e2b

BTW, do we do this check in v2?

Nope, v2 is very different on how it picks intro points. I haven't analyze if it is also affected but I don't think so, I've never seen that behavior on v2.

comment:7 Changed 13 months ago by asn

Status: needs_reviewmerge_ready


comment:8 Changed 13 months ago by nickm

Milestone: Tor: 0.4.2.x-finalTor: 0.4.1.x-final

Okay. I've made a squashed branch ticket31548_035_01_squashed with PR at https://github.com/torproject/tor/pull/1395

Merged it to master, marking for backport.

comment:9 Changed 13 months ago by teor

Actual Points: 0.2
Keywords: consider-backport-after-0424 added
Version: Tor:

comment:10 Changed 11 months ago by teor

Milestone: Tor: 0.4.1.x-finalTor: 0.3.5.x-final
Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.5 and later.

Merged #32575, #31939, #31548, #30344, #30258, #28970, #31091, and #32108 together.

Note: See TracTickets for help on using tickets.