feat!: Ed25519 expanded signing key #978
Open
Conversation
This file contains hidden or 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
|
@tarcieri I can put it behind a feature flag if needed. |
|
hey! just chiming it to say thank you for taking the time to develop this feature. it's greatly appreciated and we've found it useful when dealing with yubihsm-exported keys |
Comment on lines
-9
to
+17
| #[derive(Clone, Debug)] | ||
| pub struct SigningKey(ed25519_consensus::SigningKey); | ||
| #[allow(clippy::large_enum_variant)] | ||
| #[derive(Debug)] | ||
| pub enum SigningKey { | ||
| /// Ed25519 signing key. | ||
| Ed25519(ed25519_consensus::SigningKey), | ||
| /// Ed25519 expanded signing key. | ||
| Ed25519Expanded(ExpandedSecretKey), | ||
| } |
There was a problem hiding this comment.
If we're going this route, I think I'd like to just migrate wholesale to ExpandedSecretKey as the internal type, rather than using an enum
There was a problem hiding this comment.
I migrated (back) completely to ed25519-dalek in #991.
You should now be able to implement this entirely in terms of ExpandedSecretKey rather than using this enum.
tarcieri
pushed a commit
that referenced
this pull request
Aug 19, 2025
We've had many requests to leverage `ed25519-dalek`'s `ExpandedSecretKey` functionality in order to support keys exported from YubiHSMs. See #978, #891, and #742. I have been a bit wary of the implementation though, because it has always involved having both `ed25519-consensus` and `ed25519-dalek`, often with enums over multiple key types, which seems incredibly overcomplicated just to implement this feature. We originally switched to `ed25519-consensus` because `tendermint-rs` did, and it makes sense there where ZIP-215 provides consensus-critical signature verification rules that don't diverge between implementations. However that's a bit irrelevant for our purposes as this is a signing service, and ZIP-215 allows us to use whatever signing implementation we want. Now that we've vendored `tendermint-p2p` as `tmkms-p2p` we can unilaterally switch the Ed25519 implementation (back) to `ed25519-dalek`, which should make adding `ExpandedSecretKey` support much easier and avoid having to worry about two implementations. This additionally switches (back) to upstream `curve25519-dalek` so as to support X25519 as part of the SecretConnection protocol.
tarcieri
pushed a commit
that referenced
this pull request
Aug 19, 2025
We've had many requests to leverage `ed25519-dalek`'s `ExpandedSecretKey` functionality in order to support keys exported from YubiHSMs. See #978, #891, and #742. I have been a bit wary of the implementation though, because it has always involved having both `ed25519-consensus` and `ed25519-dalek`, often with enums over multiple key types, which seems incredibly overcomplicated just to implement this feature. We originally switched to `ed25519-consensus` because `tendermint-rs` did, and it makes sense there where ZIP-215 provides consensus-critical signature verification rules that don't diverge between implementations. However that's a bit irrelevant for our purposes as this is a signing service, and ZIP-215 allows us to use whatever signing implementation we want. Now that we've vendored `tendermint-p2p` as `tmkms-p2p` we can unilaterally switch the Ed25519 implementation (back) to `ed25519-dalek`, which should make adding `ExpandedSecretKey` support much easier and avoid having to worry about two implementations. This additionally switches (back) to upstream `curve25519-dalek` so as to support X25519 as part of the SecretConnection protocol.
tony-iqlusion
added a commit
that referenced
this pull request
Aug 19, 2025
We've had many requests to leverage `ed25519-dalek`'s `ExpandedSecretKey` functionality in order to support keys exported from YubiHSMs. See #978, #891, and #742. I have been a bit wary of the implementation though, because it has always involved having both `ed25519-consensus` and `ed25519-dalek`, often with enums over multiple key types, which seems incredibly overcomplicated just to implement this feature. We originally switched to `ed25519-consensus` because `tendermint-rs` did, and it makes sense there where ZIP-215 provides consensus-critical signature verification rules that don't diverge between implementations. However that's a bit irrelevant for our purposes as this is a signing service, and ZIP-215 allows us to use whatever signing implementation we want. Now that we've vendored `tendermint-p2p` as `tmkms-p2p` we can unilaterally switch the Ed25519 implementation (back) to `ed25519-dalek`, which should make adding `ExpandedSecretKey` support much easier and avoid having to worry about two implementations. This additionally switches (back) to upstream `curve25519-dalek` so as to support X25519 as part of the SecretConnection protocol.
tony-iqlusion
requested changes
Aug 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This PR adds support for expanded
ed25519keys, enabling compatibility with different key formats (including those exported from YubiHSM).Copilot Summary
This pull request introduces support for expanded Ed25519 signing keys in the keyring, enabling compatibility with different key formats (including those exported from YubiHSM) and providing more flexible signing capabilities. The changes refactor the
SigningKeytype to handle both standard and expanded keys.Expanded Ed25519 key support and refactoring:
SigningKeytype from a struct to an enum, allowing it to represent either a standard Ed25519 signing key (Ed25519) or an expanded Ed25519 signing key (Ed25519Expanded). This enables handling multiple key formats.SigningKeyto correctly process both key types for serialization (as_bytes), verification key conversion (verifying_key), and external key conversions (e.g., totendermint_p2p::secret_connection::PublicKey).TryFrom<&[u8]> for SigningKeyimplementation to support parsing seed keys, big-endian expanded keys, and little-endian expanded keys (such as those exported from YubiHSM), improving compatibility with external hardware and key sources.Signer<Signature>implementation to use the appropriate signing method for expanded keys (raw_sign), ensuring correct signature generation for both key types.Dependency addition:
ed25519-dalekcrate with thehazmatfeature toCargo.toml, providing the necessary cryptographic primitives for expanded Ed25519 key support.