[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #23387 [Core Tor/Tor]: prop224: HSdir index desynch between client and service
#23387: prop224: HSdir index desynch between client and service
-----------------------------+------------------------------------
Reporter: asn | Owner: (none)
Type: defect | Status: needs_revision
Priority: Very High | Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor | Version:
Severity: Normal | Resolution:
Keywords: prop224, tor-hs | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-----------------------------+------------------------------------
Changes (by asn):
* status: needs_review => needs_revision
Comment:
Dumping some comments from my review and testing here:
a) There is no commit msg in `daf72124` of why we don't like the previous
approach of figuring out whether overlap period just started/ended. I
don't really like this new approach of checking `consensus->valid_after ==
tp_start_time`, since there is no guarantee that Tor will get all the
consensuses: it can skip a consensus, and if it skips the right one then
we will never detect an overlap change.
This logic also caused an assert in my testing since we do:
{{{
if (!overlap_mode_is_active && !overlap_mode_just_ended) {
tor_assert(!service->desc_next);
}}}
and I accidentally triggered that because I suspended my laptop for 6
hours, which means that Tor missed the overlap-changing consensus, so it
never actually rotated descriptors. Not sure how to fix this issue. I
think the old logic was more robust; not sure why we ditched it.
b) Also seems like we still need a `if (service->desc_next)` guard before
`rotate_service_descriptors()` since it seems that it can go there with
`desc_next` being NULL and it will overwrite `desc_current` with NULL.
c) Also should we change all places we get the current time period num to
use the consensus valid after? To reduce desynch possibilities?
c) This final comment is not related to this branch, but I think there is
a case of HSDir desynch that ''we don't handle in the current prop224
spec'': Consider an HS with 13:00 consensus, and a client with 11:00
consensus. This means that the client is in time period #N and the HS is
in time period #N+1. The HS thinks it's in non-overlap mode so it only
uploads the current descriptor for time period #N+1. Meanwhile, the client
always downloads the current descriptor so it will attempt to download the
descriptor for time period #N which could have expired in theory.
This happens because we have an overlap period covering TP #N+1 from TP
#N, but we don't have one that covers TP #N from TP #N+1.
I'm dumping these thoughts here. Depending on my schedule today I might or
might not be able to fix these on time.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23387#comment:4>
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