Opened 4 years ago

Closed 23 months ago

#14312 closed defect (fixed)

tor-spec says additional fields in exitpolicy response are "optional" when they're not

Reported by: arma Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-spec easy
Cc: Actual Points:
Parent ID: Points: 0
Reviewer: Sponsor:

Description

A #tor user reported seeing

    if (rh.length < 9) { /* reason+ipv4+dns_ttl */
      log_notice(LD_PROTOCOL,
             "Short path bias probe response length field (%d).", rh.length);
      return - END_CIRC_REASON_TORPROTOCOL;
    }

I think this was triggered by Tom's new relay implementation.

It turns out our spec says

   The payload of a RELAY_END cell begins with a single 'reason' byte to
   describe why the stream is closing, plus optional data (depending on
   the reason.)
   [...]
   (With REASON_EXITPOLICY, the 4-byte IPv4 address or 16-byte IPv6 address
   forms the optional data, along with a 4-byte TTL; no other reason
   currently has extra data.)

Tom and I are now thinking that this word 'optional' means 'required for some types of end cells but not included in others'. But he misinterpreted 'optional' to mean 'you don't have to implement it'. Which is a fine interpretation, except Tor clients complain at log-level notice when you don't.

I think it was originally optional because some very old Tor versions didn't implement it. But now they all do (well, up until yesterday, when Tom's version came online). Should we just make this extra data for reason-exitpolicy be optionally mandatory?

Child Tickets

Change History (14)

comment:1 Changed 4 years ago by nickm

That sounds right to me. It should probably be "additional" not "optional".

comment:2 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-final

comment:3 Changed 4 years ago by nickm

Status: newassigned

comment:4 Changed 4 years ago by nickm

Keywords: 027-triaged-1-out added

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

comment:5 Changed 4 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:6 Changed 3 years ago by teor

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

Milestone renamed

comment:7 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:8 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:9 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:10 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:11 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:12 Changed 2 years ago by nickm

Status: assignednew

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

comment:13 Changed 2 years ago by nickm

Keywords: easy added
Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Owner: set to nickm
Points: 0
Severity: Normal
Status: newaccepted

comment:14 Changed 23 months ago by nickm

Resolution: fixed
Status: acceptedclosed

0a4a064de27e59980474b687c4eb8ff89da19477 fixes this.

Note: See TracTickets for help on using tickets.