feat: estimate ed25519 base and bytes separately
#7980
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The old estimation just divided the base estimation by the number of bytes to receive the cost per byte. Now the base is subtracted first and the remainder is divided by the number of bytes. Exact estimations will be done and added later. But icount estimations show that the per byte cost can be reduced from the placeholder value 423 Mgas to 12 Mgas (safety multiplier of 3 included). The base value is still a bit tricky to say. `icount` estimation gives me 90 Ggas, so with a safety multiplier it would be 270 Ggas. The current placeholder of 40 Ggas is quite a bit lower but it was roughly what we expected based on [external benchmarks](https://github.com/dalek-cryptography/ed25519-dalek#benchmarks). It could be that qemu based estimation does not work well here because AVX instructions are used. Besides the actual estimation, there is another TODO. The verify function used is not constant-time. So we need to evaluate different inputs for validation and see if the worst-case is significantly different from the time for a random private key and signature. Attack idea: An attack could try many keys until one produces a suitable `k` and combine it with a suitable `s`. Note that `s` does not need to be a valid signature, as long as it is smaller than `q` it cannot be rejected until `[s]B - [k]A` is computed, which is the expensive part. The attacker can do the analysis offline and submit the prepared (invalid) signature to the chain, which could lead to undercharged cost if we just go with a random input for the estimation.
|
cc @blasrodri and link #7567 |
TLDR: variation on input is small and has an upper bound Details: see comments on `Ed25519VerifyBase`.
|
I just updated the comments that expressed worries around timing depending on the input. My conclusion here is that the variance is small enough that we can keep a single signature for the estimation. Side channel attacks are possible even with very small changes in timing. We need to deal with much higer variance between hardware anyway. Thus, we round up estimations generously (e.g. safety multiplier of 3x) and these small variations become negligible. |
matklad
approved these changes
Nov 2, 2022
Comment on lines
+485
to
+486
| // private key: OReNDSAXOnl-U6Wki95ut01ehQW_9wcAF_utjzRNreg | ||
| // public key: M4QwJx4Sogjr0KcMI_gsvt-lEU6tgd9GWmgejE_JYlA |
Co-authored-by: Alex Kladov <aleksey.kladov@gmail.com>
nikurt
pushed a commit
that referenced
this pull request
Nov 7, 2022
The old estimation just divided the base estimation by the number of bytes to receive the cost per byte. Now the base is subtracted first and the remainder is divided by the number of bytes. Exact estimations will be done and added later. But icount estimations show that the per byte cost can be reduced from the placeholder value 423 Mgas to 12 Mgas (safety multiplier of 3 included). The base value is still a bit tricky to say. `icount` estimation gives me 90 Ggas, so with a safety multiplier it would be 270 Ggas. The current placeholder of 40 Ggas is quite a bit lower but it was roughly what we expected based on [external benchmarks](https://github.com/dalek-cryptography/ed25519-dalek#benchmarks). It could be that qemu based estimation does not work well here because AVX instructions are used.
nikurt
pushed a commit
that referenced
this pull request
Nov 9, 2022
The old estimation just divided the base estimation by the number of bytes to receive the cost per byte. Now the base is subtracted first and the remainder is divided by the number of bytes. Exact estimations will be done and added later. But icount estimations show that the per byte cost can be reduced from the placeholder value 423 Mgas to 12 Mgas (safety multiplier of 3 included). The base value is still a bit tricky to say. `icount` estimation gives me 90 Ggas, so with a safety multiplier it would be 270 Ggas. The current placeholder of 40 Ggas is quite a bit lower but it was roughly what we expected based on [external benchmarks](https://github.com/dalek-cryptography/ed25519-dalek#benchmarks). It could be that qemu based estimation does not work well here because AVX instructions are used.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The old estimation just divided the base estimation by the number of
bytes to receive the cost per byte. Now the base is subtracted first and
the remainder is divided by the number of bytes.
Exact estimations will be done and added later. But icount estimations
show that the per byte cost can be reduced from the placeholder value
423 Mgas to 12 Mgas (safety multiplier of 3 included).
The base value is still a bit tricky to say.
icountestimation givesme 90 Ggas, so with a safety multiplier it would be 270 Ggas.
The current placeholder of 40 Ggas is quite a bit lower but it was
roughly what we expected based on
external benchmarks.
It could be that qemu based estimation does not work well here because
AVX instructions are used.