Improve curve25519-based crypto #6050
Conversation
This is a rewrite of the x25519 code, that generalizes support for common primitives based on the same finite field. - Low-level operations can now be performed over the curve25519 and edwards25519 curves, as well as the ristretto255 group. - Ed25519 signatures have been implemented. - X25519 is now about twice as fast. - mem.timingSafeEqual() has been added for constant-time comparison. Domains have been clearly separated, making it easier to later add platform-specific implementations.
kubkon
added
the
standard library
|
Hey @jedisct1, thanks for the PR! It seems Azure didn't fire up properly. To fix this, would you mind closing and reopening the PR? Believe it or not, but this actually seems to be the best solution! |
|
Hi @kubkon! No worries, the cloud can act funny sometimes. Let me close and reopen this PR, then! |
This requires assembly implementations, and is not needed for signature verification. Thanks @daurnimator
Co-authored-by: Jakub Konka <[email protected]>
Co-authored-by: Jakub Konka <[email protected]>
Suggested by @kubkon, thanks!
* '25519' of github.com:jedisct1/zig: Update lib/std/crypto/25519/ristretto255.zig Update lib/std/crypto/25519/field25519.zig
Suggested by @kubkon
| limbs[4] &= MASK51; | ||
| } | ||
|
|
||
| pub inline fn add(a: Fe, b: Fe) Fe { |
There was a problem hiding this comment.
This is a short, very frequently called function, that can leverage data present in registers without matching the calling convention. This is similar to other functions explicitly marked as inline such as shifts in fmt/parse_float.zig.
We really want these to always be inlined.
But LLVM is extremely likely to inline that function even without the qualifier, so it can be removed.
There was a problem hiding this comment.
Generally inline annotations for performance in the zig standard library require an accompanying benchmark to show there is a material difference.
If the optimiser gets it wrong, we consider that a bug in the optimiser to fix or something we can work around in ZIR.
In other circumstances, inline is used to e.g. make sure there is no frame for the function in a debugger, or because of some linker layout requirement
There was a problem hiding this comment.
Removing all the inlines seems to slightly degrade performance, particularly in release-safe mode.
- x25519/release-safe
- inline: 14792 exchanges/s
- no inline: 13104 exchanges/s
- x25519/release-fast
- inline: 22650 exchanges/s
- no inline: 21930 exchanges/s
There was a problem hiding this comment.
Inlining more (Fe.sub, Fe.mul, Fe.sq) increases performance even more:
- inline more: 3099 exchanges/s
Still in release-safe mode.
There was a problem hiding this comment.
Benchmarks for signatures (using crypto/benchmark.zig)
- x86_64/macOS
- release-safe:
- inline: 12645 signatures/s
- no inline: 10656 signatures/s
- inline more: 16024 signatures/s
- release-fast:
- inline: 18677 signatures/s
- no inline: 18621 signatures/s
- inline more: 19047 signatures/s
- release-safe:
There was a problem hiding this comment.
- aarch64/linux
- release-safe:
- no inline: 1928 signatures/s
- inline more: 3048 signatures/s
- release-fast:
- no inline: 3520 signatures/s
- inline more: 3561 signatures/s
- release-safe:
There was a problem hiding this comment.
So, inlining doesn't make much of a difference in release-fast mode.
However, it can make a big difference in release-safe mode.
With explicit inlining, using release-safe is generally fine, as performance isn't too far from release-fast.
There was a problem hiding this comment.
Same for hash functions.
Inlining their round function makes release-safe essentially free, or even faster than release-fast without explicit inlining (observed with blake3).
| } | ||
|
|
||
| /// Flip the sign of the X coordinate. | ||
| pub inline fn neg(p: Edwards25519) Edwards25519 { |
There was a problem hiding this comment.
Probably shouldn't have inline
* master: stage2: astgen for labeled blocks and labeled breaks stage2: populate some of the astgen switch possibilities stage2 astgen for LabeledBlock std.zig.ast: extract out Node.LabeledBlock from Node.Block stage2: proper semantic analysis of improper returning of implicit void ci linux: bump qemu-5.1.0 stage2: astgen for non-labeled blocks langref: fix html error
* master: stage1: remove buggy "unable to inline function" compile error
|
Looks like the WASI tests found something:
Looks like something here caused a large stack frame (maybe a big array buffer inside a function) You can reproduce it by running wasmtime: |
|
WASI tests are passing again. |
|
Nice work! I rebased your commits in a branch before merging, which is why GitHub shows it as "closed" even though it is really merged. |
This is a rewrite of the x25519 code, that generalizes support for common primitives based on the same finite field.
Domains have been clearly separated, making it easier to later add platform-specific implementations.