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

Re: [tor-bugs] #17663 [Tor]: Add SHA512 support in crypto.c

#17663: Add SHA512 support in crypto.c
-------------------------+------------------------------------
 Reporter:  teor         |          Owner:
     Type:  enhancement  |         Status:  needs_revision
 Priority:  Medium       |      Milestone:  Tor: 0.2.8.x-final
Component:  Tor          |        Version:
 Severity:  Normal       |     Resolution:
 Keywords:               |  Actual Points:
Parent ID:               |         Points:
  Sponsor:               |
-------------------------+------------------------------------
Changes (by teor):

 * status:  new => needs_revision


Comment:

 Thanks for this feature, and thanks for the unit tests!

 Code review:

 The code looks great overall. Just a few minor suggestions.

 In crypto_digest_smartlist_prefix:
 * if an unknown digest is passed to the function, let's warn,
 tor_fragile_assert(), and wipe memory like crypto_digest_get_digest;
 rather than providing SHA256 as a default.

 In crypto_digest512:
 * the function returns 1 on failure, not -1. (This is likely a copy-paste
 of a similar error on crypto_digest256 that's fixed in #17655.)

 In crypto_digest_local:
 * Using this function will give 32-bit platforms less protection.
 (Typically, we just let them take the performance hit.)
 * I don't like the idea of truncation to 160 bits, although it might be a
 while before attacks on 160 bits become feasible. We could let the user
 specify a length, perhaps by adding SHA_LOCAL_* equivalents of the
 SHANNN_* constants.
 * the function never returns -1. If you can check for failures and return
 -1, please do, otherwise, just document it as always returning 0. (This is
 likely a copy-paste of a similar error on crypto_digest256 that's fixed in
 #17655.)

 Are you happy if we delete crypto_digest_local? Or is there a specific
 circumstance where we might use it?

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