Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#13214 closed defect (fixed)

HS clients don't validate descriptor-id returned by HSDir

Reported by: special Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-hs, 025-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor: SponsorR

Description

The rend descriptor cache for clients uses the permanent hostname as a key, instead of the descriptor-id (changed every 24 hours). When a descriptor is returned from a HSDir, the response is kept if the permanent hostname matches. The descriptor-id is not compared to the original request.

switch (rend_cache_store_v2_desc_as_client(body, conn->rend_data)) {
...
rend_client_desc_trynow(conn->rend_data->onion_address);

HS descriptors are currently valid for 3 days (#13207). A HSDir could fetch and serve a days-old descriptor for the same service, and that response will be used by clients.

There is no denial of service; the client will fetch from another directory after all introductions fail. The worst effect I can see is that a HSDir could briefly distract clients by serving a very old descriptor.

Child Tickets

Change History (12)

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-final

comment:2 Changed 5 years ago by arma

Keywords: SponsorR added

comment:3 Changed 5 years ago by nickm

Keywords: 025-backport added
Status: newneeds_review

Possible fix in bug13214_025; needs testing! (I have not tested this at all.)

comment:4 Changed 5 years ago by special

+  if (base32_decode(want_desc_id, sizeof(want_desc_id),
+                    desc_id_base32, strlen(desc_id_base32)) != DIGEST_LEN) {

base32_decode returns 0 on success.

Otherwise, it looks good to me. After fixing that, this branch works as a client.

comment:5 Changed 5 years ago by special

Status: needs_reviewneeds_revision

comment:6 Changed 5 years ago by nickm

Status: needs_revisionneeds_review

Better now?

comment:7 Changed 5 years ago by special

Yes, LGTM

comment:8 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.5.x-final

I squashed it into bug13214_025_squashed , and merged it to master for inclusion in 0.2.6.2-alpha. Marking this ticket for possible backport to 0.2.5.

comment:9 Changed 5 years ago by arma

I think it's mostly harmless, so no need for backport. Onward and upward!

comment:10 Changed 5 years ago by nickm

How sure are we that it's harmless?

comment:11 Changed 5 years ago by special

Resolution: fixed
Status: needs_reviewclosed

This was merged for 0.2.6, and looks like it was never backported to 0.2.5. I still agree that it was harmless, so not worth backporting now. Closing.

comment:12 Changed 4 years ago by dgoulet

Keywords: SponsorR removed
Sponsor: SponsorR
Note: See TracTickets for help on using tickets.