[tor-bugs] #31561 [Core Tor/Tor]: hs-v3: Service can keep unused intro points in its list

[Tor Bug Tracker & Wiki]

#31561: hs-v3: Service can keep unused intro points in its list
---------------------------------------+-----------------------------------
 Reporter:  dgoulet                    |          Owner:  dgoulet
     Type:  defect                     |         Status:  needs_revision
 Priority:  Medium                     |      Milestone:  Tor:
                                       |  0.4.2.x-final
Component:  Core Tor/Tor               |        Version:
 Severity:  Normal                     |     Resolution:
 Keywords:  tor-hs, hv-v3, 042-should  |  Actual Points:  0.1
Parent ID:  #30200                     |         Points:  0.2
 Reviewer:  asn                        |        Sponsor:  Sponsor27-must
---------------------------------------+-----------------------------------
Changes (by asn):

 * status:  merge_ready => needs_revision
 * cc: mikeperry (added)


Comment:

 Replying to [comment:13 nickm]:
 > I'm concerned that this patch makes `purpose ==
 CIRCUIT_PURPOSE_HS_VANGUARDS` count as both a client purpose and a service
 purpose.  Won't that result in us sometimes calling
 `hs_service_circuit_timed_out` on a client circuit?

 Hmm, there is indeed a bug here! Thanks for catching that, and sorry for
 missing that.

 `CIRCUIT_PURPOSE_HS_VANGUARDS` is the purpose that Tor uses to pre-built
 vanguard circuits '''before''' they are used by the vanguard subsystem for
 specific HS-related purposes (they can be used for HS client or for HS
 service activities).

 However, while the circuit is in this vanguards purpose, it shouldn't have
 any duties as an HS circuit (i.e. be an active intro circuit) and hence we
 shouldn't call `hs_service_circuit_timed_out()` on it.

 Vanguards circuits are taken into account in the
 `circuit_purpose_is_hidden_service()` function because it is used by the
 vanguard subsystem to take decision about vanguards; but for our use case
 it's safe to ignore vanguard circuits.

 So takeaways and plans for the patch:
 a) The `circuit_purpose_is_hidden_service()` should remain as is
 (behavior-wise) because it's used (correctly) by the vanguard subsystem.
 But IMO the name is a bit misleading because IIUC vanguard circuits don't
 have active hidden service duties, but they can be repurposed to HS circs
 in the future.
 b) Our new function `circuit_purpose_is_hs_service()` should not take into
 account vanguard circuits since we only care about active onion service
 circs. Perhaps `circuit_purpose_is_hidden_service()` can use our new
 function to avoid code dedup.
 c) Let's ask Mike if he agrees with the above.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31561#comment:14>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online