Opened 3 years ago

Closed 3 years ago

#20089 closed defect (fixed)

Require "p" lines in consensuses

Reported by: Sebastian Owned by:
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/DirAuth Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Karsten and I looked at the historical data, and there are no consensuses past method 5 where the p lines are omitted. Additionally, I analyzed the code. Here are my findings:

in routerstatus_format_entry(), we can either create V2 or V3-style entries. V2 has no practical relevancy here, because the spec is for V3. For V3, we either have a consensus, a vote, a microdesc consensus, or a control port output. If we're using a method != V2, we always initialize a routerinfo_t *desc. If this step fails we do not generate an entry for this router.

Later, if desc != NULL (remember, this can only happen for V2 style documents) we call policy_summarize() and add the result as a "p" line. Looking at this function, it can never fail. Result is set to "reject 1-65535", "accept 1-65535" or "%s %s" where the first is prefix and the latter the shorter policy entry.

Child Tickets

Change History (7)

comment:1 Changed 3 years ago by Sebastian

This is implemented in ticket20089 in my repo.

comment:2 Changed 3 years ago by nickm

Status: newneeds_review

comment:3 Changed 3 years ago by nickm

Are you sure you uploaded it ? I don't see it in your repository

comment:4 Changed 3 years ago by Sebastian

yes, maybe you looked in the tor repo, not the torspec repo? https://gitweb.torproject.org/sebastian/torspec.git/log/?h=ticket20089

comment:5 Changed 3 years ago by nickm

Status: needs_reviewneeds_revision

I think this is incorrect; p6 does _not_ appear exactly once in each microdesc, even when the consensus is produced with method 15 or later. It only seems to appear when IPv6 is supported.

comment:6 Changed 3 years ago by Sebastian

Status: needs_revisionneeds_review

Ouch. Yeah. I thought we required ipv6. Hrm. I think I'll leave the ipv6 part out and we can revisit that when we're ready to mandate v6 support. added a fixup commit.

comment:7 Changed 3 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

squashed and merged; thanks!

Note: See TracTickets for help on using tickets.