[tor-bugs] #23466 [Core Tor/Tor]: hs: v3 client descriptor cache entry should be indexed by blinded key

[Tor Bug Tracker & Wiki]

#23466: hs: v3 client descriptor cache entry should be indexed by blinded key
------------------------------+-----------------------------------------
     Reporter:  dgoulet       |      Owner:  (none)
         Type:  defect        |     Status:  new
     Priority:  High          |  Milestone:  Tor: 0.3.2.x-final
    Component:  Core Tor/Tor  |    Version:
     Severity:  Normal        |   Keywords:  tor-hs, tor-client, prop224
Actual Points:                |  Parent ID:
       Points:                |   Reviewer:  asn
      Sponsor:                |
------------------------------+-----------------------------------------
 We currently index `hs_cache_client_descriptor_t` by service identity key
 in the `hs_cache_v3_client` but imagine this scenario (that our mad bug
 hunter armadev stumbled upon):

 Day 0: You fetch the descriptor for service S with rev counter 8 (blinded
 key: BK0)

 Day 1: Computer in suspend mode to tor is sleepy-sleepy.

 Day 2: Wake up, and try to access the descriptor of blinded key BK2 so you
 lookup the cache for service S and get the descriptor you fetched on Day 0
 (yes because lifetime is ridiculously long that is 72h). It happens that
 the revision counter at that time is 5. Because 8 > 5, you won't keep the
 descriptor and trigger this log info:

 {{{
     if (cache_entry->desc->plaintext_data.revision_counter >
         client_desc->desc->plaintext_data.revision_counter) {
       log_info(LD_REND, "We already have fresher descriptor. Ignoring.");
 }}}

 ... and end up using the descriptor of Day 0 which is just unusable.

 There are several issues here:

 1. The lifetime of a descriptor in our cache is just insanely too big. In
 reality, a descriptor blinded key has a lifetime of 24h maximum so it
 really doesn't make sense for a client to keep a descriptor more than that
 in its cache. Furthermore, because the hashring is computed using the
 consensus valid_after time, we don't need to add some buffer for clock
 skewed client. Thus, let bring it down to 24h only. We could be clever and
 put its expire time up to the next time period where the client will fetch
 a descriptor using a different blinded key.

 2. We need to index the client descriptor cache entries by blinded key
 which is what the HSDir do.

 3. I kind of want to turn this `log_info()` into a `BUG()` because we
 should never be triggering directory fetches for a blinded key for which
 we already have a valid descriptor. Rev counter for a specific blinded key
 can NOT go backward directory side nor client side. And can't be reused
 either over time.

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