Opened 5 years ago

Closed 4 years ago

#7932 closed defect (fixed)

Clarify that pre-method 9 footers contains signatures

Reported by: karsten Owned by: atagar
Priority: Very Low Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When parsing moria1's two votes from 2011-01-15 17:00:00 and from 18:00:00, Stem complains that the key certificate in the 18:00:00 vote doesn't end with a 'dir-key-certification' line:

ParsingFailure: Key certificates must end with a 'dir-key-certification' line:
dir-key-certificate-version 3
fingerprint D586D18309DED4CD6D57C18FDB97EFA96D330566
dir-key-published 2011-01-09 19:54:35
dir-key-expires 2012-01-09 19:54:35
dir-identity-key
-----BEGIN RSA PUBLIC KEY-----
MIIBigKCAYEAvi5+A+XPw4jxMYhmEI4+MpnaX3dUEbsMGHA+xAMnmVhuxbm3Dn5c
TyhQNY2LOlsieE84UYG+J4dABfaFH4w0l6zUJkuytX4+6WRQontw9puR/IcXkRwM
8Tv/tY675OYRCm9DgDAWfqZM0IgTzSrYRDl8eFPSFCOP0NhMrQZeUrdKgwAXVZWP
xt9nTCwT4K9BMp47LEmZKdEokeVsr0l29Z9v5+r24k9x8EQjDexsoHwlVrxWfarG
1klWssfSFpkMN+FkTQnBC6ByiBh5ZKM5AC/HkVFvuHjehUpfrtNk6XNFcKbDvEIg
qPdg1QWuuSWpZVA+/EwSBtwMNcq9pv60L8Cm9WCJoSC691WByiGwFCy1/XcBI4J/
BkoMEvP3kAxzm92jqGbpFSJawFRPZKy89FDKpha/So3CERQPV0ar+DTpVqDlryWV
N4x1IzpPeSHFj7T74q8qdrxx0wcAjWJ9WYoGQif6FK3hHcmbSGSgyvAFeoYxyUCL
JHkjBCD4WTWVAgMBAAE=
-----END RSA PUBLIC KEY-----
dir-signing-key
-----BEGIN RSA PUBLIC KEY-----
MIGJAoGBAJgW/y9OWLvXnYr9KAqtnEIz+UEXrwcKgCiQLl45s7EftnHLVUqjCJg5
aBKotu+kqo6W2ZKjRP+dkLi8OcjgD1vj/Q0A+248u6pIYAEUg/xJ1T8wBOwzlOuA
yiy082mSAgjhX0yPrl9LDVbnlrUuCOxS2iGjYlotikQw35I5OKdTAgMBAAE=
-----END RSA PUBLIC KEY-----
dir-key-crosscert
-----BEGIN ID SIGNATURE-----
L9b0s7SUZqjDtkZJcegVA9OecbdBljfWlQE3JOsf3QOe5c5G8eGZtOeO1IgEcK2j
pOATJyGVgDigHV9yPEbBNah1q1g56Tdk5XQLr2qFhf6ffZtpFoJkZFo7IWWGIGL+
V0XPcT9ux1el+53Cakik8AN5AApneBm2kA2BEsgswUI=
-----END ID SIGNATURE-----
dir-key-certification
-----BEGIN SIGNATURE-----
jkQrrgvn+7aXSDSDjDv6cUIfTURhklCoib5i3UU4gA/uqf4iJA+21FFi5J2nE8Uo
F2A+XsAdpyaf2bHOMD0ToCOUE3KZFBdV5Njp2beextIkPmNVBdIkhXiKA2KE6CRd
rYz+3XVubcePMc5dRS0Vyhdl87W0j5DHBqkzZq3/mb5wR35cjEiE6lZyQX0Y0ULG
dJkMsAXgsDu7bdW7XQ/VmGM9qPh5r2/gQW6fuESiyDsbKRl3k1VGpHZ7Y/qtz4jr
DE4b4nxz2KHcc7DdSAuHU/0vBa/83tNePUkknvVNeNnZ1fA0z6656woZcRovkw/G
8qL54Fd+rj0StCRM/fav2xjQ9yHNhURroBViYXfi5AigfBipOdWgkCuYxu0U/AQw
D8QXmbwNQ7PQii0HROA5u4no/sHf7z4BrbtKdBbz5lOjaeRreyN1HkGDoXr7utUD
Kd13TpVM7TvIL3sX5WMFfXhK/Ynsm/rmzQpSHPvTb1UnQuAY9q+P6x6LSYNIb7n7
-----END SIGNATURE-----
directory-signature D586D18309DED4CD6D57C18FDB97EFA96D330566 2B83C6BE3EC75E91270444DAE41F80A9EFD38912
-----BEGIN SIGNATURE-----
dG60Bz/UV5isZRwowblGYjZhSpeKDo/t5T6UnnPPpZSFdFiVB2LZczfNB7ydt0s6
Jo8IaR4wAqThtWtHSwoVR3pPmLnlDNruNvSjSGfoCpb9jhnDEh0C399A1rimGRNF
6wVp2TJsNQCk91h/nfAtl9g3pepcefpDTWO0Ckr1Phg=
-----END SIGNATURE-----

Parsing the 17:00:00 vote works fine. I don't see what's different between these two votes, and why Stem wouldn't like the 18:00:00 vote. There are other votes that Stem doesn't like for the same reason. I temporarily uploaded the two votes here for easier analysis, though they're also contained in the metrics tarballs.

Child Tickets

Change History (10)

comment:1 Changed 5 years ago by atagar

Hi Karsten. This made me scratch my head for a little while but I think I now know what's up. Those votes end with...

...
w Bandwidth=20
p reject 1-65535
directory-signature D586D18309DED4CD6D57C18FDB97EFA96D330566 2B83C6BE3EC75E91270444DAE41F80A9EFD38912
-----BEGIN SIGNATURE-----
dG60Bz/UV5isZRwowblGYjZhSpeKDo/t5T6UnnPPpZSFdFiVB2LZczfNB7ydt0s6
Jo8IaR4wAqThtWtHSwoVR3pPmLnlDNruNvSjSGfoCpb9jhnDEh0C399A1rimGRNF
6wVp2TJsNQCk91h/nfAtl9g3pepcefpDTWO0Ckr1Phg=
-----END SIGNATURE-----

Note the lack of a 'directory-footer' line. Instead it has a 'directory-signature' which is the footer for a version 2 network status document. Looks like I missed that the 'directory-footer' line was introduced in consensus method 9.

comment:2 Changed 5 years ago by atagar

Ah ha, I had taken this into account...

https://gitweb.torproject.org/stem.git/blob/HEAD:/test/unit/descriptor/networkstatus/document_v3.py#l546

The footer section is delineated in all votes and consensuses supporting
consensus method 9 and above with the following:

 "directory-footer" NL

It contains two subsections, a bandwidths-weights line and a
directory-signature.
...

I interpret this as meaning "network status v3 documents can only have a footer when using consensus method 9 or later". In these votes however it lacks a properly marked footer yet has a directory-signature line. By my read of the spec that's malformed.

How should footers behave prior to consensus method 9?

comment:3 Changed 5 years ago by karsten

From the beginning of Section 3.3:

   Status documents contain a preamble, an authority section, a list of
   router status entries, and one or more footer signature, in that order.

So, everything after the last router status entry is the footer (signature). It's just that consensus method 9 started to explicitly delineate the footer to include the "bandwidth-weights" line and maybe other lines in the future. So, in consensus methods prior to 9, the footer starts at the "directory-signature" line, or more generally at the first non-router-status-entry line. -- That being said, it might make sense to tweak those parts of dir-spec.txt to make them less confusing.

But: note that Stem parses the 17:00:00 vote without issues, but has trouble parsing the 18:00:00 vote. I don't see what difference between the two votes would cause that. Any ideas? Can you try out parsing these two votes with Stem and take a look yourself?

comment:4 Changed 5 years ago by atagar

  • Component changed from Stem to Tor
  • Priority changed from normal to trivial
  • Summary changed from Stem complains that key certificate in old vote doesn't end with a 'dir-key-certification' line to Clarify that pre-method 9 footers contains signatures

Ahh, I missed that. Thanks!

Pushed a fix...

https://gitweb.torproject.org/stem.git/commitdiff/98a8842c9ab800f9891a01caf35d02f2dfc60809

But: note that Stem parses the 17:00:00 vote without issues, but has trouble parsing the 18:00:00 vote. I don't see what difference between the two votes would cause that. Any ideas? Can you try out parsing these two votes with Stem and take a look yourself?

I did and they both appeared to break the parser. Maybe I misread the output yesterday...

That being said, it might make sense to tweak those parts of dir-spec.txt to make them less confusing.

A small clarification would be nice. Maybe something like...

diff --git a/dir-spec.txt b/dir-spec.txt
index ca5434b..eb1b55b 100644
--- a/dir-spec.txt
+++ b/dir-spec.txt
@@ -1489,6 +1489,9 @@
    It contains two subsections, a bandwidths-weights line and a
    directory-signature.
 
+   Prior to conensus method 9 footers only contained directory-signatures
+   without a 'directory-footer' line.
+
    The bandwidths-weights line appears At Most Once for a consensus. It does
    not appear in votes.

Sending this over to Nick to see what he thinks.

comment:5 Changed 5 years ago by nickm

  • Resolution set to fixed
  • Status changed from new to closed

Merged that sentence approximately. Thanks

comment:6 Changed 5 years ago by karsten

  • Component changed from Tor to Stem
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening this issue, but for the Stem side. I found that ides published 52 votes in January 2010 supporting consensus method 9 but without "directory-footer" line. I'm not sure what to do there. On the one hand it's no big harm for Stem to ignore them as malformed, but on the other hand it might be useful for some analyses to parse them anyway. Here's an example, and here's Stem's output: "skipped /home/karsten/tasks/task-7828/stem/data/votes-2010-01/28/2010-01-28-00-00-00-vote-27B6B5996C426270A5C95488AA5BCEB6BCC86956-7B8C92F717C508C203EBD626B5969AEB7D211BDE due to 'Network status document's footer should start with a 'directory-footer' line in consensus-method 9 or later' (type: <class 'stem.descriptor.reader.ParsingFailure'>)". I don't feel strongly here. Up to you, atagar.

comment:7 Changed 5 years ago by atagar

Hmmm. Users can turn off validation to parse this, but it would be nice if stem accepted all of the descriptor archives.

Do you know how this happened? Was this a tor bug? Ideally I'd like to add a check that says 'if the voting authority is a version with this bug then account for it', but it looks like ides' version isn't noted in the consensus.

comment:8 Changed 4 years ago by atagar

  • Status changed from reopened to needs_information

Swapping the status. I'm happy to make stem parse those descriptors, though it would be nice to know where they came from.

comment:9 Changed 4 years ago by karsten

My guess is that this was just testing code in the public Tor network. Stem's probably doing the right thing, rejecting these votes when validation is on. I'd say feel free to close this ticket.

comment:10 Changed 4 years ago by atagar

  • Resolution set to fixed
  • Status changed from needs_information to closed

Sounds good. Resolving.

Note: See TracTickets for help on using tickets.