#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