Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#6415 closed defect (fixed)

rendclient.c doesn't set auth_len in INTRODUCE1 sometimes

Reported by: andrea Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Keywords: tor-hs
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

While working on 6177, I've discovered that when auth_type is REND_NO_AUTH, rend_client_send_introduction() leaves auth_len uninitialized. This worked with the old INTRODUCE2 parsing code in rend_service_introduce(), but doesn't match the spec and broke with my initial rewrite of the parser. I'll make sure my new rend_service_introduce() handles this behavior, but it should still get fixed - it should be a pretty quick fix.

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by andrea

Turns out it *does not emit* auth_len when auth_type == REND_NO_AUTH, actually, so the format parsed by the old rend_service_introduce() fails to match the spec in such a way that changing it to handle the specified format would cause it to break on existing rendclient.c. Looks like the least painful option would be to change the spec to match this behavior.

comment:2 Changed 7 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.3.x-final

Hm. Can we change the spec, leave the parser tolerant, _and_ emit the correct format? Or does the current parser not parse the correct format either?

comment:3 Changed 7 years ago by nickm

Ah, we can't make the parser tolerate the right format, and it doesn't tolerate the right format. Yeah, just changing the spec is the right thing to do.

See branch "bug6415" in my public torspec repository. Does it look okay to you?

comment:4 Changed 7 years ago by nickm

Status: newneeds_review

(my public torspec repository is user/nickm/torspec.git on {git,git-rw,gitweb}.torproject.org)

comment:5 Changed 7 years ago by andrea

Looks good; that matches the code in rendclient.c.

comment:6 Changed 7 years ago by arma

looks plausible to me too

comment:7 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged then. Thanks!

comment:8 Changed 7 years ago by nickm

Keywords: tor-hs added

comment:9 Changed 7 years ago by nickm

Component: Tor Hidden ServicesTor
Note: See TracTickets for help on using tickets.