From 387e3c4907f832b814b225bc65a89af620ce7aca Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 31 Dec 2019 15:10:05 -0500 Subject: [PATCH] [WIP] ECDSA (secp256k1) key support Support for ECDSA (secp256k1) account keys, with the goal of using them to support KMS-backed Cosmos transaction signing (#386) --- Cargo.lock | 19 +++++++ Cargo.toml | 9 ++-- src/chain/registry.rs | 32 +++++------ src/config/provider/softsign.rs | 18 +++++++ src/keyring.rs | 95 +++++++++++++++++++++++++------- src/keyring/ecdsa.rs | 9 ++++ src/keyring/ecdsa/signer.rs | 57 ++++++++++++++++++++ src/keyring/ecdsa/softsign.rs | 96 +++++++++++++++++++++++++++++++++ src/keyring/ed25519/ledgertm.rs | 5 +- src/keyring/ed25519/signer.rs | 2 +- src/keyring/ed25519/softsign.rs | 5 +- src/keyring/ed25519/yubihsm.rs | 5 +- src/session.rs | 2 +- 13 files changed, 305 insertions(+), 49 deletions(-) create mode 100644 src/keyring/ecdsa.rs create mode 100644 src/keyring/ecdsa/signer.rs create mode 100644 src/keyring/ecdsa/softsign.rs diff --git a/Cargo.lock b/Cargo.lock index e0f2930..9cd8bef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1458,6 +1458,22 @@ dependencies = [ "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "secp256k1" +version = "0.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "secp256k1-sys 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "secp256k1-sys" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cc 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "secrecy" version = "0.6.0" @@ -2329,6 +2345,7 @@ dependencies = [ "pbkdf2 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "ring 0.16.9 (registry+https://github.com/rust-lang/crates.io-index)", "rusb 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", + "secp256k1 0.17.1 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", "sha2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2550,6 +2567,8 @@ dependencies = [ "checksum salsa20-core 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "2fe6cc1b9f5a5867853ade63099de70f042f7679e408d1ffe52821c9248e6e69" "checksum scopeguard 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "94258f53601af11e6a49f722422f6e3425c52b06245a5cf9bc09908b174f5e27" "checksum secp256k1 0.15.5 (registry+https://github.com/rust-lang/crates.io-index)" = "4d311229f403d64002e9eed9964dfa5a0a0c1ac443344f7546bf48e916c6053a" +"checksum secp256k1 0.17.1 (registry+https://github.com/rust-lang/crates.io-index)" = "94caa8148c403b862eb1da1d2e3fc9ea9c38f76491202058779dc9ff36cf4d8d" +"checksum secp256k1-sys 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a8438091ed5420083ccf4ed23090c1663ad6863881366471d3aeeb3d21853cbb" "checksum secrecy 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9182278ed645df3477a9c27bfee0621c621aa16f6972635f7f795dae3d81070f" "checksum semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" "checksum semver-parser 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" diff --git a/Cargo.toml b/Cargo.toml index a1a1328..fbf131a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ serde_json = "1" sha2 = "0.8" signatory = { version = "0.17", features = ["ecdsa", "ed25519", "encoding"] } signatory-dalek = "0.17" -signatory-secp256k1 = "0.17" +signatory-secp256k1 = { version = "0.17", optional = true } signatory-ledger-tm = { version = "0.17", optional = true } subtle = "2" subtle-encoding = { version = "0.4", features = ["bech32-preview"] } @@ -45,17 +45,14 @@ thiserror = "1" tiny-bip39 = "0.6" wait-timeout = "0.2" x25519-dalek = "0.5" -yubihsm = { version = "0.30", features = ["setup", "usb"], optional = true } +yubihsm = { version = "0.30", features = ["secp256k1", "setup", "usb"], optional = true } zeroize = "1" [dev-dependencies] +abscissa_core = { version = "0.5", features = ["testing"] } tempfile = "3" rand = "0.7" -[dev-dependencies.abscissa_core] -version = "0.5" -features = ["testing"] - [features] default = [] softsign = [] diff --git a/src/chain/registry.rs b/src/chain/registry.rs index 9456aaf..a57f4fd 100644 --- a/src/chain/registry.rs +++ b/src/chain/registry.rs @@ -3,7 +3,6 @@ use super::{Chain, Guard, Id}; use crate::{ error::{Error, ErrorKind::*}, - keyring, prelude::*, }; use lazy_static::lazy_static; @@ -19,25 +18,6 @@ lazy_static! { pub struct Registry(BTreeMap); impl Registry { - /// Add a key to a keyring for a chain stored in the registry - pub fn add_to_keyring( - &mut self, - chain_id: &Id, - signer: keyring::ed25519::Signer, - ) -> Result<(), Error> { - // TODO(tarcieri): - let chain = self.0.get_mut(chain_id).ok_or_else(|| { - format_err!( - InvalidKey, - "can't add signer {} to unregistered chain: {}", - signer.provider(), - chain_id - ) - })?; - - chain.keyring.add(signer) - } - /// Register a `Chain` with the registry pub fn register_chain(&mut self, chain: Chain) -> Result<(), Error> { let chain_id = chain.id; @@ -54,6 +34,18 @@ impl Registry { pub fn get_chain(&self, chain_id: &Id) -> Option<&Chain> { self.0.get(chain_id) } + + /// Get a mutable reference to the given chain + pub(crate) fn get_chain_mut(&mut self, chain_id: &Id) -> Result<&mut Chain, Error> { + self.0.get_mut(chain_id).ok_or_else(|| { + format_err!( + InvalidKey, + "can't add signer to unregistered chain: {}", + chain_id + ) + .into() + }) + } } /// Global registry of blockchain networks known to the KMS diff --git a/src/config/provider/softsign.rs b/src/config/provider/softsign.rs index 103d70a..0ad1d21 100644 --- a/src/config/provider/softsign.rs +++ b/src/config/provider/softsign.rs @@ -7,6 +7,7 @@ use crate::{ }; use serde::Deserialize; use std::{ + fmt::{self, Display}, path::{Path, PathBuf}, str::FromStr, }; @@ -54,6 +55,17 @@ pub enum KeyFormat { Json, } +impl KeyFormat { + /// Get a string reference describing this key format + pub fn as_str(&self) -> &str { + match self { + KeyFormat::Raw => "raw", + KeyFormat::Base64 => "base64", + KeyFormat::Json => "json", + } + } +} + impl Default for KeyFormat { fn default() -> Self { // TODO(tarcieri): change to Base64 @@ -61,6 +73,12 @@ impl Default for KeyFormat { } } +impl Display for KeyFormat { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + impl FromStr for KeyFormat { type Err = Error; diff --git a/src/keyring.rs b/src/keyring.rs index ef776a0..0258c3a 100644 --- a/src/keyring.rs +++ b/src/keyring.rs @@ -1,10 +1,10 @@ //! Signing keyring. Presently specialized for Ed25519. +pub mod ecdsa; pub mod ed25519; pub mod format; pub mod providers; -use self::ed25519::Signer; pub use self::{format::Format, providers::SigningProvider}; use crate::{ chain, @@ -12,7 +12,7 @@ use crate::{ error::{Error, ErrorKind::*}, prelude::*, }; -use std::collections::BTreeMap; +use std::collections::BTreeMap as Map; use subtle_encoding; use tendermint::TendermintKey; @@ -21,8 +21,11 @@ pub type SecretKeyEncoding = subtle_encoding::Base64; /// Signing keyring pub struct KeyRing { - /// Keys in the keyring - keys: BTreeMap, + /// ECDSA (secp256k1) keys in the keyring + ecdsa_keys: Map, + + /// Ed25519 keys in the keyring + ed25519_keys: Map, /// Formatting configuration when displaying keys (e.g. bech32) format: Format, @@ -32,14 +35,44 @@ impl KeyRing { /// Create a new keyring pub fn new(format: Format) -> Self { Self { - keys: BTreeMap::new(), + ecdsa_keys: Map::new(), + ed25519_keys: Map::new(), format, } } - /// Add a key to the keyring, returning an error if we already have a - /// signer registered for the given public key - pub fn add(&mut self, signer: Signer) -> Result<(), Error> { + /// Add an ECDSA key to the keyring, returning an error if we already + /// have a signer registered for the given public key. + pub fn add_ecdsa(&mut self, signer: ecdsa::Signer) -> Result<(), Error> { + let provider = signer.provider(); + let public_key = signer.public_key(); + let public_key_serialized = self.format.serialize(public_key); + let key_type = match public_key { + TendermintKey::AccountKey(_) => "account", + TendermintKey::ConsensusKey(_) => "consensus", + }; + + info!( + "[keyring:{}] added ECDSA {} key {}", + provider, key_type, public_key_serialized + ); + + if let Some(other) = self.ecdsa_keys.insert(public_key, signer) { + fail!( + InvalidKey, + "[keyring:{}] duplicate ECDSA key {} already registered as {}", + provider, + public_key_serialized, + other.provider(), + ) + } else { + Ok(()) + } + } + + /// Add an Ed25519 key to the keyring, returning an error if we already + /// have a signer registered for the given public key. + pub fn add_ed25519(&mut self, signer: ed25519::Signer) -> Result<(), Error> { let provider = signer.provider(); let public_key = signer.public_key(); let public_key_serialized = self.format.serialize(public_key); @@ -49,14 +82,14 @@ impl KeyRing { }; info!( - "[keyring:{}] added {} key {}", + "[keyring:{}] added Ed25519 {} key {}", provider, key_type, public_key_serialized ); - if let Some(other) = self.keys.insert(public_key, signer) { + if let Some(other) = self.ed25519_keys.insert(public_key, signer) { fail!( InvalidKey, - "[keyring:{}] duplicate key {} already registered as {}", + "[keyring:{}] duplicate Ed25519 key {} already registered as {}", provider, public_key_serialized, other.provider(), @@ -66,9 +99,9 @@ impl KeyRing { } } - /// Get the default public key for this keyring - pub fn default_pubkey(&self) -> Result { - let mut keys = self.keys.keys(); + /// Get the default Ed25519 public key for this keyring + pub fn default_ed25519_pubkey(&self) -> Result { + let mut keys = self.ed25519_keys.keys(); if keys.len() == 1 { Ok(*keys.next().unwrap()) @@ -77,19 +110,45 @@ impl KeyRing { } } - /// Sign a message using the secret key associated with the given public key - /// (if it is in our keyring) + /// Sign a message using the ECDSA secret key associated with the given + /// public key (if it is in our keyring) + pub fn sign_ecdsa( + &self, + public_key: Option<&TendermintKey>, + msg: &[u8], + ) -> Result { + let signer = match public_key { + Some(public_key) => self.ecdsa_keys.get(public_key).ok_or_else(|| { + format_err!(InvalidKey, "not in keyring: {}", public_key.to_bech32("")) + })?, + None => { + let mut vals = self.ecdsa_keys.values(); + + if vals.len() > 1 { + fail!(SigningError, "expected only one key in keyring"); + } else { + vals.next() + .ok_or_else(|| format_err!(InvalidKey, "keyring is empty"))? + } + } + }; + + signer.sign(msg) + } + + /// Sign a message using the Ed25519 secret key associated with the given + /// public key (if it is in our keyring) pub fn sign_ed25519( &self, public_key: Option<&TendermintKey>, msg: &[u8], ) -> Result { let signer = match public_key { - Some(public_key) => self.keys.get(public_key).ok_or_else(|| { + Some(public_key) => self.ed25519_keys.get(public_key).ok_or_else(|| { format_err!(InvalidKey, "not in keyring: {}", public_key.to_bech32("")) })?, None => { - let mut vals = self.keys.values(); + let mut vals = self.ed25519_keys.values(); if vals.len() > 1 { fail!(SigningError, "expected only one key in keyring"); diff --git a/src/keyring/ecdsa.rs b/src/keyring/ecdsa.rs new file mode 100644 index 0000000..ca547ef --- /dev/null +++ b/src/keyring/ecdsa.rs @@ -0,0 +1,9 @@ +//! ECDSA (secp256k1) signing keys + +pub use signatory::ecdsa::curve::secp256k1::{FixedSignature as Signature, PublicKey, SecretKey}; + +pub mod signer; +#[cfg(feature = "softsign")] +pub mod softsign; + +pub use self::signer::Signer; diff --git a/src/keyring/ecdsa/signer.rs b/src/keyring/ecdsa/signer.rs new file mode 100644 index 0000000..51d244b --- /dev/null +++ b/src/keyring/ecdsa/signer.rs @@ -0,0 +1,57 @@ +//! ECDSA (secp256k1) signer + +use super::Signature; +use crate::{ + error::{Error, ErrorKind::*}, + keyring::SigningProvider, + prelude::*, +}; +use signatory::signature; +use std::sync::Arc; +use tendermint::TendermintKey; + +/// Trait object wrapper for ECDSA signers +#[derive(Clone)] +pub struct Signer { + /// Provider for this signer + provider: SigningProvider, + + /// Tendermint public key + public_key: TendermintKey, + + /// Signer trait object + signer: Arc + Send + Sync>>, +} + +impl Signer { + /// Create a new ECDSA signer + pub fn new( + provider: SigningProvider, + public_key: TendermintKey, + signer: Box + Send + Sync>, + ) -> Self { + Self { + provider, + public_key, + signer: Arc::new(signer), + } + } + + /// Get the Tendermint public key for this signer + pub fn public_key(&self) -> TendermintKey { + self.public_key + } + + /// Get the provider for this signer + pub fn provider(&self) -> SigningProvider { + self.provider + } + + /// Sign the given message using this signer + pub fn sign(&self, msg: &[u8]) -> Result { + Ok(self + .signer + .try_sign(msg) + .map_err(|e| format_err!(SigningError, "{}", e))?) + } +} diff --git a/src/keyring/ecdsa/softsign.rs b/src/keyring/ecdsa/softsign.rs new file mode 100644 index 0000000..d70dcb8 --- /dev/null +++ b/src/keyring/ecdsa/softsign.rs @@ -0,0 +1,96 @@ +//! libsecp256k1 software-based signer +//! +//! This is mainly intended for testing/CI. Ideally validators will use HSMs. + +use super::{SecretKey, Signer}; +use crate::{ + chain, + config::provider::softsign::{KeyFormat, SoftsignConfig}, + error::{Error, ErrorKind::*}, + keyring::SigningProvider, + prelude::*, +}; +use signatory::public_key::PublicKeyed; +use signatory_secp256k1::EcdsaSigner; +use std::{convert::TryFrom, fs}; +use subtle_encoding::base64; +use tendermint::TendermintKey; +use zeroize::Zeroizing; + +/// Create software-backed ECDSA signer objects from the given configuration +pub fn init(chain_registry: &mut chain::Registry, configs: &[SoftsignConfig]) -> Result<(), Error> { + if configs.is_empty() { + return Ok(()); + } + + if configs.len() != 1 { + fail!( + ConfigError, + "expected one [providers.softsign] in config, found: {}", + configs.len() + ); + } + + let config = &configs[0]; + let key_format = config.key_format.as_ref().cloned().unwrap_or_default(); + + let secret_key: SecretKey = match key_format { + KeyFormat::Base64 => { + let secret_key_base64 = + Zeroizing::new(fs::read_to_string(&config.path).map_err(|e| { + format_err!( + ConfigError, + "couldn't read key from {}: {}", + &config.path.as_ref().display(), + e + ) + })?); + + // TODO(tarcieri): constant-time string trimming + let secret_key_bytes = Zeroizing::new( + base64::decode(secret_key_base64.trim_end().as_bytes()).map_err(|e| { + format_err!( + ConfigError, + "can't decode key from {}: {}", + config.path.as_ref().display(), + e + ) + })?, + ); + + SecretKey::try_from(secret_key_bytes.as_ref()).map_err(|e| { + format_err!( + ConfigError, + "can't decode key from {}: {}", + config.path.as_ref().display(), + e + ) + })? + } + other => fail!( + ConfigError, + "unsupported encoding `{}` for ECDSA key: {}", + other, + config.path.as_ref().display() + ), + }; + + let provider = EcdsaSigner::from(&secret_key); + let public_key = provider.public_key().map_err(|_| Error::from(InvalidKey))?; + let consensus_pubkey = TendermintKey::ConsensusKey(public_key.into()); + + let signer = Signer::new( + SigningProvider::SoftSign, + consensus_pubkey, + Box::new(provider), + ); + + for chain_id in &config.chain_ids { + chain_registry + .get_chain_mut(chain_id)? + .keyring + .add_ecdsa(signer.clone())?; + } + + Ok(()) +} diff --git a/src/keyring/ed25519/ledgertm.rs b/src/keyring/ed25519/ledgertm.rs index 4ad550a..f10633b 100644 --- a/src/keyring/ed25519/ledgertm.rs +++ b/src/keyring/ed25519/ledgertm.rs @@ -41,7 +41,10 @@ pub fn init( ); for chain_id in &ledgertm_configs[0].chain_ids { - chain_registry.add_to_keyring(chain_id, signer.clone())?; + chain_registry + .get_chain_mut(chain_id)? + .keyring + .add_ed25519(signer.clone())?; } Ok(()) diff --git a/src/keyring/ed25519/signer.rs b/src/keyring/ed25519/signer.rs index 639be2c..c104148 100644 --- a/src/keyring/ed25519/signer.rs +++ b/src/keyring/ed25519/signer.rs @@ -1,4 +1,4 @@ -//! Wrapper for Ed25519 signers +//! Ed25519 signer use crate::{ error::{Error, ErrorKind::*}, diff --git a/src/keyring/ed25519/softsign.rs b/src/keyring/ed25519/softsign.rs index 24e42d4..087f54d 100644 --- a/src/keyring/ed25519/softsign.rs +++ b/src/keyring/ed25519/softsign.rs @@ -106,7 +106,10 @@ pub fn init(chain_registry: &mut chain::Registry, configs: &[SoftsignConfig]) -> ); for chain_id in &config.chain_ids { - chain_registry.add_to_keyring(chain_id, signer.clone())?; + chain_registry + .get_chain_mut(chain_id)? + .keyring + .add_ed25519(signer.clone())?; } Ok(()) diff --git a/src/keyring/ed25519/yubihsm.rs b/src/keyring/ed25519/yubihsm.rs index 0f6fc06..e0bfcf1 100644 --- a/src/keyring/ed25519/yubihsm.rs +++ b/src/keyring/ed25519/yubihsm.rs @@ -51,7 +51,10 @@ pub fn init( let signer = Signer::new(SigningProvider::Yubihsm, consensus_pubkey, Box::new(signer)); for chain_id in &config.chain_ids { - chain_registry.add_to_keyring(chain_id, signer.clone())?; + chain_registry + .get_chain_mut(chain_id)? + .keyring + .add_ed25519(signer.clone())?; } } diff --git a/src/session.rs b/src/session.rs index c41a807..c8f159a 100644 --- a/src/session.rs +++ b/src/session.rs @@ -194,7 +194,7 @@ impl Session { let chain = registry.get_chain(&self.config.chain_id).unwrap(); Ok(Response::PublicKey(PubKeyResponse::from( - *chain.keyring.default_pubkey()?, + *chain.keyring.default_ed25519_pubkey()?, ))) }