Opened 4 years ago

Closed 4 years ago

#7866 closed defect (fixed)

dir-spec doesn't cover '-legacy' authority entries

Reported by: karsten Owned by: atagar
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client tor-spec
Cc: nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Some consensus headers contain "dir-source *-legacy" lines. Here's an example from 2011-10-29-05-00-00-consensus (attached):

[...]
dir-source urras 80550987E1D626E3EBA5E5E75A458DE0626D088C 208.83.223.34 208.83.223.34 443 80
contact 4096R/E012B42D Jacob Appelbaum <jacob@appelbaum.net>
vote-digest C6E35236E655F00EBE1A750C75E38AFF17B56A62
dir-source gabelmoo-legacy 81349FC1F2DBA2C2C11B45CB9706637D480AB913 212.112.245.170 212.112.245.170 80 443
dir-source moria1 D586D18309DED4CD6D57C18FDB97EFA96D330566 128.31.0.34 128.31.0.34 9131 9101
contact 1024D/28988BF5 arma mit edu
vote-digest 3D115F7BA5E0A2C624D32C8C514171C9CE3E790F
[...]

Stem complains:

ParsingFailure: Authority entries must have a 'contact' line:
dir-source gabelmoo-legacy 81349FC1F2DBA2C2C11B45CB9706637D480AB913 212.112.245.170 212.112.245.170 80 443

See dir-spec.txt Section 3.5 for how to handle these lines correctly.

Child Tickets

Attachments (1)

2011-10-29-05-00-00-consensus.gz (169.2 KB) - added by karsten 4 years ago.

Download all attachments as: .zip

Change History (20)

Changed 4 years ago by karsten

comment:1 Changed 4 years ago by atagar

Thanks Karsten, though I don't quite follow. Section 3.3 says that a contact line will happen once per an authority entry (that's what stem is complaining about here).

The only mention to "-legacy" in the dirspec is section 3.5 as you mentioned, with...

If the consensus
method is 3 or later, a dir-source line must be included for
every vote with legacy-key entry, using the legacy-key's
fingerprint, the voter's ordinary nickname with the string
"-legacy" appended, and all other fields as from the original
vote's dir-source line.

I'm not spotting where it specifies differences in how '-legacy' entries should be parsed.

comment:2 Changed 4 years ago by karsten

The difference is that "dir-source *-legacy" lines don't have a subsequent "contact" or "vote-digest" line. That's what Stem complains about, that there's no "contact" line for this authority entry.

comment:3 Changed 4 years ago by atagar

Gotcha. What I was trying to ask though is where in the dir-spec it says this (I copied the only part I could find about legacy lines above and by my read it doesn't say anything about the contact/vote-digest entries).

Would you mind copying the part of the spec that says '*-legacy' lines lack a contact and vote-digest entry? What I'm trying to figure out is if we need a spec update. :)

comment:4 Changed 4 years ago by karsten

Oh, I see. I agree that dir-spec may need an update. The "dir-source *-legacy" line should be considered a special kind of authority section, because these lines are not part of the non-legacy groups of the respective authorities. And then the "exactly once" parts don't apply anymore. I guess 3.3 needs to be changed for this, whereas 3.5 can (mostly) stay the same. Here's a quick suggestion, feel free to polish.

diff --git a/dir-spec.txt b/dir-spec.txt
index ca5434b..875501e 100644
--- a/dir-spec.txt
+++ b/dir-spec.txt
@@ -1340,28 +1340,34 @@
 
    The authority section of a consensus contains groups the following items,
    in the order given, with one group for each authority that contributed to
-   the consensus, with groups sorted by authority identity digest:
+   the consensus and for each vote with legacy-key entry, with groups sorted
+   by authority identity digest:
 
     "dir-source" SP nickname SP identity SP address SP IP SP dirport SP
        orport NL
 
         [Exactly once, at start]
 
-        As in the authority section of a vote.
+        As in the authority section of a vote, or using the legacy-key's
+        fingerprint, the voter's ordinary nickname with the string
+        "-legacy" appended, and all other fields as from the original
+        vote's dir-source line.
 
     "contact" SP string NL
 
-        [Exactly once.]
+        [At most once.]
 
-        As in the authority section of a vote.
+        As in the authority section of a vote.  Only present if this group
+        does not represent a legacy key.
 
     "vote-digest" SP digest NL
 
-        [Exactly once.]
+        [At most once.]
 
         A digest of the vote from the authority that contributed to this
         consensus, as signed (that is, not including the signature).
-        (Hex, upper-case.)
+        (Hex, upper-case.)  Only present if this group does not represent
+        a legacy key.
 
    Each router status entry contains the following items.  Router status
    entries are sorted in ascending order by identity digest.

comment:5 Changed 4 years ago by arma

I wonder if it's better, rather than cluttering the spec with details about the hack we did a few years ago, to just declare some of the fields optional.

comment:6 Changed 4 years ago by arma

  • Cc nickm added

cc'ing nickm since he's unlikely to see this Tor-related ticket otherwise

comment:7 Changed 4 years ago by atagar

Thanks Karsten! If this no longer appears in current consensuses then I agree that we should try to make this more of a side note. Maybe just an extra paragraph for the consensus' authority section?

diff --git a/dir-spec.txt b/dir-spec.txt
index ca5434b..bcb16f0 100644
--- a/dir-spec.txt
+++ b/dir-spec.txt
@@ -1340,7 +1340,12 @@
 
    The authority section of a consensus contains groups the following items,
    in the order given, with one group for each authority that contributed to
-   the consensus, with groups sorted by authority identity digest:
+   the consensus, with groups sorted by authority identity digest.
+
+   If the nickname attribute on the 'dir-source' line ends with "-legacy" then
+   this follows a legacy formatting. Legacy entries differ only in that they
+   do not have a 'contact' or 'vote-digest' line. This format stopped appearing
+   as of Tor version XXXXXX.
 
     "dir-source" SP nickname SP identity SP address SP IP SP dirport SP
        orport NL

comment:8 Changed 4 years ago by karsten

Looks fine, though I'm not sure if we really took out this format in recent Tor versions. It's just that no directory authority uses a legacy key right now. But in theory, once a directory authority starts using a legacy key, this format would be back. AFAIK.

comment:9 follow-up: Changed 4 years ago by atagar

Thanks! Added support for '-legacy' entries...

https://gitweb.torproject.org/stem.git/commitdiff/153ada322653f95eb21c360d3cac4fe6ad436ed7

Could you please try running this over some more legacy entries to ensure that there aren't any more hiccups? I assumed that votes still have a mandatory key certificate.

If all is well then we can reassign this to the tor component for the spec addition.

Cheers! -Damian

comment:10 follow-up: Changed 4 years ago by atagar

Looks fine, though I'm not sure if we really took out this format in recent Tor versions.

In that case maybe we should?

I don't have the context on where these come from but if they were a hack and no present authorities use them then it seems like we should drop support for them. Then that "This format stopped appearing as of Tor version XXXXXX." can be the present version.

comment:11 in reply to: ↑ 9 Changed 4 years ago by karsten

Replying to atagar:

Thanks! Added support for '-legacy' entries...

https://gitweb.torproject.org/stem.git/commitdiff/153ada322653f95eb21c360d3cac4fe6ad436ed7

Could you please try running this over some more legacy entries to ensure that there aren't any more hiccups? I assumed that votes still have a mandatory key certificate.

Yes, I'm planning to run this code over all consensuses as part of #7828. Will let you know there and in this ticket how that worked.

If all is well then we can reassign this to the tor component for the spec addition.

Sounds good.

comment:12 in reply to: ↑ 10 Changed 4 years ago by karsten

Replying to atagar:

Looks fine, though I'm not sure if we really took out this format in recent Tor versions.

In that case maybe we should?

I don't have the context on where these come from but if they were a hack and no present authorities use them then it seems like we should drop support for them. Then that "This format stopped appearing as of Tor version XXXXXX." can be the present version.

It was a hack because we needed it quickly, but we may need it in the future again. We shouldn't take it out.

comment:13 Changed 4 years ago by atagar

  • Component changed from Stem to Tor
  • Summary changed from Stem doesn't handle "dir-source *-legacy" lines correctly to dir-spec doesn't cover '-legacy' authority entries

If all is well then we can reassign this to the tor component for the spec addition.

Sounds good.

Reassigning.

comment:14 Changed 4 years ago by nickm

  • Keywords spec tor-client added
  • Milestone set to Tor: 0.2.4.x-final

comment:15 follow-up: Changed 4 years ago by nickm

Does Tor currently even accept that format? From what I can tell, we have T1N for K_CONTACT and K_VOTE_DIGEST in those formats, meaning that they _are_ mandatory in currently generated directory objects.

Maybe it's time to poke around and see when we stopped accepting them or what.

comment:16 in reply to: ↑ 15 Changed 4 years ago by karsten

Replying to nickm:

Does Tor currently even accept that format? From what I can tell, we have T1N for K_CONTACT and K_VOTE_DIGEST in those formats, meaning that they _are_ mandatory in currently generated directory objects.

AFAICS, that's per consensus, not per authority section; and given that an authority with a legacy key must also have a non-legacy key, there's always at least one "contact" line and one "vote-digest" line per consensus.

comment:17 Changed 4 years ago by nickm

Ah, okay. If somebody writes up the right thing here, I'll merge it.

comment:18 Changed 4 years ago by nickm

  • Keywords tor-spec added; spec removed

Bulk-replacing "spec" and "torspec" keywords with "tor-spec".

comment:19 Changed 4 years ago by nickm

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.