[tor-bugs] #23387 [Core Tor/Tor]: prop224: HSdir index desynch between client and service

[Tor Bug Tracker & Wiki]

#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