[tor-bugs] #16381 [Tor]: Bad timestamp check when storing an HS descriptor on the client

[Tor Bug Tracker & Wiki]

#16381: Bad timestamp check when storing an HS descriptor on the client
------------------------------+------------------------------------
 Reporter:  dgoulet           |          Owner:
     Type:  defect            |         Status:  new
 Priority:  normal            |      Milestone:  Tor: 0.2.7.x-final
Component:  Tor               |        Version:
 Keywords:  SponsorR, tor-hs  |  Actual Points:
Parent ID:                    |         Points:
------------------------------+------------------------------------
 Fortunately, this bug "should" happen rarely in the real Tor network but
 still could lead to reachability issues for high traffic hidden service.

 Let's assume that all introduction points (IP) of an hidden service need
 to be changed (intro2 max count or expiring) in a single `RendPostPeriod`
 (default is 1 hour), we do the intro dance with our newly picked
 introduction points, close the old ones and upload the new descriptor. The
 usual.

 Now, let's say we have a client that did connect to the service before in
 the same hour so the client has the descriptor in its cache. Then, the
 client tries to connect again, the IPs located in the cached descriptor
 are used and the client will eventually get a NACK from all of them since
 the service just cycled them all. This will eventually trigger a refetch
 of the descriptor since we have no more IP that we can use. So far so
 good.

 Here is when it goes wrong, when receiving the new descriptor just
 published by the HS, we do a series of check in
 `rend_cache_store_v2_desc_as_client()` which all passes correctly in our
 case except this one:
 {{{
   e = (rend_cache_entry_t*) strmap_get_lc(rend_cache, key);
   if (e && e->parsed->timestamp >= parsed->timestamp) {
     log_info(LD_REND, "We already have a new enough service descriptor for
 "
                       "service ID %s with the same desc ID and version.",
              safe_str_client(service_id));
     goto okay;
   }
 }}}

 The HS protocol speficies that the timestamp in the descriptor MUST be
 rounded down to the nearest hour. For instance, if it's 16h54, it will be
 set to 16h00. See `rend_service_update_descriptor()`:
 {{{
   d->timestamp = time(NULL);
   d->timestamp -= d->timestamp % 3600; /* Round down to nearest hour */
 }}}

 The side effect of this is that if an HS uploads a new descriptor with a
 new set of IPs, they won't be usable until the next hour because the tor
 client doesn't keep the new descriptor even though the intro points did
 actually change. Thus this means that anything that changes in a time
 period of one hour will not be seen by the client if the descriptor was
 already cached before.

 Keep in mind that a client reuses a rendez-vous circuit but stops after
 `MaxCircuitDirtiness` (default: 10 minutes) so after 10 minutes it will
 redo the introduction and rendezvous circuits.

 (I was able to trigger this in a chutney network by setting a lifetime of
 30 seconds for introduction point which creates a lot of rotation.)

 I think the fix here should look like this (fun fact, it's what
 `rend_cache_store_v2_desc_as_dir()` does):
 {{{
 - if (e && e->parsed->timestamp >= parsed->timestamp) {
 + if (e && e->parsed->timestamp > parsed->timestamp) {
 }}}

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