Opened 5 years ago

Closed 2 years ago

#14905 closed defect (not a bug)

client descriptor-cookies are wrong for stealth auth

Reported by: meejah Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: tor-hs annoyance
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Tor writes the descriptor-cookies for stealth-authentications into two spots: "client_keys" and the "hostname" files. In the client_keys file, there appears to be an off-by-one in rendservice.c:1026, around this code:

if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1,

extended_desc_cookie,
REND_DESC_COOKIE_LEN+1) < 0) {

log_warn(LD_BUG, "Could not base64-encode descriptor cookie.");
goto err;

}

The REND_DESC_COOKIE_LEN shouldn't have the +1, I don't believe. (Or, the base64_encode call higher up *should* have one). In any case, both descriptor cookies are the same after that change (and work).

This is in master, as of commit 3bcdb26267502e0d1de5d01854c8a2cf29a5e5f4

I put a simple fix in https://github.com/meejah/tor/tree/descriptor-cookie-serialization

Child Tickets

Change History (14)

comment:1 Changed 5 years ago by dgoulet

Hrm, just above the auth type value is actually put in the + 1 location:

      extended_desc_cookie[REND_DESC_COOKIE_LEN] =
        ((int)s->auth_type - 1) << 4;

So, I'm guessing that the auth type needs to be b64 encoded thus passing the +1 len. Now why this is done only in the hostname field and not client_keys, I don't know.

Did you encounteer an issue with this behaviour?

comment:2 Changed 5 years ago by meejah

Hmm, odd; I tried again, and it seems to work with either cookie (and with or without the trailing == from the client_keys one). I notice that rendclient.c does actually check this value in the +1 slot -- but it would seem that one of the cookies is correctly indicating it's a "stealth" cookie and the other isn't. Perhaps it's always getting this right because the stealth is the "else" option?

Evidently, this doesn't actually *matter* (from an "observed behavior" perspective), but it still seems wrong ;)

comment:3 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-final

comment:4 Changed 5 years ago by nickm

Status: newassigned

comment:5 Changed 5 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:6 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:7 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:8 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:9 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:10 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:11 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:12 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:13 Changed 2 years ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:14 Changed 2 years ago by nickm

Keywords: tor-hs annoyance added
Priority: MediumLow
Resolution: not a bug
Severity: Normal
Status: newclosed
Note: See TracTickets for help on using tickets.