[tor-bugs] #20852 [Core Tor/Tor]: prop224: Update HSDir prop224 code based on newest descriptor changes

[Tor Bug Tracker & Wiki]

#20852: prop224: Update HSDir prop224 code based on newest descriptor changes
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  asn
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.0.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-hs, prop224, TorCoreTeam201612,  |  Actual Points:
  review-group-14                                |
Parent ID:                                       |         Points:  3
 Reviewer:  dgoulet                              |        Sponsor:
                                                 |  SponsorR-can
-------------------------------------------------+-------------------------

Comment (by asn):

 Replying to [comment:7 dgoulet]:
 > Replying to [comment:4 asn]:
 > > Please find an initial version of (a) in my branch `bug20852`:
 > > https://gitweb.torproject.org/user/asn/tor.git/log/?h=bug20852
 > >
 > > It basically does two things:
 > > - Changes `encrypted` to `superencrypted` in the outter HS desc layer.
 > > - Changes the max descriptor size to 50kb and introduces a consensus
 param `HSV3MaxDescriptorSize` that controls it.
 >
 > I like that this can be controlled by a consensus parameters which will
 be very helpful because we also plan to have a param for the number of
 introduction points which will ultimately affect the max size.
 >
 > One thing I would change is `hs_cache_get_max_hs_descriptor_size()`. The
 second "hs" in there is redundant as we are already in the HS subsystem
 here so `hs_cache_get_max_descriptor_size()" is enough imo.
 >
 > Also, `networkstatus_get_param()` returns a `int32_t` so we should NOT
 make it `unsigned int` for the max size but rather make sure we are in
 range and convert it before return. Even though it's probably not possible
 to have something not in range of what we ask, MORE safety net is good.
 >

 Thanks for the review, David. It was helpful!

 I pushed a branch called `bug20852_david` which addresses both comments
 above. Please review it, and if you like it I will squash the fixups into
 a more compact branch for Nick's review.

 WRT the `networkstatus_get_param()` issue I spent quite some time thinking
 of the right casting logic, and I ended up with restricting the max size
 to `INT32_MAX` and then casting to unsinged directly. Let me know if you
 think that's not the right logic.

 Thanks!

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