[tor-bugs] #5044 [Tor Relay]: Mismatch between dir-spec.txt and routerparse.c

[Tor Bug Tracker & Wiki]

#5044: Mismatch between dir-spec.txt and routerparse.c
-----------------------+----------------------------------------------------
 Reporter:  karsten    |          Owner:              
     Type:  defect     |         Status:  needs_review
 Priority:  normal     |      Milestone:              
Component:  Tor Relay  |        Version:              
 Keywords:             |         Parent:              
   Points:             |   Actualpoints:              
-----------------------+----------------------------------------------------
Changes (by karsten):

  * status:  new => needs_review


Comment:

 Replying to [comment:1 arma]:
 > Replying to [ticket:5044 karsten]:
 > >  - `protocols` lines are specified in dir-spec.txt and written to
 server descriptors, but they are not parsed in routerparse.c.
 >
 > I think that's fine. We'll parse them once we care what they say. (And
 we'll want them to be there for the current versions, once new versions
 say something different, so we can tell the difference.)

 Right.  Nothing to change here then.

 > >  - `consensus-method` lines are specified as "Exactly once for
 consensuses" but implemented using the `T01` macro in routerparse.c.
 >
 > I could go either way here. Looks like a missing value means consensus
 method 1. We're way past consensus method one now -- we'll never use that.
 So maybe T1 is slightly better.

 Hmm, now that I think more about it, I think changing the code is
 problematic.  dir-spec.txt says:

 {{{
    Before generating a consensus, an authority must decide which consensus
    method to use.  To do this, it looks for the highest version number
    supported by more than 2/3 of the authorities voting.  If it supports
 this
    method, then it uses it.  Otherwise, it falls back to method 1.
 }}}

 I think that means we'll have to fix the spec, not the code.

 > >  - `contact` lines are specified as "At most once" for votes and
 authority sections of consensuses, but are implemented using the `T1` and
 `T1N` macro in routerparse.c.
 >
 > I'm fine making these optional. Note that authorities are required to
 set ContactInfo (see config.c). But we can look it up in their relay if
 we're confused -- so long as we don't do Sebastian's plan of having
 authorities not be relays from #3023, that is.

 Sounds like it's easier (and still correct) to fix the spec here.

 > >  - `legacy-key-dir` lines are specified as `legacy-key` in dir-
 spec.txt.
 >
 > And it would appear they are 'legacy-dir-key' in the code? This is a
 case where we should fix the spec to match the code.

 Agreed.

 > >  - `s` lines are specified as "At most once" in dir-spec.txt, but are
 implemented using the `T1` macro in routerparse.c.
 >
 > A few months ago I wrote some comment on some trac ticket expressing
 concern that missing the s line would make parsing harder or riskier in
 some way. I wonder what that was.

 So, `s` lines have always been included in network statuses, AFAIK.  If a
 relay doesn't have any flags, which can be in a bridge status or in a
 consensus prior to removing non-Running relays, the `s` line is present,
 but simply contains no flags.

 I think fixing the spec is correct here, too.

 I'm attaching a patch with the dir-spec changes.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5044#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online