[tor-bugs] #13125 [Tor]: Review the guardiness python script of #9321

[Tor Bug Tracker & Wiki]

#13125: Review the guardiness python script of #9321
------------------------+--------------------------------
     Reporter:  asn     |      Owner:
         Type:  defect  |     Status:  needs_review
     Priority:  normal  |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-guard tor-auth
Actual Points:          |  Parent ID:  #9321
       Points:          |
------------------------+--------------------------------

Comment (by nickm):

 Replying to [comment:14 asn]:
 >
 > Good point. As Damian said, stem is filtering unknown lines and putting
 them in a special list. I can do some testing wrt soon, to make sure it
 works fine.

 Sounds good.

 > > * The output format should be made extensible so that it can handle
 more than
 > >   one ID format.  Soon, Ed25519+RSA identity pairs will exist.  After
 > >   that, Ed25519-alone will exist.
 > >
 > >   * The schema should also probably handle this situation.
 > >
 >
 > That's also a good point.
 >
 > Currently, I'm using the fingerprint of the guard as its unique
 identifier. The problem I guess is that in the future, each guard will
 have 2 such fingerprints. And in the future future, it will have one but
 not the one it has now.
 >
 > How should this be fixed I wonder? Should I change the schema and output
 file format to support multiple fingerprints? So that we include both RSA
 and Ed25519?
 >
 > To do so, I will have to change the output file format quite a bit
 again... Both Python and Tor code will need to change, and unittests will
 need to be rewritten. Should I change the output file format to be yaml or
 something? :/ But then how will Tor parse it...

 I would suggest the following: don't worry about it for now.  We can add
 an option to tell it to output a new format, and have tor accept the old
 format and the new one.

 If we're really smart, we can add a version line as the start for the
 output.

 > > * README should explain how and what to backup.
 > >
 >
 > Hm, what do you mean backup?

 Let's say the system gets wrecked.  What files would we want a copy of to
 restore it?

 > > * Check that we're not going backwards in time?
 > >
 >
 > Hm, how should I do that?
 > Should I take the latest consensus from the database, and make sure that
 the current time is later than the `valid-after` of that consensus?

 That's a good way.

 > > * find_missing_hours_from_list() isn't actually right: We don't
 guarantee a one-hour timer.  Instead you need to check for gaps in the
 (valid-after, fresh-until) series of times.  But this would mean that just
 counting consensuses isn't right.  Maybe each consensus needs a duration-
 fresh field.
 > >
 >
 > Yeah this was a hard problem. The current implementation is a rough
 hack, to give dirauth operators a rough idea of which consensuses are
 missing...
 >
 > > * How does the nested query in read_db_file() perform?  Is it even
 right?  I don't think the parentheses balance.
 >
 > I ```think``` it's right. At least its output seems to be correct, and
 the unittests test it.
 > Which parentheses don't match?

 Okay, if it works okay, and performs okay, that's fine.

 > Thanks for the code review again! We can discuss this during the next
 tor meeting if you want :)

 You're welcome, and thanks for the code!

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