[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #31561 [Core Tor/Tor]: hs-v3: Service can keep unused intro points in its list
#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
---------------------------------------+-----------------------------------
Comment (by mikeperry):
Replying to [comment:14 asn]:
> 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.
This is a good reason to have vanguard purpose circuits count as neither
client *nor* service HS circs. Maybe they should not even count as HS
circs at all, for function sanity.
> 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.
I think for every HS case it is safe to ignore vanguards purpose circuits.
They are pre-built only and not used yet.. This might be a good reason to
have every place where vanguards calls circuit_purpose_is_hidden_service()
*also* check explicitly for its vanguard purpose...
> 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. Perhaps the function should have a vanguard-centric name,
instead of an onion service-centric name.
If we decide to leave this as including vanguards, they definitely should
not count as hs client circuits (which the branch currently does). There
should just be an extra check for them in this function.
> 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.
Additionally, circuits can time out in
circuit_build_times_handle_completed_hop(), where they can get marked as
measurement circuits via circuit_change_purpose(). However, the fix for
#29034 cause them to get removed from the hs circuit map whenever we
change purposes. Is this enough to keep them out of the HS descriptor's
intropoint list as well? If so, then I guess that part is handled, at
least.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31561#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs