Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ed25519: refactor #1311

Merged
merged 1 commit into from
Mar 13, 2024
Merged

ed25519: refactor #1311

merged 1 commit into from
Mar 13, 2024

Conversation

0x0ece
Copy link
Contributor

@0x0ece 0x0ece commented Mar 11, 2024

Refactor ed25519, x25519. Add ristretto255.

Results:

  • Simplified the code: only 1 impl for protocols
  • Use fiat-crypto as ref implementation, keep r43x6 for avx512
  • Security best practices for sensitive functions (const time, clear stack, clear registers...)
  • Tests from Wycheproof and CCTV
  • Diff fuzz testing against ed25519-dalek v1.0.1 (used by Agave)

Performance:

  1. Ref is significantly faster, essentially by moving from 32-bit arithmetic to 64-bit (and, formally verified!)
  2. Avx is slightly faster. There's no AVX code anymore, it's just ref (I tried to vectorize but get worse performance, left as future todo).
    • Note: it's interesting to see that when we compile with gcc with avx, we actually get worse performance than compiling without avx. So, gcc auto-vectorization of fiat-crypto code is bad (This is also left to future exploration).
  3. Avx512 is pretty much the same. Because it's wrapped in the same iface, it's now available for consumers. So, batch verification, syscalls, programs actually uses avx512 (unlike before).

Not in this PR / for future PR:

  1. scalar (I've added the code from fiat-crypto, but didn't integrate it yet -- it's marginally useful for ed25519, it'll only be useful for programs)
  2. avx backend (since ref is fast enough). We need to impl something like avx512 where the point on the curve are "transposed"

Performance

OLD

                                   &        ref &        avx &   avx-512
                               gcc &   noarch64 &     x86_64 &   icelake
fd_ed25519_public_from_private     &  31718.691 &  19353.846 &  8276.699 ns
fd_ed25519_sign(128)               &  33583.734 &  20620.098 &  9650.514 ns
fd_ed25519_sign(1024)              &  38708.883 &  23724.736 & 12618.239 ns
fd_ed25519_verify(good 128)        & 108215.117 &  53289.055 & 24813.391 ns
fd_ed25519_verify(good 1024)       & 112859.891 &  55605.930 & 26581.926 ns
fd_x25519_exchange                 &  90889.117 &  64284.312 & 25103.035 ns
fd_x25519_public                   &  91261.125 &  64044.039 & 25135.113 ns

                                   &        ref &        avx &   avx-512
                             clang &   noarch64 &     x86_64 &   icelake
fd_ed25519_public_from_private     &  32506.811 &  20393.812 & 10241.384 ns
fd_ed25519_sign(128)               &  34260.387 &  21554.414 & 11651.526 ns
fd_ed25519_sign(1024)              &  39009.402 &  24452.961 & 14599.696 ns
fd_ed25519_verify(good 128)        & 113959.016 &  53838.117 & 27656.070 ns
fd_ed25519_verify(good 1024)       & 118376.656 &  56053.090 & 29499.730 ns
fd_x25519_exchange                 & 100180.359 &  63278.648 & 29062.783 ns
fd_x25519_public                   & 100305.375 &  63234.508 & 29181.174 ns

NEW

                                   &        ref &        avx &    avx-512
                               gcc &   noarch64 &     x86_64 &    icelake
fd_ed25519_public_from_private     &  17534.652 &  19260.412 &   8483.957 ns
fd_ed25519_sign(128)               &  19446.574 &  20548.191 &   9801.010 ns
fd_ed25519_sign(1024)              &  24328.910 &  23531.916 &  12872.107 ns
fd_ed25519_verify(good 128)        &  50120.871 &  49460.293 &  24561.240 ns
fd_ed25519_verify(good 1024)       &  53232.930 &  51375.199 &  26625.250 ns
fd_x25519_exchange                 &  40091.062 &  41203.578 &  23813.031 ns
fd_x25519_public                   &  40063.516 &  41304.203 &  23818.215 ns


                                   &        ref &        avx &    avx-512
                             clang &   noarch64 &     x86_64 &    icelake
fd_ed25519_public_from_private     &  16959.553 &  15723.130 &  10023.702 ns
fd_ed25519_sign(128)               &  18543.150 &  17048.125 &  11170.512 ns
fd_ed25519_sign(1024)              &  23338.172 &  19967.980 &  14284.867 ns
fd_ed25519_verify(good 128)        &  52519.738 &  49233.145 &  27079.184 ns
fd_ed25519_verify(good 1024)       &  55356.863 &  51126.832 &  29144.020 ns
fd_x25519_exchange                 &  44316.691 &  42782.301 &  26512.234 ns
fd_x25519_public                   &  44347.320 &  42774.984 &  26527.486 ns


RUST

                                dalek (rust) &     u64 &    fiat &    simd
Ed25519 keypair generation                   &  18.407 &  19.079 &  18.394 µs
Ed25519 signing                              &  19.217 &  20.215 &  19.207 µs
Ed25519 strict signature verification        &  56.510 &  60.249 &  27.097 µs
X25519  diffie_hellman                       &  56.232 &  57.759 &  55.795 µs

@0x0ece 0x0ece force-pushed the 0x0ece/ed25519 branch 3 times, most recently from 6f22888 to f5c36e1 Compare March 12, 2024 20:04
@0x0ece 0x0ece force-pushed the 0x0ece/ed25519 branch 2 times, most recently from bf7e1d7 to 3129dc5 Compare March 13, 2024 15:19
@0x0ece 0x0ece added this pull request to the merge queue Mar 13, 2024
Merged via the queue into main with commit 89da411 Mar 13, 2024
7 checks passed
@0x0ece 0x0ece deleted the 0x0ece/ed25519 branch March 13, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants