Re: [tor-bugs] #16467 [Tor]: Faster Ed25519 implementation.

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#16467: Faster Ed25519 implementation.
-----------------------------+------------------------------------
     Reporter:  yawning      |      Owner:
         Type:  enhancement  |     Status:  needs_review
     Priority:  normal       |  Milestone:  Tor: 0.2.7.x-final
    Component:  Tor          |    Version:  Tor: 0.2.7
   Resolution:               |   Keywords:  performance, tor-relay
Actual Points:               |  Parent ID:  #9663
       Points:               |
-----------------------------+------------------------------------

Comment (by yawning):

 Replying to [comment:3 teor]:
 > clang would prefer iters to be a `#define`
 > {{{
 > test_crypto_ed25519_fuzz_donna(void *arg)
 > {
 >   const int iters = 1024;
 >   uint8_t msg[iters];
 > }}}
 > `tor/src/test/test_crypto.c:1643:15: Variable length array folded to
 constant array as an extension`

 Fixed. (`116a56a234631978c46a1bf9ac1649eef8ae6bf4`)

 > OS X defines an `ALIGN` macro to calculate pointer alignment based on a
 passed pointer value. It's not the same as the compiler alignment
 attribute. Can we rename the macro across the ed25519/donna codebase?
 > `tor/src/ext/ed25519/donna/ed25519-donna-portable.h:23:10: 'ALIGN' macro
 redefined`

 I'd rather not (OSX defining `ALIGN` to me is "shitting up the global
 namespace), since it makes it even harder to fold in changes to upstream.
 There's a pull request open that undef's ALIGN
 (https://github.com/floodyberry/ed25519-donna/pull/21), which keeps the
 codebases closer.

 Fixed. (`d83a65041b648a30835be29a9a54cfe34ab5608b`)

 > expand256_modm doesn't appear to initialise the bytes of work above len,
 or, if len is less than 32, the bytes of out above 32.

 Huh? `unsigned char work[64] = {0};` The entirety of `work` is initialized
 to `0`.  This is language standard behavior.  All of `out` gets set as
 well (`typedef bignum256modm_element_t bignum256modm[5];`).

 > Should we be using the di_ops functions for memset, memcpy and similar?

 None of the memsets are in danger of being optimized away (I use `memwipe`
 where this is a possibility).

 The one place where a data dependent memcmp is used outside of tests is in
 the batch verify code, in a routine explicitly tagged as `vartime` I will
 assume the implementer knew what he was doing.

 > There's also a typo in README.tor: "next-gen hidden srevice descriptors"

 Fixed. (`d83a65041b648a30835be29a9a54cfe34ab5608b`)

 > The clang static analyzer to complain about garbage values being passed
 to the + operator in the call stack:

 Are these actual issues or just false positives?  If they're false
 positives, I'm inclined not to change anything (squashing things just to
 appease tools isn't behavior I particularly like).

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