[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #14846 [Tor]: Controller: retrieve an HS descriptor of a service run by a user
#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.8.x-final
Priority: trivial | Version: Tor: 0.2.7
Component: Tor | Keywords: SponsorR, tor-hs, controller,
Resolution: | 027-triaged-1-in, SponsorS, TorCoreTeam201509
Actual Points: | Parent ID: #3521
Points: small |
-------------------------+-------------------------------------------------
Comment (by donncha):
Replying to [comment:27 dgoulet]:
> Replying to [comment:26 special]:
> > I think the GETINFO should be consistent with the existing
`hs/client/desc/id/<ADDR>` option. How about `hs/service/desc/id/<ADDR>`?
>
> Agree with special here.
Changed.
>
> >
> > Trivial typo in the changelog (for -> from).
> >
Fixed.
> > Which replica of the descriptor you get is an implementation detail of
the code, and the controller can't get both. Does that matter? I'm
guessing no.
>
> Code suggest that you'll get both actually. See
`control_event_hs_descriptor_created()`, the replica is outputed in the
control message.
Your correct special. Tor will attempt to generate and store the two
replica descriptors. The local service descriptor cache is indexed by
service ID so the second replica descriptor will replace the first
descriptor. The controller will only be able to retrieve the second
replica which was added to the cache last.
The `HS_DESC CREATED` event alerts the controller that each descriptor
(for each replica) has been created, the code doesn't currently allow both
replicas to be retrieved.
>
> >
> > From a read through of the code, this looks good to me.
>
> Any chance we can modify control-spec.txt also as we merge this feature?
I see a branch for this ticket in your github but pointing it here
explicitely with the latest branch ready for merge would be good once we
make the change above.
>
> In `rend_cache_clean()`, I see this being added with static for which I
don't think it's desired here, you just want a temporary "cache pointer"
for the lifetime of the function.
>
> {{{
> + static strmap_t *cache = NULL;
> }}}
>
> The side effect here would be that it's initialize to NULL *once* and
then the if/else after sets it to whatever cache we need. The next call to
that function, `cache` will point to the latest assignement and *NOT* be
initialized to NULL thus always passing the assert() test after even if
the cache type is wrong.
Thank you for the explanation, I have fixed this now.
I've pushed updated branches for the
[https://github.com/DonnchaC/tor/tree/feature14846_4 tor] and
[https://github.com/DonnchaC/torspec/tree/feature14846_2 torspec] patches.
Thank you special and dgoulet for your feedback on this patch.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/14846#comment:28>
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