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

Re: [tor-bugs] #13642 [Tor]: Implement offline encrypted master keys for Ed25519 identities

#13642: Implement offline encrypted master keys for Ed25519 identities
-------------------------+-------------------------------------------------
     Reporter:  nickm    |      Owner:
         Type:           |     Status:  needs_revision
  enhancement            |  Milestone:  Tor: 0.2.7.x-final
     Priority:  major    |    Version:  Tor: 0.2.7
    Component:  Tor      |   Keywords:  tor-relay, prop-220,
   Resolution:           |  027-triaged-1-in, SponsorU
Actual Points:           |  Parent ID:  #15054
       Points:  small-   |
  remaning               |
-------------------------+-------------------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 Code review:

 * 4a79fad1d095221c80b00b6378a3dd246e358c50
  - In configure.ac: (nitpick) seems to have weird indentation issues:
 {{{
          getifaddrs \
 +    getpass \
          getrlimit \
 }}}
  - In common/compat.c: `tor_getpass` takes a `ssize_t` buflen, I think it
    should be a `size_t` because 1) that shouldn't be negative and 2)
    `readpassphrase()` takes a `size_t` as bufsiz.
  - In common/compat.c: `tor_getpass()`, I think `readpassphrase()`
 shouldn't
    echo the passphrase which can be fixed by using `RPP_ECHO_OFF`.
  - In common/compat.c: I agree with you that the alternatives are not
 pretty :)

 * d8628ee3d04e4ffe89430b12a0c2dae01d0ea5b4
  - In routerkeys.c: function `read_encrypted_secret_key()`, the while(1)
    readpassphrase, when we call crypto_unpwbox(), we don't handle
    `UNPWBOX_BAD_SECRET` so this means we are fine with an infinite loop or
    until the user ctrl+c or enters the right passphrase?
  - In routerkeys.c: function `write_encrypted_secret_key()`, this looks
 like a
    typo, should be `pwbuf1` in the second memwipe I would imagine (and 0
    instead of 1 maybe?)
 {{{
 +  memwipe(pwbuf0, 0, sizeof(pwbuf0));
 +  memwipe(pwbuf0, 1, sizeof(pwbuf1));
 }}}

 The rest lgtm!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13642#comment:8>
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