[tor-bugs] #14846 [Tor]: Controller: retrieve an HS descriptor of a service run by a user

[Tor Bug Tracker & Wiki]

#14846: Controller: retrieve an HS descriptor of a service run by a user
-------------------------+-------------------------------------------------
     Reporter:  dgoulet  |      Owner:  donncha
         Type:           |     Status:  needs_revision
  enhancement            |  Milestone:  Tor: 0.2.7.x-final
     Priority:  trivial  |    Version:  Tor: 0.2.7
    Component:  Tor      |   Keywords:  SponsorR, tor-hs, controller,
   Resolution:           |  027-triaged-1-in, SponsorS, TorCoreTeam201508
Actual Points:           |  Parent ID:  #3521
       Points:  small    |
-------------------------+-------------------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 Here is a review per commit based on branch `feature14846_2`:

 59b6d85a6f6a4abdad22d9efa6e5eccff6c1dbfe

  * In rendcache.c: `rend_cache_service` should be cleaned up in
 `rend_cache_free_all()`.
  * In rendcache.c, fct `rend_cache_store_v2_desc_as_service()`, this
 statement should be move just before the `if (!e) {` so we avoid calling
 tor_free() twice on `intro_contents` if timestamp is newer. Good thing
 `tor_free` sets the pointer to NULL! but if it was not the case, we have a
 double free issue.

 {{{
 +  /* We don't care about the introduction points. */
 +  tor_free(intro_content);
 }}}
  * For the `int service` extra argument of `rend_cache_lookup_entry`, I
 propose an enum value here representing the "type of the cache" and the
 reason is that this argument now makes the function choose which cache to
 lookup in, so adding that enum would tell us which cache is being looked
 at when reading the code. Not only that, but we could see in the future
 having only one function to do lookup in rend caches which shares common
 code and could call more specific functions for different type of cache.
 Something like:

 {{{
 typedef enum {
   REND_CACHE_TYPE_CLIENT  = 1,
   REND_CACHE_TYPE_SERVICE = 2,
 } rend_cache_type_t
 }}}
  and only one would be allowed (no `|` shanigans). At some point
 `REND_CACHE_TYPE_DIR` would be nice :).


 fe43c0fb259cb331c1d91cc27968c2a230b44ca0

  * lgtm;

 adca2261c4be56e769c35a5042e34e1e94023462

  * `git show -b` has nothing so lgtm :)

 96969252a54f09555998c7e05ed43abe84324f87
 347f0b116c2a02350519fbea4047b96424f4b1e0
 56195224314e7637d5617e550660f47593ef53e0

  * lgtm;

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