[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #12498 [Tor]: Implement ed25519 identity keys (prop 220)

#12498: Implement ed25519 identity keys (prop 220)
-------------------------+-------------------------------------------------
     Reporter:  asn      |      Owner:  nickm
         Type:  task     |     Status:  needs_review
     Priority:  major    |  Milestone:  Tor: 0.2.7.x-final
    Component:  Tor      |    Version:  Tor: 0.2.7
   Resolution:           |   Keywords:  026-triaged-1, 027-triaged-1-in,
Actual Points:           |  SponsorU
       Points:  large    |  Parent ID:  #15054
-------------------------+-------------------------------------------------

Comment (by andrea):

 By request, the same code review with just the stuff I was bothered about:

 {{{
 Code review for nickm's 12498_ed25519_keys_v5 branch - just the complaints
 version:

 cf9d780b570fa3ebf02e555c45f62d8b1bc38bcf:

  - In routerkeys.c:

    - load_ed_keys() look okay modulo the two XXXX comments

  - In torcert.c:

    - tor_cert_sign_impl() leaks memory (encoded is never freed), but
 otherwise
      appears correct

 567e42e894c2d06f3934bc90f7f75c9154481023:

  - Adds the crypto_digest_smartlist_prefix() utility function in
    src/commom/crypto.c; looks correct but comment doesn't describe
    the new prepend arg.

  - Why are ed25519_signature_from/to_base64() declared in
 crypto_ed25519.h,
    but defined in crypto_format.c?

 f7931c11cb37c4e1f6d85800ae113b43df44d9f6:

  - Key-pinning mechanism; I presume 'associated Ed25519 key' in commit
    message should be 'associated RSA key'

 1e3a98f88d5e19239d00356d50f6b598a681d70c:

  - As a question of sysadminning the dirauths, one probably wants a way
    to keep backups of the keypin journal, and copying it out from under
    a running Tor process might lead to a corrupt copy with partially
    written lines.  Should we consider making any provision for backups
    of the keypin journal without stopping the dirauth's Tor process?

 41cbaf0f267b0d1831aa3cf42e9d279cb171bc6a:

  - We're switching microdescriptors in votes over to containing ed25519
 lines
    instead of rsa1024 lines if we have a recent enough consensus method;
 are
    we sure instead of rather than in addition to is the right choice here?

 72d0d2c9c44cb6df47b35c07f94898f952a52fbc:

  - Are we sure checking generated files into the repository like this is
    the right thing vs. generating them at build time?

 End code review
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12498#comment:25>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs