-
Notifications
You must be signed in to change notification settings - Fork 224
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
Consider ed25519-zebra
crate?
#355
Consider ed25519-zebra
crate?
#355
Comments
Just to update/clarify this: ZIP 215 defines precise validation criteria for Ed25519 signatures and verification keys to ensure that there are no edge cases in verification and that individual and batch verification produce identical results. However, while these criteria are internally consistent (and thus suitable for consensus-critical applications), they aren't exactly the same as RFC8032 (which doesn't require conformant implementations to agree on whether signatures are valid) or the Go standard library (which does not conform to RFC8032 because it allows non-canonical point encodings). However, it would not be difficult to implement ZIP 215 rules in Go and use that instead of the standard library implementation, and it might be preferable for a consensus-critical system like Tendermint to do so. I'm not sure it's safe for The tracking issue for ZIP 215 support in |
Wow thanks for raising this and working on it. Is it correct that neither ed25519-dalek nor ed25519-zebra are expected to be compatible with Go's x/crypto/ed25519 ? And does the ambiguity in RFC 8032 apply even if not doing batch verification? And what does ed25519-zebra 1.X target? Is it compatibility with some C++ implementation? Presumably this is something other blockchain systems are going to be grappling with as well as ed25519 adoption picks up and we'd presumably want to see something like ZIP 215 established as a common standard for libraries to target. We should probably open a corresponding issue in the Tendermint Go repo and target the next block protocol breaking release which will also include other changes to prepare Tendermint to support signature aggregation (eg. tendermint/tendermint#2840) |
Thanks, I'm glad it's useful work! I've been working on a cross-ecosystem survey of how different implementations behave on these edge cases using a list of test vectors I created for ZIP 215 and a blog post writeup explaining more of the context and motivation, but I haven't finished it yet. I'd be happy to share a draft with you if you'd like.
Yes, ed25519-zebra 1.x is expected to be compatible with the specific point release of libsodium originally used in Zcash, and ed25519-zebra 2.x targets the ZIP 215 rules. In general I think it's not safe to assume that any Ed25519 implementations are compatible unless they specifically advertise compatibility with each other. Even if they are coincidentally compatible at one point, if that compatibility isn't an explicit goal, a future release might be incompatible. (This is what happened with libsodium, which changed validation criteria in point releases).
The ambiguity is in the spec, in the sense that conformant implementations are not required to agree on which signatures are valid. It's possible to select a precise set of rules, but then it's not that an implementation is conformant with RFC 8032 or not, but with some specific choices. Basically there are two choices to make:
Each pair of choices on these decision points gives a different set of validation criteria. The ZIP 215 rules are obtained by choosing yes to both, in order to allow batch verification and to ease the upgrade process.
Yes, I'd like it if other projects adopted these rules. To this end I've started work on a Go implementation of these rules here: https://github.com/hdevalence/ed25519consensus So far this just has vendored crypto/ed25519 source, but I should have the ZIP 215 rules (& tests) done soon. I'm less familiar with the practical mechanics of "doing Go" (from the software engineering point of view, releases, publishing, modules, api design, etc), so if you know anyone who'd like to help I'd appreciate a second set of eyes on it. |
Update: I've added a |
cf golang/go#40478 (comment) with description of the current consensus rules inherited by tendermint from crypto/ed25519 |
Update to this -- since this issue was filed, Tendermint now uses the ZIP215 rules. I've created https://github.com/penumbra-zone/ed25519-consensus for use in Penumbra. |
Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library.
…#1067) * Use ed25519-consensus instead of ed25519-dalek Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library. * clippy fixes Co-authored-by: Thane Thomson <[email protected]> * Remove redundant dependency Signed-off-by: Thane Thomson <[email protected]> * cargo fmt Signed-off-by: Thane Thomson <[email protected]> * Add changelog entries Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Henry de Valence <[email protected]>
…#1067) * Use ed25519-consensus instead of ed25519-dalek Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library. * clippy fixes Co-authored-by: Thane Thomson <[email protected]> * Remove redundant dependency Signed-off-by: Thane Thomson <[email protected]> * cargo fmt Signed-off-by: Thane Thomson <[email protected]> * Add changelog entries Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Henry de Valence <[email protected]>
…#1067) * Use ed25519-consensus instead of ed25519-dalek Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library. * clippy fixes Co-authored-by: Thane Thomson <[email protected]> * Remove redundant dependency Signed-off-by: Thane Thomson <[email protected]> * cargo fmt Signed-off-by: Thane Thomson <[email protected]> * Add changelog entries Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Henry de Valence <[email protected]>
…#1067) * Use ed25519-consensus instead of ed25519-dalek Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library. * clippy fixes Co-authored-by: Thane Thomson <[email protected]> * Remove redundant dependency Signed-off-by: Thane Thomson <[email protected]> * cargo fmt Signed-off-by: Thane Thomson <[email protected]> * Add changelog entries Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Henry de Valence <[email protected]>
…#1067) * Use ed25519-consensus instead of ed25519-dalek Closes informalsystems#355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library. * clippy fixes Co-authored-by: Thane Thomson <[email protected]> * Remove redundant dependency Signed-off-by: Thane Thomson <[email protected]> * cargo fmt Signed-off-by: Thane Thomson <[email protected]> * Add changelog entries Signed-off-by: Thane Thomson <[email protected]> Co-authored-by: Henry de Valence <[email protected]>
* deps: Use ed25519-consensus instead of ed25519-dalek (#1067) * Use ed25519-consensus instead of ed25519-dalek Closes #355 (see that issue for more context; `ed25519-consensus` is a fork of `ed25519-zebra` that's Zcash-independent). This change ensures that `tendermint-rs` has the same signature verification as `tendermint`, which uses the validation criteria provided by the Go `ed25519consensus` library. Co-authored-by: Thane Thomson <[email protected]> Co-authored-by: Henry de Valence <[email protected]>
Presently the
tendermint
crate uses theed25519-dalek
vicariously through thesignatory-dalek
crate.There's an alternative I think that's worth considering:
ed25519-zebra
. Bothed25519-dalek
anded25519-zebra
are based on the underlyingcurve25519-dalek
library. However,ed25519-zebra
focuses on Ed25519 usage in consensus-critical (i.e. blockchain) contexts, where discrepancies in Ed25519 validation have been problematic in the past. Also,ed25519-zebra
is written bycurve25519-dalek
author @hdevalence.There are a few issues at play here: existing Ed25519 libraries don't properly implement RFC 8032, and RFC 8032 doesn't go far enough to remove edge cases in verification. Per @hdevalence, he aims for
ed25519-zebra
to be the first Ed25519 library to actually implement RFC 8032 verification logic correctly.If you do decide to adopt
ed25519-zebra
it seems like you'd want to ensure the Go side has a matching solution, either by implementing the same verification checks thated25519-zebra
uses, or by usinged25519-zebra
directly. I'm guessing the latter might be problematic as it seems unlikely Tendermint would adopt a Rust-based dependency.More generally, even if you don't wind up adopting
ed25519-zebra
, I think it would make sense to more precisely specify the Ed25519 verification logic Tendermint uses as part of an overall Tendermint specification, as I assume it's presently "whatever Go does".This seems like the sort of change it might be good to introduce in a chain upgrade (e.g. Stargate). Namely Tendermint only uses Ed25519 signatures for consensus, so previous chains' Ed25519 signatures are irrelevant (only the ECDSA/secp256k1 signed transactions are persisted across chain upgrades). This means introducing stricter Ed25519 verification logic during a chain upgrade carries no risk (to my knowledge) of existing signatures which previously verified but wouldn't verify under the stricter logic, since it's a brand new chain and therefore has no existing Ed25519 signatures to worry about.
/cc @zmanian
The text was updated successfully, but these errors were encountered: