[tor-bugs] #24977 [Core Tor/Tor]: Non-fatal assertion !(tor_mem_is_zero((const char*)node->hsdir_index->fetch, DIGEST256_LEN))

[Tor Bug Tracker & Wiki]

#24977: Non-fatal assertion !(tor_mem_is_zero((const
char*)node->hsdir_index->fetch, DIGEST256_LEN))
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  dgoulet
     Type:  defect                               |         Status:
                                                 |  accepted
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.4.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.3.2.1-alpha
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-hs, prop224,                     |  Actual Points:
  034-triage-20180328, 034-removed-20180502      |
Parent ID:                                       |         Points:  1
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by asn):

 OK I went deep and debugged this.

 The basic problem is that upon fixing the clock we end up with
 `next_rotation_time` in the past, whereas normally it should be in the
 future like the comment from #25306 suggests:
 {{{
      * So, when tor starts, the next rotation time is set to the start of
 the
      * next SRV period using the consensus valid after time so it should
      * always be set to a future time value.
 }}}

 The root problem lies elsewhere tho: in the voting schedule calculation
 logic.

 Here are the bug steps:

 1) We start up with a skewed clock. We are fetching a consensus from the
 future which we don't consider live, and hence we don't build any HS
 descs.

 2) However, when we fetch the consensus we also call
 `voting_schedule_recalculate_timing()` which calls `get_voting_schedule()`
 with  a skewed `approx_time()`.  This means that `get_voting_schedule()`
 calculates `new_voting_schedule->interval_starts` with a skewed clock, and
 the result is also skewed.

 3) Then **we fix up the clock**. We now start trusting the consensus
 again, but we never call `get_voting_schedule()` again because it's only
 called when receiving a fetched consensus and we've already done that...
 This means that we remain with a skewed `interval_starts` schedule.

 4) Then the HS service code calls `set_rotation_time()` which uses the
 skewed `interval_starts` through
 `sr_state_get_start_time_of_current_protocol_run() ->
 get_start_time_of_current_round() ->
 voting_schedule_get_next_valid_after_time()` and ends up with a skewed
 `next_rotation_time`.

 5) Then we go into `should_rotate_descriptors()` with the basic assumption
 defeated (we just started up but `next_rotation_time` is in the past)
 which causes the BUG because we needlessly try to rotate the descriptors.

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