[tor-bugs] #13664 [Tor]: Potential issue with rend cache object when intro points falls to 0.

[Tor Bug Tracker & Wiki]

#13664: Potential issue with rend cache object when intro points falls to 0.
-------------------------+---------------------------------
     Reporter:  dgoulet  |      Owner:
         Type:  defect   |     Status:  new
     Priority:  normal   |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor      |    Version:
   Resolution:           |   Keywords:  tor-hs 025-backport
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+---------------------------------

Comment (by asn):

 Replying to [ticket:13664 dgoulet]:
 > (Reproduced on Tor v0.2.6.1-alpha-dev (git-a142fc29aff4b476))
 >
 > <snip>
 >
 > However, the rend cache is not cleared of the old entry before fetching
 that new descriptor. So once the v2 descriptor is received, we store it in
 the cache using "rend_cache_store_v2_desc_as_client()" that prints this:
 >
 > {{{
 > Nov 04 15:36:09.000 [info] rend_cache_store_v2_desc_as_client(): We
 already have this service descriptor rejxmpqgho5vqdl4. [rendezvous-
 service-descriptor i7hkcux5dghqv6ahstewyccltr6aud2x
 > }}}
 >
 > So since we "have it" in the cache, we call "rend_client_desc_trynow()"
 and it completely fails because all intro points in the cache object are
 gone so this closes all pending connections.
 >

 Hello,

 three questions to better understand the issue:
 a) This behavior only triggers when all the IPs are down or found
 unreachable, right?

 b) How does Tor finally escape this loop in the current codebase? Since no
 IPs are reachable and no new descriptor can be fetched?

 c) IIUC, the `desc` in `if (e && !strcmp(desc, e->desc)) {` is the whole
 descriptor as a string. So if that check succeeds, we already have the
 exact same descriptor? How would it help if we kept that same descriptor,
 since all the IPs listed in there are apparently unreachable? Would it
 just try to reconnect to one of those IPs, in case the connection
 succeeds?

 Three stupid comments on code style:
 a) `NULL byte` is usually referenced as `NUL` in Tor code. AFAIK, that's
 the official ASCII name of `\0`. `NULL` is the C macro.

 b) Function arguments that are supposed to be filled with stuff are
 usually named `something_out` in Tor code. Maybe `rend_cache_build_key`
 should take `key_out`/`key_len_out` instead of `key`/`len`?

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