[tor-bugs] #3523 [Tor]: Allow controllers to post HS descriptors to the HSDir system

[Tor Bug Tracker & Wiki]

#3523: Allow controllers to post HS descriptors to the HSDir system
-----------------------------+--------------------------------
     Reporter:  rransom      |      Owner:
         Type:  enhancement  |     Status:  needs_revision
     Priority:  minor        |  Milestone:  Tor: 0.2.7.x-final
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:  tor-hs controller
Actual Points:               |  Parent ID:  #8993
       Points:               |
-----------------------------+--------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:21 donncha]:
 > Updated the code to remove some redundant code
 >
 https://github.com/DonnchaC/tor/compare/ca03b10b0c39a02fd66824d04deccf40cbd66951...3234-patch

 In `handle_control_hspost()`

 * I think here it should be `512`, see control-spec.txt (512 Syntax error
 in command argument)
 {{{
 connection_printf_to_buf(conn, "552 Unexpected argument \"%s\"\r\n",
                          arg);
 }}}

 * This is not necessary. No chance that `descs` contains more than one
 descriptor so you should keep `desc` intact (not setting it to NULL) and
 free it in the done label (even if NULL). If anything pass the `desc`
 allocation in the future makes a goto done, you'll be safe already :).

 {{{
 SMARTLIST_FOREACH(descs, rend_encoded_v2_service_descriptor_t *, d, {
 ...
 }}}

 * `intro_content` can leak. Fun fact, `rend_parse_v2_service_descriptor()`
 can set `intro_content` and still go on error after... Now, tbh I would
 fix rend_parse_v2_service_descriptor() instead here to ONLY sets out
 pointers when it actually succeed ;). But better safe than sorry, set it
 to NULL and free it once you are done. (If you feel like it, do a commit
 on top to fix rend_parse.., that would be neat :)

 In `directory_post_to_hs_dir`

 * This will *not* trigger an UPLOAD event which I think it should
 according to the spec?

 {{{
 +    } else {
 +      /* Determine responsible dirs. */
 +      if (hid_serv_get_responsible_directories(.......
 }}}

 Rest looks good! We are getting there! :D

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