Controller: retrieve an HS descriptor from the client's cache

Split #3521 (moved) addressing one of the three original goals from the ticket first mentionned by rransom.

The approach here is to add the key hs/desc/id/<addr.onion> to GETINFO command. If found, the descriptor is printed formatted as described in rend-spec.txt section 1.3.

changed milestone to %Tor: 0.2.7.x-final

added 027-triaged-1-in SponsorS component::core tor/tor controller milestone::Tor: 0.2.7.x-final nickm-review owner::dgoulet parent::3521 points::small priority::very low resolution::fixed sponsor::R status::closed tor-hs type::enhancement version::tor 0.2.7 labels

To be clear, this is only if te descriptor is in the cache. We shouldn't fetch anything based on a getinfo.

Also, we should probably add separate keys for the client-side cache or the service-side cache. They are different, and it's important to keep them separate.

Replying to nickm:

To be clear, this is only if te descriptor is in the cache. We shouldn't fetch anything based on a getinfo.

Absolutely correct. This feature is another upcoming ticket for a new command. Other one of #3521 (moved).

Also, we should probably add separate keys for the client-side cache or the service-side cache. They are different, and it's important to keep them separate.

The HS service descriptor, that's also an other part of #3521 (moved) that I didn't yet have time to open :).

See branch ticket14845_01 for the control-spec.txt change (https://git.torproject.org/user/dgoulet/torspec.git)

If that makes sense, I'll submit for review the little-t tor code.

Trac:
Status: new to needs_review

Hrm wait, this change is wrong, it uses the same command for the service and client which is not right because an HS can be a clienté. It's important to differentiate both of them in the controller command.

Trac:
Status: needs_review to needs_revision

Ok, try 2! :)

I've settled for hs/client/desc/id/<ADDR>, a bit long but clearly indicates what we are looking for.

See branch ticket14845_02 for the control-spec.txt change (​https://git.torproject.org/user/dgoulet/torspec.git)

Trac:
Status: needs_revision to needs_review

To be clear, i don't think there should be a hsdir server equivalent.

And if the descriptor isn't found, it replies with an empty string, that is,

250 hs/client/desc/id/<ADDR>=

?

Replying to arma:

And if the descriptor isn't found, it replies with an empty string, that is, {{{ 250 hs/client/desc/id/= }}} ?

Oh true, forgot that, for instance GETINFO ns/id/<OR identity> returns "552 Unrecognized key" if not found so going with that makes sense. But then GETINFO ns/purpose/timeout returns "551 Internal error"... Much prefer 552 though. Unless we want to move towards a scheme of "250 NOT FOUND" ?...

Ok so seems 551 is more suited after looking at the control code and how getinfo error msg are handled.

Here is the spec + tor code patch (original patch from asn, cleaned up a bit)

Control-spec: see branch ticket14845_03. tor patch: see branch bug14845_026_01

I think the added error messages should be more explicit. The first one should say that address is considered invalid specifically due to wrong length (if, for example, user added $ needlessly in front). The other should probably say "not found in client cache".

Otherwise, this looks good to me.

The patch LGTM, with a separate patch at a later date adding a generic onion address validator routine to rendcommon.h/c that can return descriptive reasons for why we dislike a particular onion address (and to ease the amount of cleanup we need to do when Ed25519 happens).

Maybe do: "const rend_cache_entry_t *e = NULL;" to catch dumb in the future, though it's fine either way.

Marking more tickets as triaged-in for 0.2.7

Trac:
Keywords: SponsorR tor-hs controller deleted, SponsorR, controller, tor-hs, 027-triaged-1-in added

Trac:
Points: N/A to small
Keywords: N/A deleted, SponsorS added
Version: N/A to Tor: 0.2.7
Priority: normal to trivial

Merged, and added f5fa6ac534d9262701382cab3152826cc56a9e5d to fix a memory leak and d48df89d19a71752b1c849f7410877c852702b5a for changes file.

Merged the control-spec change too.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Not working for me as of commit 411049d0d44963b8d9ec6f96c8dc62a106d6cc30

Transcript:

authenticate "" 250 OK setevents hs_desc 250 OK hsfetch facebookcorewwwi 250 OK 650 HS_DESC REQUESTED facebookcorewwwi NO_AUTH $B53F5DE5FD55C836BED6BDDBFC1DC906B4543821curly wu5uc7ph33qdsgnvet5yesdvvw2bhn6r 650 HS_DESC RECEIVED facebookcorewwwi NO_AUTH $B53F5DE5FD55C836BED6BDDBFC1DC906B4543821curly getinfo hs/client/desc/id/facebookcorewwwi 551 Not found in cache

I've found found any way of making hs/client/desc/id/anything return anything except "551 Not found in cache" - either by hsfetch command, or by connecting to the HS via socks.

Trac:
Username: markxr@gmail.com

Trac:
Username: markxr@gmail.com
Status: closed to reopened
Cc: N/A to markxr@gmail.com
Resolution: fixed to N/A

https://github.com/Yawning/tor/compare/bug14845

Trac:
Status: reopened to needs_review
Keywords: N/A deleted, nickm-review added

Goot catch, this broke when we merged the refactoring of rend cache lookup :S.

lgtm! Thanks!

Merged; thanks!

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Trac:
Sponsor: N/A to SponsorR
Keywords: SponsorR deleted, N/A added

closed

mentioned in issue #14846 (moved)

mentioned in issue #14847 (moved)

mentioned in issue #15005 (closed)

mentioned in issue #15818 (moved)

moved to tpo/core/tor#14845 (closed)

mentioned in issue tpo/core/tor#15818 (closed)