[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #20852 [Core Tor/Tor]: prop224: Update HSDir prop224 code based on newest descriptor changes
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs