[tor-bugs] #22029 [Core Tor/Tor]: Allow ed25519 keys to be banned in the approved-routers file

[Tor Bug Tracker & Wiki]

#22029: Allow ed25519 keys to be banned in the approved-routers file
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  neel
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.4.1.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  034-triage-20180328,                 |  Actual Points:
  034-removed-20180328                           |
Parent ID:                                       |         Points:  1
 Reviewer:  asn                                  |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:23 neel]:
 > I have simplified the if loop and pushed it.
 >
 > I do not plan to write a test for `add_ed25519_to_dir()`.

 Hello neel,

 sounds good wrt not writing tests.

 However, I still feel that the parts of `dirserv_load_fingerprint_file()`
 that got changed are still much measier than before. In particular:
 - It's not clear to me what happens if `is_rsa` and `is_ed25519` are
 false. Is this case handled? Should it be handled?
 - It's not clear to me why the `if` statement needs to be `if
 ((a||b)||c)`. Is this equivalent to `(a||b||c)`? Why the extra parentheses
 or why so many stuff in the same `if`? It's not clear to me what it's
 trying to do. Like, will it send a `log_notice` everytime `is_rsa` is
 true? That does not seem like the previous behavior. Please it would be
 great if this could be simplified and also a comment added (perhaps even
 in its own function).

 My suggestion would be to move the `nickname` checking above the key-type
 checking and then do stuff based on the key-type more cleanly and more
 commented.

 Sorry for the nitpicks. Thanks! :)

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