From 8c06970bc61df353e1da60e9d81ef8da00dfe776 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Tue, 5 Nov 2024 10:39:20 -0500 Subject: [PATCH 01/42] feat: initial work into system keychain --- Cargo.lock | 111 ++++++++++++++++++++++++++ cmd/soroban-cli/Cargo.toml | 11 ++- cmd/soroban-cli/src/bin/secret.rs | 16 ++++ cmd/soroban-cli/src/signer.rs | 2 + cmd/soroban-cli/src/signer/keyring.rs | 100 +++++++++++++++++++++++ 5 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 cmd/soroban-cli/src/bin/secret.rs create mode 100644 cmd/soroban-cli/src/signer/keyring.rs diff --git a/Cargo.lock b/Cargo.lock index 3f8fcf4ac..dbf818db8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1179,6 +1179,30 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e8566979429cf69b49a5c740c60791108e86440e8be149bbea4fe54d2c32d6e2" +[[package]] +name = "dbus" +version = "0.9.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bb21987b9fb1613058ba3843121dd18b163b254d8a6e797e144cbac14d96d1b" +dependencies = [ + "libc", + "libdbus-sys", + "winapi", +] + +[[package]] +name = "dbus-secret-service" +version = "4.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b42a16374481d92aed73ae45b1f120207d8e71d24fb89f357fadbd8f946fd84b" +dependencies = [ + "dbus", + "futures-util", + "num", + "once_cell", + "rand", +] + [[package]] name = "der" version = "0.7.9" @@ -2581,6 +2605,18 @@ dependencies = [ "cpufeatures", ] +[[package]] +name = "keyring" +version = "3.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5fa83d1ca02db069b5fbe94b23b584d588e989218310c9c15015bb5571ef1a94" +dependencies = [ + "byteorder 1.5.0", + "dbus-secret-service", + "security-framework", + "windows-sys 0.59.0", +] + [[package]] name = "kv-log-macro" version = "1.0.7" @@ -2682,6 +2718,15 @@ version = "0.2.155" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97b3888a4aecf77e811145cadf6eef5901f4782c53886191b2f693f24761847c" +[[package]] +name = "libdbus-sys" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06085512b750d640299b79be4bad3d2fa90a9c00b1fd9e1b46364f66f0485c72" +dependencies = [ + "pkg-config", +] + [[package]] name = "libm" version = "0.2.8" @@ -2870,6 +2915,20 @@ dependencies = [ "winapi", ] +[[package]] +name = "num" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b05180d69e3da0e530ba2a1dae5110317e49e3b7f3d41be227dc5f92e49ee7af" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + [[package]] name = "num-bigint" version = "0.4.4" @@ -2881,6 +2940,15 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-complex" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23c6602fda94a57c990fe0df199a035d83576b496aa29f4e634a8ac6004e68a6" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.1.0" @@ -2908,6 +2976,29 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-iter" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d869c01cc0c455284163fd0092f1f93835385ccab5a98a0dcc497b2f8bf055a9" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-rational" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0638a1c9d0a3c0914158145bc76cff373a75a627e6ecbfb71cbe6f453a5a19b0" +dependencies = [ + "autocfg", + "num-bigint", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.17" @@ -4320,6 +4411,7 @@ dependencies = [ "itertools 0.10.5", "jsonrpsee-core", "jsonrpsee-http-client", + "keyring", "mockito", "num-bigint", "open", @@ -4370,6 +4462,8 @@ dependencies = [ "wasm-opt", "wasmparser", "which", + "whoami", + "zeroize", ] [[package]] @@ -5587,6 +5681,12 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "wasite" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8dad83b4f25e74f184f64c43b150b91efe7647395b42289f38e50566d82855b" + [[package]] name = "wasm-bindgen" version = "0.2.92" @@ -5795,6 +5895,17 @@ dependencies = [ "rustix 0.38.34", ] +[[package]] +name = "whoami" +version = "1.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "372d5b87f58ec45c384ba03563b03544dc5fadc3983e434b286913f5b4a9bb6d" +dependencies = [ + "redox_syscall", + "wasite", + "web-sys", +] + [[package]] name = "widestring" version = "1.1.0" diff --git a/cmd/soroban-cli/Cargo.toml b/cmd/soroban-cli/Cargo.toml index 2f2a26255..0d6da167d 100644 --- a/cmd/soroban-cli/Cargo.toml +++ b/cmd/soroban-cli/Cargo.toml @@ -20,6 +20,10 @@ path = "src/bin/stellar.rs" name = "soroban" path = "src/bin/soroban.rs" +[[bin]] +name = "secret" +path = "src/bin/secret.rs" + [package.metadata.binstall] pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ version }-{ target }{ archive-suffix }" bin-dir = "{ bin }{ binary-ext }" @@ -72,7 +76,8 @@ rand = "0.8.5" wasmparser = { workspace = true } sha2 = { workspace = true } csv = "1.1.6" -ed25519-dalek = { workspace = true } +# zeroize feature ensures that all sensitive data is zeroed out when dropped +ed25519-dalek = { workspace = true, features = ["zeroize"] } reqwest = { version = "0.12.7", default-features = false, features = [ "rustls-tls", "http2", @@ -124,6 +129,10 @@ fqdn = "0.3.12" open = "5.3.0" url = "2.5.2" wasm-gen = "0.1.4" +zeroize = "1.8.1" +keyring = { version = "3", features = ["apple-native", "windows-native", "sync-secret-service"] } +whoami = "1.5.2" + [build-dependencies] crate-git-revision = "0.0.6" diff --git a/cmd/soroban-cli/src/bin/secret.rs b/cmd/soroban-cli/src/bin/secret.rs new file mode 100644 index 000000000..26d1a51c7 --- /dev/null +++ b/cmd/soroban-cli/src/bin/secret.rs @@ -0,0 +1,16 @@ +use soroban_cli::signer::keyring::{add_key, get_public_key, StellarEntry}; + +fn main() { + let entry = StellarEntry::new("test").unwrap(); + if let Ok(key) = entry.get_public_key() { + println!("{key}") + }; + + let secret = soroban_cli::config::secret::Secret::from_seed(None).unwrap(); + let pub_key = secret.public_key(None).unwrap(); + let key_pair = secret.key_pair(None).unwrap(); + entry.add_password(key_pair.as_bytes()).unwrap(); + let pub_key_2 = entry.get_public_key().unwrap(); + assert_eq!(pub_key, pub_key_2); + println!("{pub_key} == {pub_key_2}"); +} diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index 5bf22499c..a141aaf6b 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -11,6 +11,8 @@ use crate::xdr::{ use crate::{config::network::Network, print::Print, utils::transaction_hash}; +pub mod keyring; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("Contract addresses are not supported to sign auth entries {address}")] diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs new file mode 100644 index 000000000..eca6d8657 --- /dev/null +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -0,0 +1,100 @@ +use base64::{engine::general_purpose::STANDARD as base64, Engine as _}; +use ed25519_dalek::Signer; +use keyring::Entry; +use zeroize::Zeroize; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error(transparent)] + Keyring(#[from] keyring::Error), + #[error(transparent)] + Base64(#[from] base64::DecodeError), +} + +pub struct StellarEntry { + name: String, +} + +impl TryFrom<&StellarEntry> for Entry { + type Error = Error; + fn try_from(StellarEntry { name }: &StellarEntry) -> Result { + Ok(Entry::new( + &format!("org.stellar.cli.{name}"), + &whoami::username(), + )?) + } +} + +impl StellarEntry { + pub fn new(name: &str) -> Result { + Ok(StellarEntry { + name: name.to_string(), + }) + } + + pub fn set_password(&self, password: &[u8]) -> Result<(), Error> { + let data = base64.encode(password); + let entry: Entry = self.try_into()?; + entry.set_password(&data)?; + Ok(()) + } + + pub fn get_password(&self) -> Result, Error> { + let entry: Entry = self.try_into()?; + Ok(base64.decode(entry.get_password()?)?) + } + + pub fn get_public_key(&self) -> Result { + let mut key_vec = self.get_password()?; + let mut key_bytes: [u8; 32] = key_vec.as_slice().try_into().unwrap(); + + let pub_key = { + // Use this scope to ensure the keypair is zeroized + let keypair = ed25519_dalek::SigningKey::from_bytes(&key_bytes); + stellar_strkey::ed25519::PublicKey(*keypair.verifying_key().as_bytes()) + }; + key_vec.zeroize(); + key_bytes.zeroize(); + Ok(pub_key) + } +} + +pub fn sign_data(name: &str, data: &[u8]) -> Result, Box> { + // Retrieve the key from the secure storage + let entry = Entry::new("stellar", name)?; + let key_bytes: [u8; 32] = entry.get_secret()?.try_into().unwrap(); + // Create a keypair from the retrieved bytes + let keypair = ed25519_dalek::SigningKey::from_bytes(&key_bytes); + + // Sign the data + let signature = keypair.sign(data); + + // Clear the key from memory + let mut key_bytes = key_bytes; + key_bytes.zeroize(); + + Ok(signature.to_bytes().to_vec()) +} + +pub fn add_key(name: &str, key_bytes: &[u8]) -> Result<(), Box> { + // Create a new keyring entry for "stellar" + StellarEntry::new(name)?.set_password(key_bytes)?; + Ok(()) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_sign_data() -> Result<(), Box> { + let secret = crate::config::secret::Secret::from_seed(None)?; + let pub_key = secret.public_key(None)?; + let key_pair = secret.key_pair(None)?; + + add_key("test", &key_pair.to_bytes()).unwrap(); + let pub_key_2 = get_public_key("test")?; + assert_eq!(pub_key, pub_key_2); + Ok(()) + } +} From 883cb8cf3bb7e53817a35dc70749524ac1588137 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Tue, 5 Nov 2024 15:56:04 -0500 Subject: [PATCH 02/42] chore: clean up --- cmd/soroban-cli/src/bin/secret.rs | 7 ++-- cmd/soroban-cli/src/signer/keyring.rs | 51 ++++++++++++--------------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/cmd/soroban-cli/src/bin/secret.rs b/cmd/soroban-cli/src/bin/secret.rs index 26d1a51c7..4fc43ec6e 100644 --- a/cmd/soroban-cli/src/bin/secret.rs +++ b/cmd/soroban-cli/src/bin/secret.rs @@ -1,15 +1,16 @@ -use soroban_cli::signer::keyring::{add_key, get_public_key, StellarEntry}; +use soroban_cli::signer::keyring::StellarEntry; fn main() { let entry = StellarEntry::new("test").unwrap(); if let Ok(key) = entry.get_public_key() { - println!("{key}") + println!("{key}"); + return; }; let secret = soroban_cli::config::secret::Secret::from_seed(None).unwrap(); let pub_key = secret.public_key(None).unwrap(); let key_pair = secret.key_pair(None).unwrap(); - entry.add_password(key_pair.as_bytes()).unwrap(); + entry.set_password(key_pair.as_bytes()).unwrap(); let pub_key_2 = entry.get_public_key().unwrap(); assert_eq!(pub_key, pub_key_2); println!("{pub_key} == {pub_key_2}"); diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index eca6d8657..bde7e97d6 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -44,42 +44,37 @@ impl StellarEntry { Ok(base64.decode(entry.get_password()?)?) } - pub fn get_public_key(&self) -> Result { + fn use_key( + &self, + f: impl FnOnce(ed25519_dalek::SigningKey) -> Result, + ) -> Result { let mut key_vec = self.get_password()?; let mut key_bytes: [u8; 32] = key_vec.as_slice().try_into().unwrap(); - let pub_key = { + let result = { // Use this scope to ensure the keypair is zeroized let keypair = ed25519_dalek::SigningKey::from_bytes(&key_bytes); - stellar_strkey::ed25519::PublicKey(*keypair.verifying_key().as_bytes()) + f(keypair)? }; key_vec.zeroize(); key_bytes.zeroize(); - Ok(pub_key) + Ok(result) } -} - -pub fn sign_data(name: &str, data: &[u8]) -> Result, Box> { - // Retrieve the key from the secure storage - let entry = Entry::new("stellar", name)?; - let key_bytes: [u8; 32] = entry.get_secret()?.try_into().unwrap(); - // Create a keypair from the retrieved bytes - let keypair = ed25519_dalek::SigningKey::from_bytes(&key_bytes); - // Sign the data - let signature = keypair.sign(data); - - // Clear the key from memory - let mut key_bytes = key_bytes; - key_bytes.zeroize(); - - Ok(signature.to_bytes().to_vec()) -} + pub fn get_public_key(&self) -> Result { + self.use_key(|keypair| { + Ok(stellar_strkey::ed25519::PublicKey( + *keypair.verifying_key().as_bytes(), + )) + }) + } -pub fn add_key(name: &str, key_bytes: &[u8]) -> Result<(), Box> { - // Create a new keyring entry for "stellar" - StellarEntry::new(name)?.set_password(key_bytes)?; - Ok(()) + pub fn sign_data(&self, data: &[u8]) -> Result, Error> { + self.use_key(|keypair| { + let signature = keypair.sign(data); + Ok(signature.to_bytes().to_vec()) + }) + } } #[cfg(test)] @@ -91,9 +86,9 @@ mod test { let secret = crate::config::secret::Secret::from_seed(None)?; let pub_key = secret.public_key(None)?; let key_pair = secret.key_pair(None)?; - - add_key("test", &key_pair.to_bytes()).unwrap(); - let pub_key_2 = get_public_key("test")?; + let entry = StellarEntry::new("test")?; + entry.set_password(&key_pair.to_bytes()); + let pub_key_2 = entry.get_public_key()?; assert_eq!(pub_key, pub_key_2); Ok(()) } From eb7073402548e24101ea2f2fe4d37664b1264d79 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:23:11 -0500 Subject: [PATCH 03/42] Add KeyName struct in address --- cmd/soroban-cli/src/commands/keys/add.rs | 10 +++++-- cmd/soroban-cli/src/commands/keys/generate.rs | 4 +-- cmd/soroban-cli/src/config/address.rs | 29 +++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/add.rs b/cmd/soroban-cli/src/commands/keys/add.rs index d8f528bae..fd796bb86 100644 --- a/cmd/soroban-cli/src/commands/keys/add.rs +++ b/cmd/soroban-cli/src/commands/keys/add.rs @@ -1,21 +1,25 @@ use clap::command; -use crate::config::{locator, secret}; +use crate::config::{ + address::{self, KeyName}, + locator, secret, +}; #[derive(thiserror::Error, Debug)] pub enum Error { #[error(transparent)] Secret(#[from] secret::Error), - #[error(transparent)] Config(#[from] locator::Error), + #[error(transparent)] + Address(#[from] address::Error), } #[derive(Debug, clap::Parser, Clone)] #[group(skip)] pub struct Cmd { /// Name of identity - pub name: String, + pub name: KeyName, #[command(flatten)] pub secrets: secret::Args, diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index c6623386c..315fbb7ec 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -69,10 +69,10 @@ impl Cmd { if self.config_locator.read_identity(&self.name).is_ok() { if !self.overwrite { - return Err(Error::IdentityAlreadyExists(self.name.clone())); + return Err(Error::IdentityAlreadyExists(self.name.to_string())); } - print.exclaimln(format!("Overwriting identity '{}'", &self.name)); + print.exclaimln(format!("Overwriting identity '{}'", &self.name.to_string())); } if !self.fund { diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 066bc8d91..57c229c76 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -25,6 +25,8 @@ pub enum Error { Secret(#[from] secret::Error), #[error("Address cannot be used to sign {0}")] CannotSign(xdr::MuxedAccount), + #[error("Invalid key name: {0}\n only alphanumeric characters, `_`and `-` are allowed")] + InvalidKeyName(String), } impl FromStr for Address { @@ -61,3 +63,30 @@ impl Address { } } } + +#[derive(Clone, Debug)] +pub struct KeyName(pub String); + +impl std::ops::Deref for KeyName { + type Target = str; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::str::FromStr for KeyName { + type Err = Error; + fn from_str(s: &str) -> Result { + if !s.chars().all(allowed_char) { + return Err(Error::InvalidKeyName(s.to_string())); + } + if s == "ledger" { + return Err(Error::InvalidKeyName(s.to_string())); + } + Ok(KeyName(s.to_string())) + } +} + +fn allowed_char(c: char) -> bool { + c.is_ascii_alphanumeric() || c == '_' || c == '-' +} From d4e25003db21f05fef829d600e98ff25515342bc Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:25:03 -0500 Subject: [PATCH 04/42] Add Secret::Keychain --- cmd/soroban-cli/src/config/secret.rs | 97 +++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index a7fd86fda..d07e644ac 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -34,11 +34,15 @@ pub enum Error { pub struct Args { /// Add using `secret_key` /// Can provide with `SOROBAN_SECRET_KEY` - #[arg(long, conflicts_with = "seed_phrase")] + #[arg(long, conflicts_with_all = ["seed_phrase", "keychain"])] pub secret_key: bool, /// Add using 12 word seed phrase to generate `secret_key` - #[arg(long, conflicts_with = "secret_key")] + #[arg(long, conflicts_with_all = ["secret_key", "keychain"])] pub seed_phrase: bool, + + /// Add using `keychain` + #[arg(long, conflicts_with_all = ["seed_phrase", "secret_key"])] + pub keychain: bool, } impl Args { @@ -67,6 +71,15 @@ impl Args { .collect::>() .join(" "), }) + } else if self.keychain { + // generate a secret, and save it in the keychain + // return a new type of secret? + // for now, put it all in here + println!("generate a secret in the keychain"); + // let keychain = keyring::Keyring::new(" + Ok(Secret::SecretKey { + secret_key: "test".to_owned(), + }) } else { Err(Error::PasswordRead {}) } @@ -78,6 +91,7 @@ impl Args { pub enum Secret { SecretKey { secret_key: String }, SeedPhrase { seed_phrase: String }, + Keychain, } impl FromStr for Secret { @@ -92,6 +106,8 @@ impl FromStr for Secret { Ok(Secret::SeedPhrase { seed_phrase: s.to_string(), }) + } else if s == "keychain" { + Ok(Secret::Keychain) } else { Err(Error::InvalidAddress(s.to_string())) } @@ -116,6 +132,7 @@ impl Secret { .private() .0, )?, + Secret::Keychain => panic!("Keychain does not reveal secret key"), }) } @@ -132,6 +149,7 @@ impl Secret { let key = self.key_pair(index)?; SignerKind::Local(LocalKey { key }) } + Secret::Keychain => todo!(), }; Ok(Signer { kind, print }) } @@ -160,3 +178,78 @@ fn read_password() -> Result { std::io::stdout().flush().map_err(|_| Error::PasswordRead)?; rpassword::read_password().map_err(|_| Error::PasswordRead) } + +#[cfg(test)] +mod tests { + use super::*; + + const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH"; + const TEST_SEED_PHRASE: &str = + "depth decade power loud smile spatial sign movie judge february rate broccoli"; + + #[test] + fn test_secret_from_key() { + let secret = Secret::from_str(TEST_SECRET_KEY).unwrap(); + // assert that it is a Secret::SecretKey + match secret { + Secret::SecretKey { secret_key: _ } => assert!(true), + _ => assert!(false), + } + // assert that we can get the private key from it + let private_key = secret.private_key(None).unwrap(); + assert_eq!(private_key.to_string(), TEST_SECRET_KEY); + + let signer = secret.signer(None, Print::new(false)).unwrap(); + println!("signer: {:?}", signer.kind); + } + + #[test] + fn test_secret_from_seed_phrase() { + let secret = Secret::from_str(TEST_SEED_PHRASE).unwrap(); + match secret { + Secret::SeedPhrase { seed_phrase: _ } => assert!(true), + _ => assert!(false), + } + + let private_key = secret.private_key(None).unwrap(); + assert_eq!(private_key.to_string(), TEST_SECRET_KEY); + } + + #[test] + fn test_ledger_secret() { + let secret = Secret::from_str("ledger").unwrap(); + match secret { + Secret::Ledger => assert!(true), + _ => assert!(false), + } + } + + #[test] + #[should_panic] + fn test_ledger_secret_will_not_reveal_private_key() { + let secret = Secret::from_str("ledger").unwrap(); + secret.private_key(None).unwrap(); + } + + #[test] + fn test_keychain_secret() { + let keychain_secret = Secret::from_str("keychain").unwrap(); + match keychain_secret { + Secret::Keychain => assert!(true), + _ => assert!(false), + } + } + + #[test] + #[should_panic] + fn test_keychain_secret_will_not_reveal_private_key() { + let secret = Secret::from_str("keychain").unwrap(); + secret.private_key(None).unwrap(); + } + + #[test] + fn test_secret_from_invalid_string() { + let secret = Secret::from_str("invalid"); + assert!(secret.is_err()); + } +} From 3ee20b8d634e6749cc27cc30ca59ca950cddae80 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:17:36 -0500 Subject: [PATCH 05/42] keys generate: allow for generating keys that are stored in keychain --- cmd/soroban-cli/src/commands/keys/generate.rs | 108 +++++++++++++++--- cmd/soroban-cli/src/config/secret.rs | 55 +++------ 2 files changed, 111 insertions(+), 52 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 315fbb7ec..e04c22a1e 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -39,6 +39,10 @@ pub struct Cmd { #[arg(long, short = 's')] pub as_secret: bool, + /// Save in `keychain` + #[arg(long)] + pub keychain: bool, + #[command(flatten)] pub config_locator: locator::Args, @@ -63,6 +67,8 @@ pub struct Cmd { pub overwrite: bool, } +const KEYCHAIN_CONSTANT: &str = "keychain"; + impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { let print = Print::new(global_args.quiet); @@ -83,19 +89,7 @@ impl Cmd { warning. It can be suppressed with -q flag.", ); } - - let seed_phrase = if self.default_seed { - Secret::test_seed_phrase() - } else { - Secret::from_seed(self.seed.as_deref()) - }?; - - let secret = if self.as_secret { - seed_phrase.private_key(self.hd_path)?.into() - } else { - seed_phrase - }; - + let secret = self.secret()?; self.config_locator.write_identity(&self.name, &secret)?; if !self.no_fund { @@ -112,4 +106,92 @@ impl Cmd { Ok(()) } + + fn secret(&self) -> Result { + let seed_phrase = self.seed_phrase()?; + Ok(if self.as_secret { + seed_phrase.private_key(self.hd_path)?.into() + } else if self.keychain { + KEYCHAIN_CONSTANT.parse()? + } else { + seed_phrase + }) + } + + fn seed_phrase(&self) -> Result { + Ok(if self.default_seed { + Secret::test_seed_phrase() + } else { + Secret::from_seed(self.seed.as_deref()) + }?) + } +} + +#[cfg(test)] +mod tests { + use crate::config::secret::Secret; + + fn set_up_test() -> (super::locator::Args, super::Cmd) { + let temp_dir = tempfile::tempdir().unwrap(); + let locator = super::locator::Args { + global: false, + config_dir: Some(temp_dir.path().to_path_buf()), + }; + + let cmd = super::Cmd { + name: "test_name".to_string(), + no_fund: true, + seed: None, + as_secret: false, + keychain: false, + config_locator: locator.clone(), + hd_path: None, + default_seed: false, + network: Default::default(), + fund: false, + }; + + (locator, cmd) + } + + fn global_args() -> super::global::Args { + let mut global_args = super::global::Args::default(); + global_args.quiet = true; + global_args + } + + #[tokio::test] + async fn test_storing_secret_as_a_seed_phrase() { + let (test_locator, cmd) = set_up_test(); + let global_args = global_args(); + + let result = cmd.run(&global_args).await; + assert!(result.is_ok()); + let identity = test_locator.read_identity("test_name").unwrap(); + assert!(matches!(identity, Secret::SeedPhrase { .. })); + } + + #[tokio::test] + async fn test_storing_secret_as_a_secret_key() { + let (test_locator, mut cmd) = set_up_test(); + cmd.as_secret = true; + let global_args = global_args(); + + let result = cmd.run(&global_args).await; + assert!(result.is_ok()); + let identity = test_locator.read_identity("test_name").unwrap(); + assert!(matches!(identity, Secret::SecretKey { .. })); + } + + #[tokio::test] + async fn test_storing_secret_in_keychain() { + let (test_locator, mut cmd) = set_up_test(); + cmd.keychain = true; + let global_args = global_args(); + + let result = cmd.run(&global_args).await; + assert!(result.is_ok()); + let identity = test_locator.read_identity("test_name").unwrap(); + assert!(matches!(identity, Secret::Keychain { .. })); + } } diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index d07e644ac..c88eaa88c 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -9,6 +9,8 @@ use crate::{ utils, }; +const KEYCHAIN_ENTRY_NAME: &str = "org.stellar.cli"; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error("invalid secret key")] @@ -72,13 +74,8 @@ impl Args { .join(" "), }) } else if self.keychain { - // generate a secret, and save it in the keychain - // return a new type of secret? - // for now, put it all in here - println!("generate a secret in the keychain"); - // let keychain = keyring::Keyring::new(" - Ok(Secret::SecretKey { - secret_key: "test".to_owned(), + Ok(Secret::Keychain { + entry_name: KEYCHAIN_ENTRY_NAME.to_owned(), }) } else { Err(Error::PasswordRead {}) @@ -91,7 +88,7 @@ impl Args { pub enum Secret { SecretKey { secret_key: String }, SeedPhrase { seed_phrase: String }, - Keychain, + Keychain { entry_name: String }, } impl FromStr for Secret { @@ -107,7 +104,9 @@ impl FromStr for Secret { seed_phrase: s.to_string(), }) } else if s == "keychain" { - Ok(Secret::Keychain) + Ok(Secret::Keychain { + entry_name: KEYCHAIN_ENTRY_NAME.to_owned(), //TODO: namespace the entry_name to the system user or key name? + }) } else { Err(Error::InvalidAddress(s.to_string())) } @@ -132,7 +131,7 @@ impl Secret { .private() .0, )?, - Secret::Keychain => panic!("Keychain does not reveal secret key"), + Secret::Keychain { entry_name: _ } => panic!("Keychain does not reveal secret key"), }) } @@ -149,7 +148,7 @@ impl Secret { let key = self.key_pair(index)?; SignerKind::Local(LocalKey { key }) } - Secret::Keychain => todo!(), + Secret::Keychain { .. } => todo!(), }; Ok(Signer { kind, print }) } @@ -191,51 +190,29 @@ mod tests { fn test_secret_from_key() { let secret = Secret::from_str(TEST_SECRET_KEY).unwrap(); // assert that it is a Secret::SecretKey - match secret { - Secret::SecretKey { secret_key: _ } => assert!(true), - _ => assert!(false), - } + assert!(matches!(secret, Secret::SecretKey { .. })); // assert that we can get the private key from it let private_key = secret.private_key(None).unwrap(); assert_eq!(private_key.to_string(), TEST_SECRET_KEY); - - let signer = secret.signer(None, Print::new(false)).unwrap(); - println!("signer: {:?}", signer.kind); } #[test] fn test_secret_from_seed_phrase() { let secret = Secret::from_str(TEST_SEED_PHRASE).unwrap(); - match secret { - Secret::SeedPhrase { seed_phrase: _ } => assert!(true), - _ => assert!(false), - } + assert!(matches!(secret, Secret::SeedPhrase { .. })); let private_key = secret.private_key(None).unwrap(); assert_eq!(private_key.to_string(), TEST_SECRET_KEY); } - #[test] - fn test_ledger_secret() { - let secret = Secret::from_str("ledger").unwrap(); - match secret { - Secret::Ledger => assert!(true), - _ => assert!(false), - } - } - - #[test] - #[should_panic] - fn test_ledger_secret_will_not_reveal_private_key() { - let secret = Secret::from_str("ledger").unwrap(); - secret.private_key(None).unwrap(); - } - #[test] fn test_keychain_secret() { let keychain_secret = Secret::from_str("keychain").unwrap(); + match keychain_secret { - Secret::Keychain => assert!(true), + Secret::Keychain { entry_name } => { + assert_eq!(entry_name, KEYCHAIN_ENTRY_NAME); + } _ => assert!(false), } } From 1a24c000314ece3306a449060f4dc99e8fb1e15a Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 18 Nov 2024 17:16:20 -0500 Subject: [PATCH 06/42] keys generate: Namespace keychain entry to identity name --- cmd/soroban-cli/src/commands/keys/generate.rs | 40 +++++++++++++++++-- cmd/soroban-cli/src/config/secret.rs | 23 ++++++----- cmd/soroban-cli/src/signer/keyring.rs | 8 ++-- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index e04c22a1e..b7ebdba3d 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -4,7 +4,11 @@ use super::super::config::{ locator, network, secret::{self, Secret}, }; -use crate::{commands::global, print::Print}; +use crate::{ + commands::global, + print::Print, + signer::keyring::{self, StellarEntry}, +}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -19,6 +23,9 @@ pub enum Error { #[error("An identity with the name '{0}' already exists")] IdentityAlreadyExists(String), + + #[error(transparent)] + Keyring(#[from] keyring::Error), } #[derive(Debug, clap::Parser, Clone)] @@ -67,8 +74,6 @@ pub struct Cmd { pub overwrite: bool, } -const KEYCHAIN_CONSTANT: &str = "keychain"; - impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { let print = Print::new(global_args.quiet); @@ -112,7 +117,21 @@ impl Cmd { Ok(if self.as_secret { seed_phrase.private_key(self.hd_path)?.into() } else if self.keychain { - KEYCHAIN_CONSTANT.parse()? + // keychain:org.stellar.cli: + let entry_name_with_prefix = format!( + "{}{}-{}", + keyring::KEYCHAIN_ENTRY_PREFIX, + keyring::KEYCHAIN_ENTRY_SERVICE, + self.name + ); + + let secret: Secret = entry_name_with_prefix.parse()?; //checking that the entry name is valid before writing to the keychain + + if let Secret::Keychain { entry_name } = &secret { + self.write_to_keychain(entry_name.clone(), seed_phrase)?; + } + + secret } else { seed_phrase }) @@ -125,6 +144,19 @@ impl Cmd { Secret::from_seed(self.seed.as_deref()) }?) } + + fn write_to_keychain(&self, entry_name: String, seed_phrase: Secret) -> Result<(), Error> { + println!("Writing to keychain: {entry_name}"); + let entry = StellarEntry::new(&entry_name)?; + if let Ok(key) = entry.get_public_key() { + println!("A key for {entry_name} already exists in your keychain: {key}"); + } else { + println!("Saving a new key to your keychain: {entry_name}"); + let key_pair = seed_phrase.key_pair(None)?; + entry.set_password(key_pair.as_bytes())?; + } + Ok(()) + } } #[cfg(test)] diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index c88eaa88c..e6d0ce8ec 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -5,12 +5,10 @@ use stellar_strkey::ed25519::{PrivateKey, PublicKey}; use crate::{ print::Print, - signer::{self, LocalKey, Signer, SignerKind}, + signer::{self, keyring, LocalKey, Signer, SignerKind}, utils, }; -const KEYCHAIN_ENTRY_NAME: &str = "org.stellar.cli"; - #[derive(thiserror::Error, Debug)] pub enum Error { #[error("invalid secret key")] @@ -74,9 +72,10 @@ impl Args { .join(" "), }) } else if self.keychain { - Ok(Secret::Keychain { - entry_name: KEYCHAIN_ENTRY_NAME.to_owned(), - }) + todo!(); + // Ok(Secret::Keychain { + // entry_name: KEYCHAIN_ENTRY_NAME.to_owned(), + // }) } else { Err(Error::PasswordRead {}) } @@ -103,9 +102,13 @@ impl FromStr for Secret { Ok(Secret::SeedPhrase { seed_phrase: s.to_string(), }) - } else if s == "keychain" { + } else if s.starts_with(keyring::KEYCHAIN_ENTRY_PREFIX) { + let entry_name = s + .strip_prefix(keyring::KEYCHAIN_ENTRY_PREFIX) + .ok_or(Error::InvalidAddress(s.to_string()))?; + Ok(Secret::Keychain { - entry_name: KEYCHAIN_ENTRY_NAME.to_owned(), //TODO: namespace the entry_name to the system user or key name? + entry_name: entry_name.to_owned(), }) } else { Err(Error::InvalidAddress(s.to_string())) @@ -207,11 +210,11 @@ mod tests { #[test] fn test_keychain_secret() { - let keychain_secret = Secret::from_str("keychain").unwrap(); + let keychain_secret = Secret::from_str("keychain:org.stellar.cli-alice").unwrap(); match keychain_secret { Secret::Keychain { entry_name } => { - assert_eq!(entry_name, KEYCHAIN_ENTRY_NAME); + assert_eq!(entry_name, "org.stellar.cli-alice"); } _ => assert!(false), } diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index bde7e97d6..35d72e9b4 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -3,6 +3,9 @@ use ed25519_dalek::Signer; use keyring::Entry; use zeroize::Zeroize; +pub(crate) const KEYCHAIN_ENTRY_PREFIX: &str = "keychain:"; //TODO: does this belong here, or in secret? +pub(crate) const KEYCHAIN_ENTRY_SERVICE: &str = "org.stellar.cli"; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error(transparent)] @@ -18,10 +21,7 @@ pub struct StellarEntry { impl TryFrom<&StellarEntry> for Entry { type Error = Error; fn try_from(StellarEntry { name }: &StellarEntry) -> Result { - Ok(Entry::new( - &format!("org.stellar.cli.{name}"), - &whoami::username(), - )?) + Ok(Entry::new(name, &whoami::username())?) } } From 4a0c900aa600792c6dc9fad9ec6006fa039d749f Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:02:01 -0500 Subject: [PATCH 07/42] keys generate: don't allow 'keychain:' as a key name --- cmd/soroban-cli/src/commands/keys/generate.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index b7ebdba3d..ce8646123 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -6,6 +6,7 @@ use super::super::config::{ }; use crate::{ commands::global, + config::address::KeyName, print::Print, signer::keyring::{self, StellarEntry}, }; @@ -33,7 +34,7 @@ pub enum Error { #[allow(clippy::struct_excessive_bools)] pub struct Cmd { /// Name of identity - pub name: String, + pub name: KeyName, /// Do not fund address #[arg(long)] pub no_fund: bool, @@ -122,7 +123,7 @@ impl Cmd { "{}{}-{}", keyring::KEYCHAIN_ENTRY_PREFIX, keyring::KEYCHAIN_ENTRY_SERVICE, - self.name + self.name.to_string() ); let secret: Secret = entry_name_with_prefix.parse()?; //checking that the entry name is valid before writing to the keychain From 755c3186997a588a89874cda1e11ca683611fe0c Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Wed, 20 Nov 2024 16:41:02 -0500 Subject: [PATCH 08/42] keys address: use keychain entry in secret to get the pub key --- cmd/soroban-cli/src/config/secret.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index e6d0ce8ec..6364bc2c1 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -27,6 +27,8 @@ pub enum Error { InvalidAddress(String), #[error(transparent)] Signer(#[from] signer::Error), + #[error(transparent)] + Keyring(#[from] keyring::Error), } #[derive(Debug, clap::Args, Clone)] @@ -103,12 +105,8 @@ impl FromStr for Secret { seed_phrase: s.to_string(), }) } else if s.starts_with(keyring::KEYCHAIN_ENTRY_PREFIX) { - let entry_name = s - .strip_prefix(keyring::KEYCHAIN_ENTRY_PREFIX) - .ok_or(Error::InvalidAddress(s.to_string()))?; - Ok(Secret::Keychain { - entry_name: entry_name.to_owned(), + entry_name: s.to_owned(), }) } else { Err(Error::InvalidAddress(s.to_string())) @@ -139,10 +137,18 @@ impl Secret { } pub fn public_key(&self, index: Option) -> Result { - let key = self.key_pair(index)?; - Ok(stellar_strkey::ed25519::PublicKey::from_payload( - key.verifying_key().as_bytes(), - )?) + match self { + Secret::Keychain { entry_name } => { + let entry = keyring::StellarEntry::new(entry_name)?; + Ok(entry.get_public_key()?) + } + _ => { + let key = self.key_pair(index)?; + Ok(stellar_strkey::ed25519::PublicKey::from_payload( + key.verifying_key().as_bytes(), + )?) + } + } } pub fn signer(&self, index: Option, print: Print) -> Result { From dd07bf70d6d8c362a2b2e3922265e9252b04d023 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:12:50 -0500 Subject: [PATCH 09/42] tx sign: allow a keychain identity sign a tx --- cmd/soroban-cli/src/config/secret.rs | 8 +++++--- cmd/soroban-cli/src/signer.rs | 21 ++++++++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index 6364bc2c1..d1e15bedc 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -5,7 +5,7 @@ use stellar_strkey::ed25519::{PrivateKey, PublicKey}; use crate::{ print::Print, - signer::{self, keyring, LocalKey, Signer, SignerKind}, + signer::{self, keyring, KeychainEntry, LocalKey, Signer, SignerKind}, utils, }; @@ -106,7 +106,7 @@ impl FromStr for Secret { }) } else if s.starts_with(keyring::KEYCHAIN_ENTRY_PREFIX) { Ok(Secret::Keychain { - entry_name: s.to_owned(), + entry_name: s.to_string(), }) } else { Err(Error::InvalidAddress(s.to_string())) @@ -157,7 +157,9 @@ impl Secret { let key = self.key_pair(index)?; SignerKind::Local(LocalKey { key }) } - Secret::Keychain { .. } => todo!(), + Secret::Keychain { entry_name } => SignerKind::Keychain(KeychainEntry { + name: entry_name.to_string(), + }), }; Ok(Signer { kind, print }) } diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index a141aaf6b..e0a1568c2 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -1,4 +1,5 @@ -use ed25519_dalek::ed25519::signature::Signer as _; +use ed25519_dalek::ed25519::signature::{self, Signer as _}; +use keyring::StellarEntry; use sha2::{Digest, Sha256}; use crate::xdr::{ @@ -35,6 +36,8 @@ pub enum Error { Open(#[from] std::io::Error), #[error("Returning a signature from Lab is not yet supported; Transaction can be found and submitted in lab")] ReturningSignatureFromLab, + #[error(transparent)] + Keyring(#[from] keyring::Error), } fn requires_auth(txn: &Transaction) -> Option { @@ -209,6 +212,7 @@ pub struct Signer { pub enum SignerKind { Local(LocalKey), Lab, + Keychain(KeychainEntry), } impl Signer { @@ -237,6 +241,7 @@ impl Signer { let decorated_signature = match &self.kind { SignerKind::Local(key) => key.sign_tx_hash(tx_hash)?, SignerKind::Lab => Lab::sign_tx_env(tx_env, network, &self.print)?, + SignerKind::Keychain(entry) => entry.sign_tx_env(tx_env)?, }; let mut sigs = signatures.clone().into_vec(); sigs.push(decorated_signature); @@ -286,3 +291,17 @@ impl Lab { Err(Error::ReturningSignatureFromLab) } } + +pub struct KeychainEntry { + pub name: String, +} + +impl KeychainEntry { + pub fn sign_tx_env(&self, tx_env: &TransactionEnvelope) -> Result { + let entry = StellarEntry::new(&self.name)?; + let signed_tx_env = entry.sign_data(tx_env.to_xdr_base64(Limits::none())?.as_bytes())?; + let hint = SignatureHint(entry.get_public_key()?.0[28..].try_into()?); + let signature = Signature(signed_tx_env.to_vec().try_into()?); + Ok(DecoratedSignature { hint, signature }) + } +} From f042d19788fd6b725d88f762ea5be0fb4577f417 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:28:50 -0500 Subject: [PATCH 10/42] Cleanup --- cmd/soroban-cli/src/commands/keys/generate.rs | 5 +++-- cmd/soroban-cli/src/config/secret.rs | 9 ++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index ce8646123..8ecaa0f4e 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -162,7 +162,7 @@ impl Cmd { #[cfg(test)] mod tests { - use crate::config::secret::Secret; + use crate::config::{self, address::KeyName, secret::Secret}; fn set_up_test() -> (super::locator::Args, super::Cmd) { let temp_dir = tempfile::tempdir().unwrap(); @@ -172,7 +172,7 @@ mod tests { }; let cmd = super::Cmd { - name: "test_name".to_string(), + name: KeyName("test_name".to_string()), no_fund: true, seed: None, as_secret: false, @@ -182,6 +182,7 @@ mod tests { default_seed: false, network: Default::default(), fund: false, + overwrite: false, }; (locator, cmd) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index d1e15bedc..ebc2b8d3f 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -73,11 +73,6 @@ impl Args { .collect::>() .join(" "), }) - } else if self.keychain { - todo!(); - // Ok(Secret::Keychain { - // entry_name: KEYCHAIN_ENTRY_NAME.to_owned(), - // }) } else { Err(Error::PasswordRead {}) } @@ -132,7 +127,7 @@ impl Secret { .private() .0, )?, - Secret::Keychain { entry_name: _ } => panic!("Keychain does not reveal secret key"), + Secret::Keychain { .. } => panic!("Keychain does not reveal secret key"), }) } @@ -222,7 +217,7 @@ mod tests { match keychain_secret { Secret::Keychain { entry_name } => { - assert_eq!(entry_name, "org.stellar.cli-alice"); + assert_eq!(entry_name, "keychain:org.stellar.cli-alice"); } _ => assert!(false), } From 9cf591606ac7ddde6676634d8b149717739f85ca Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Fri, 22 Nov 2024 12:22:21 -0500 Subject: [PATCH 11/42] Use keyring mock for generate tests --- cmd/soroban-cli/src/commands/keys/generate.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 8ecaa0f4e..3bf2e48be 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -163,6 +163,7 @@ impl Cmd { #[cfg(test)] mod tests { use crate::config::{self, address::KeyName, secret::Secret}; + use keyring::{mock, set_default_credential_builder}; fn set_up_test() -> (super::locator::Args, super::Cmd) { let temp_dir = tempfile::tempdir().unwrap(); @@ -185,6 +186,8 @@ mod tests { overwrite: false, }; + set_default_credential_builder(mock::default_credential_builder()); + (locator, cmd) } From cd515d3d13ed0112ee1f41604b5fad020b5d5ae3 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:24:12 -0500 Subject: [PATCH 12/42] Refactor keyring: add keyring entry as StellarEntry field - previously we were creating a new keyring entry for each interaction with the keyring - this change will allow us use a mock keyring entry for testing --- cmd/soroban-cli/src/signer/keyring.rs | 47 ++++++++++++++------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index 35d72e9b4..5165dec43 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -15,33 +15,24 @@ pub enum Error { } pub struct StellarEntry { - name: String, -} - -impl TryFrom<&StellarEntry> for Entry { - type Error = Error; - fn try_from(StellarEntry { name }: &StellarEntry) -> Result { - Ok(Entry::new(name, &whoami::username())?) - } + keyring: Entry, } impl StellarEntry { pub fn new(name: &str) -> Result { Ok(StellarEntry { - name: name.to_string(), + keyring: Entry::new(name, &whoami::username())?, }) } pub fn set_password(&self, password: &[u8]) -> Result<(), Error> { let data = base64.encode(password); - let entry: Entry = self.try_into()?; - entry.set_password(&data)?; + self.keyring.set_password(&data)?; Ok(()) } pub fn get_password(&self) -> Result, Error> { - let entry: Entry = self.try_into()?; - Ok(base64.decode(entry.get_password()?)?) + Ok(base64.decode(self.keyring.get_password()?)?) } fn use_key( @@ -80,16 +71,28 @@ impl StellarEntry { #[cfg(test)] mod test { use super::*; + use keyring::{ + mock, + set_default_credential_builder, + }; #[test] - fn test_sign_data() -> Result<(), Box> { - let secret = crate::config::secret::Secret::from_seed(None)?; - let pub_key = secret.public_key(None)?; - let key_pair = secret.key_pair(None)?; - let entry = StellarEntry::new("test")?; - entry.set_password(&key_pair.to_bytes()); - let pub_key_2 = entry.get_public_key()?; - assert_eq!(pub_key, pub_key_2); - Ok(()) + fn test_sign_data() { + set_default_credential_builder(mock::default_credential_builder()); + + let secret = crate::config::secret::Secret::from_seed(None).unwrap(); + let pub_key = secret.public_key(None).unwrap(); + let key_pair = secret.key_pair(None).unwrap(); + + let entry = StellarEntry::new("test").unwrap(); + + // set the password + let set_password_result = entry.set_password(&key_pair.to_bytes()); + assert!(set_password_result.is_ok()); + + // confirm that we can get the public key from the entry and that it matches the one we set + let get_public_key_result = entry.get_public_key(); + assert!(get_public_key_result.is_ok()); + assert_eq!(pub_key, get_public_key_result.unwrap()); } } From 276558db228486205b29ded009f193b6d56c513e Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 12:20:22 -0500 Subject: [PATCH 13/42] Add tests for keyring --- cmd/soroban-cli/src/signer/keyring.rs | 48 +++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index 5165dec43..6c32f620d 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -71,17 +71,33 @@ impl StellarEntry { #[cfg(test)] mod test { use super::*; - use keyring::{ - mock, - set_default_credential_builder, - }; + use keyring::{mock, set_default_credential_builder}; #[test] - fn test_sign_data() { + fn test_get_password() { set_default_credential_builder(mock::default_credential_builder()); let secret = crate::config::secret::Secret::from_seed(None).unwrap(); - let pub_key = secret.public_key(None).unwrap(); + let key_pair = secret.key_pair(None).unwrap(); + + let entry = StellarEntry::new("test").unwrap(); + + // set the password + let set_password_result = entry.set_password(&key_pair.to_bytes()); + assert!(set_password_result.is_ok()); + + // get_password should return the same password we set + let get_password_result = entry.get_password(); + assert!(get_password_result.is_ok()); + assert_eq!(key_pair.to_bytes().to_vec(), get_password_result.unwrap()); + } + + #[test] + fn test_get_public_key() { + set_default_credential_builder(mock::default_credential_builder()); + + let secret = crate::config::secret::Secret::from_seed(None).unwrap(); + let public_key = secret.public_key(None).unwrap(); let key_pair = secret.key_pair(None).unwrap(); let entry = StellarEntry::new("test").unwrap(); @@ -93,6 +109,24 @@ mod test { // confirm that we can get the public key from the entry and that it matches the one we set let get_public_key_result = entry.get_public_key(); assert!(get_public_key_result.is_ok()); - assert_eq!(pub_key, get_public_key_result.unwrap()); + assert_eq!(public_key, get_public_key_result.unwrap()); + } + + #[test] + fn test_sign_data() { + set_default_credential_builder(mock::default_credential_builder()); + + //create a secret + let secret = crate::config::secret::Secret::from_seed(None).unwrap(); + let key_pair = secret.key_pair(None).unwrap(); + + // create a keyring entry and set the password + let entry = StellarEntry::new("test").unwrap(); + entry.set_password(&key_pair.to_bytes()).unwrap(); + + let tx_xdr = r#"AAAAAgAAAADh6eOnZEq1xQgKioffuH7/8D8x8+OdGFEkiYC6QKMWzQAAAGQAAACuAAAAAQAAAAAAAAAAAAAAAQAAAAAAAAAYAAAAAQAAAAAAAAAAAAAAAOHp46dkSrXFCAqKh9+4fv/wPzHz450YUSSJgLpAoxbNoFT1s8jZPCv9IJ2DsqGTA8pOtavv58JF53aDycpRPcEAAAAA+N2m5zc3EfWUmLvigYPOHKXhSy8OrWfVibc6y6PrQoYAAAAAAAAAAAAAAAA"#; + + let sign_tx_env_result = entry.sign_data(tx_xdr.as_bytes()); + assert!(sign_tx_env_result.is_ok()); } } From 20c651bc211a253472baaa719ac2ec6c1885ea76 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:17:03 -0500 Subject: [PATCH 14/42] Update config/secret tests --- cmd/soroban-cli/src/config/secret.rs | 30 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index ebc2b8d3f..e7fbea44c 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -188,39 +188,39 @@ fn read_password() -> Result { mod tests { use super::*; + const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ"; const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH"; const TEST_SEED_PHRASE: &str = "depth decade power loud smile spatial sign movie judge february rate broccoli"; #[test] - fn test_secret_from_key() { + fn test_from_str_for_secret_key() { let secret = Secret::from_str(TEST_SECRET_KEY).unwrap(); - // assert that it is a Secret::SecretKey - assert!(matches!(secret, Secret::SecretKey { .. })); - // assert that we can get the private key from it + let public_key = secret.public_key(None).unwrap(); let private_key = secret.private_key(None).unwrap(); + + assert!(matches!(secret, Secret::SecretKey { .. })); + assert_eq!(public_key.to_string(), TEST_PUBLIC_KEY); assert_eq!(private_key.to_string(), TEST_SECRET_KEY); } #[test] - fn test_secret_from_seed_phrase() { + fn test_from_str_for_seed_phrase() { let secret = Secret::from_str(TEST_SEED_PHRASE).unwrap(); - assert!(matches!(secret, Secret::SeedPhrase { .. })); - + let public_key = secret.public_key(None).unwrap(); let private_key = secret.private_key(None).unwrap(); + + assert!(matches!(secret, Secret::SeedPhrase { .. })); + assert_eq!(public_key.to_string(), TEST_PUBLIC_KEY); assert_eq!(private_key.to_string(), TEST_SECRET_KEY); } #[test] - fn test_keychain_secret() { - let keychain_secret = Secret::from_str("keychain:org.stellar.cli-alice").unwrap(); + fn test_from_str_for_keychain_secret() { + //todo: add assertion for getting public key - will need to mock the keychain and add the keypair to the keychain + let secret = Secret::from_str("keychain:org.stellar.cli-alice").unwrap(); - match keychain_secret { - Secret::Keychain { entry_name } => { - assert_eq!(entry_name, "keychain:org.stellar.cli-alice"); - } - _ => assert!(false), - } + assert!(matches!(secret, Secret::Keychain { .. })); } #[test] From 66d1bfc46b2ab8bef535c66028f7d002fae65be0 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:49:06 -0500 Subject: [PATCH 15/42] Cleanup --- cmd/soroban-cli/src/commands/keys/generate.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 3bf2e48be..93ae67a3b 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -35,9 +35,11 @@ pub enum Error { pub struct Cmd { /// Name of identity pub name: KeyName, + /// Do not fund address #[arg(long)] pub no_fund: bool, + /// Optional seed to use when generating seed phrase. /// Random otherwise. #[arg(long, conflicts_with = "default_seed")] @@ -162,7 +164,7 @@ impl Cmd { #[cfg(test)] mod tests { - use crate::config::{self, address::KeyName, secret::Secret}; + use crate::config::{address::KeyName, secret::Secret}; use keyring::{mock, set_default_credential_builder}; fn set_up_test() -> (super::locator::Args, super::Cmd) { @@ -186,8 +188,6 @@ mod tests { overwrite: false, }; - set_default_credential_builder(mock::default_credential_builder()); - (locator, cmd) } @@ -222,6 +222,7 @@ mod tests { #[tokio::test] async fn test_storing_secret_in_keychain() { + set_default_credential_builder(mock::default_credential_builder()); let (test_locator, mut cmd) = set_up_test(); cmd.keychain = true; let global_args = global_args(); From 8b9763ee158eeb746eb4d7b42072478d98e2b1b6 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:58:17 -0500 Subject: [PATCH 16/42] Rename keychain arg to secure_store in generate --- cmd/soroban-cli/src/commands/keys/generate.rs | 31 ++++++++++--------- cmd/soroban-cli/src/config/secret.rs | 2 +- cmd/soroban-cli/src/signer/keyring.rs | 4 +-- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 93ae67a3b..0314ddf7d 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -49,9 +49,9 @@ pub struct Cmd { #[arg(long, short = 's')] pub as_secret: bool, - /// Save in `keychain` + /// Save in OS-specific secure store #[arg(long)] - pub keychain: bool, + pub secure_store: bool, #[command(flatten)] pub config_locator: locator::Args, @@ -119,19 +119,20 @@ impl Cmd { let seed_phrase = self.seed_phrase()?; Ok(if self.as_secret { seed_phrase.private_key(self.hd_path)?.into() - } else if self.keychain { - // keychain:org.stellar.cli: + } else if self.secure_store { + // secure_store:org.stellar.cli: let entry_name_with_prefix = format!( "{}{}-{}", - keyring::KEYCHAIN_ENTRY_PREFIX, - keyring::KEYCHAIN_ENTRY_SERVICE, + keyring::SECURE_STORE_ENTRY_PREFIX, + keyring::SECURE_STORE_ENTRY_SERVICE, self.name.to_string() ); - let secret: Secret = entry_name_with_prefix.parse()?; //checking that the entry name is valid before writing to the keychain + //checking that the entry name is valid before writing to the secure store + let secret: Secret = entry_name_with_prefix.parse()?; if let Secret::Keychain { entry_name } = &secret { - self.write_to_keychain(entry_name.clone(), seed_phrase)?; + self.write_to_secure_store(entry_name.clone(), seed_phrase)?; } secret @@ -148,13 +149,13 @@ impl Cmd { }?) } - fn write_to_keychain(&self, entry_name: String, seed_phrase: Secret) -> Result<(), Error> { - println!("Writing to keychain: {entry_name}"); + fn write_to_secure_store(&self, entry_name: String, seed_phrase: Secret) -> Result<(), Error> { + println!("Writing to secure store: {entry_name}"); let entry = StellarEntry::new(&entry_name)?; if let Ok(key) = entry.get_public_key() { - println!("A key for {entry_name} already exists in your keychain: {key}"); + println!("A key for {entry_name} already exists in your operating system's secure store: {key}"); } else { - println!("Saving a new key to your keychain: {entry_name}"); + println!("Saving a new key to your operating system's secure store: {entry_name}"); let key_pair = seed_phrase.key_pair(None)?; entry.set_password(key_pair.as_bytes())?; } @@ -179,7 +180,7 @@ mod tests { no_fund: true, seed: None, as_secret: false, - keychain: false, + secure_store: false, config_locator: locator.clone(), hd_path: None, default_seed: false, @@ -221,10 +222,10 @@ mod tests { } #[tokio::test] - async fn test_storing_secret_in_keychain() { + async fn test_storing_secret_in_secure_store() { set_default_credential_builder(mock::default_credential_builder()); let (test_locator, mut cmd) = set_up_test(); - cmd.keychain = true; + cmd.secure_store = true; let global_args = global_args(); let result = cmd.run(&global_args).await; diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index e7fbea44c..5cdb30504 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -99,7 +99,7 @@ impl FromStr for Secret { Ok(Secret::SeedPhrase { seed_phrase: s.to_string(), }) - } else if s.starts_with(keyring::KEYCHAIN_ENTRY_PREFIX) { + } else if s.starts_with(keyring::SECURE_STORE_ENTRY_PREFIX) { Ok(Secret::Keychain { entry_name: s.to_string(), }) diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index 6c32f620d..f92280f7e 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -3,8 +3,8 @@ use ed25519_dalek::Signer; use keyring::Entry; use zeroize::Zeroize; -pub(crate) const KEYCHAIN_ENTRY_PREFIX: &str = "keychain:"; //TODO: does this belong here, or in secret? -pub(crate) const KEYCHAIN_ENTRY_SERVICE: &str = "org.stellar.cli"; +pub(crate) const SECURE_STORE_ENTRY_PREFIX: &str = "secure_store:"; +pub(crate) const SECURE_STORE_ENTRY_SERVICE: &str = "org.stellar.cli"; #[derive(thiserror::Error, Debug)] pub enum Error { From fdefa2aee53631c00761089fb4c7b9f162be79d7 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 17:02:43 -0500 Subject: [PATCH 17/42] Rename Secret::Keychain to Secret::SecureStore --- cmd/soroban-cli/src/commands/keys/generate.rs | 4 ++-- cmd/soroban-cli/src/config/secret.rs | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 0314ddf7d..492d17bd4 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -131,7 +131,7 @@ impl Cmd { //checking that the entry name is valid before writing to the secure store let secret: Secret = entry_name_with_prefix.parse()?; - if let Secret::Keychain { entry_name } = &secret { + if let Secret::SecureStore { entry_name } = &secret { self.write_to_secure_store(entry_name.clone(), seed_phrase)?; } @@ -231,6 +231,6 @@ mod tests { let result = cmd.run(&global_args).await; assert!(result.is_ok()); let identity = test_locator.read_identity("test_name").unwrap(); - assert!(matches!(identity, Secret::Keychain { .. })); + assert!(matches!(identity, Secret::SecureStore { .. })); } } diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index 5cdb30504..a1adabeb8 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -84,7 +84,7 @@ impl Args { pub enum Secret { SecretKey { secret_key: String }, SeedPhrase { seed_phrase: String }, - Keychain { entry_name: String }, + SecureStore { entry_name: String }, } impl FromStr for Secret { @@ -100,7 +100,7 @@ impl FromStr for Secret { seed_phrase: s.to_string(), }) } else if s.starts_with(keyring::SECURE_STORE_ENTRY_PREFIX) { - Ok(Secret::Keychain { + Ok(Secret::SecureStore { entry_name: s.to_string(), }) } else { @@ -127,13 +127,13 @@ impl Secret { .private() .0, )?, - Secret::Keychain { .. } => panic!("Keychain does not reveal secret key"), + Secret::SecureStore { .. } => panic!("Secure Store does not reveal secret key"), }) } pub fn public_key(&self, index: Option) -> Result { match self { - Secret::Keychain { entry_name } => { + Secret::SecureStore { entry_name } => { let entry = keyring::StellarEntry::new(entry_name)?; Ok(entry.get_public_key()?) } @@ -152,7 +152,7 @@ impl Secret { let key = self.key_pair(index)?; SignerKind::Local(LocalKey { key }) } - Secret::Keychain { entry_name } => SignerKind::Keychain(KeychainEntry { + Secret::SecureStore { entry_name } => SignerKind::Keychain(KeychainEntry { name: entry_name.to_string(), }), }; @@ -216,17 +216,17 @@ mod tests { } #[test] - fn test_from_str_for_keychain_secret() { + fn test_from_str_for_secure_store_secret() { //todo: add assertion for getting public key - will need to mock the keychain and add the keypair to the keychain - let secret = Secret::from_str("keychain:org.stellar.cli-alice").unwrap(); + let secret = Secret::from_str("secure_store:org.stellar.cli-alice").unwrap(); - assert!(matches!(secret, Secret::Keychain { .. })); + assert!(matches!(secret, Secret::SecureStore { .. })); } #[test] #[should_panic] - fn test_keychain_secret_will_not_reveal_private_key() { - let secret = Secret::from_str("keychain").unwrap(); + fn test_secure_store_will_not_reveal_private_key() { + let secret = Secret::from_str("secure_store").unwrap(); secret.private_key(None).unwrap(); } From 36559eae08f54ac504d8ba90608fe71f83bd5f97 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 25 Nov 2024 17:22:09 -0500 Subject: [PATCH 18/42] Rename SignerKind::Keychain to SignerKind::SecureStore --- cmd/soroban-cli/src/commands/keys/mod.rs | 2 +- cmd/soroban-cli/src/config/secret.rs | 12 ++++-------- cmd/soroban-cli/src/signer.rs | 10 +++++----- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/mod.rs b/cmd/soroban-cli/src/commands/keys/mod.rs index 8729ee9af..b3309fa02 100644 --- a/cmd/soroban-cli/src/commands/keys/mod.rs +++ b/cmd/soroban-cli/src/commands/keys/mod.rs @@ -12,7 +12,7 @@ pub mod show; #[derive(Debug, Parser)] pub enum Cmd { - /// Add a new identity (keypair, ledger, macOS keychain) + /// Add a new identity (keypair, ledger, OS specific secure store) Add(add::Cmd), /// Given an identity return its address (public key) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index a1adabeb8..47ac9f401 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -5,7 +5,7 @@ use stellar_strkey::ed25519::{PrivateKey, PublicKey}; use crate::{ print::Print, - signer::{self, keyring, KeychainEntry, LocalKey, Signer, SignerKind}, + signer::{self, keyring, LocalKey, SecureStoreEntry, Signer, SignerKind}, utils, }; @@ -36,15 +36,11 @@ pub enum Error { pub struct Args { /// Add using `secret_key` /// Can provide with `SOROBAN_SECRET_KEY` - #[arg(long, conflicts_with_all = ["seed_phrase", "keychain"])] + #[arg(long, conflicts_with = "seed_phrase")] pub secret_key: bool, /// Add using 12 word seed phrase to generate `secret_key` - #[arg(long, conflicts_with_all = ["secret_key", "keychain"])] + #[arg(long, conflicts_with = "secret_key")] pub seed_phrase: bool, - - /// Add using `keychain` - #[arg(long, conflicts_with_all = ["seed_phrase", "secret_key"])] - pub keychain: bool, } impl Args { @@ -152,7 +148,7 @@ impl Secret { let key = self.key_pair(index)?; SignerKind::Local(LocalKey { key }) } - Secret::SecureStore { entry_name } => SignerKind::Keychain(KeychainEntry { + Secret::SecureStore { entry_name } => SignerKind::SecureStore(SecureStoreEntry { name: entry_name.to_string(), }), }; diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index e0a1568c2..86db2dcea 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -1,4 +1,4 @@ -use ed25519_dalek::ed25519::signature::{self, Signer as _}; +use ed25519_dalek::ed25519::signature::Signer as _; use keyring::StellarEntry; use sha2::{Digest, Sha256}; @@ -212,7 +212,7 @@ pub struct Signer { pub enum SignerKind { Local(LocalKey), Lab, - Keychain(KeychainEntry), + SecureStore(SecureStoreEntry), } impl Signer { @@ -241,7 +241,7 @@ impl Signer { let decorated_signature = match &self.kind { SignerKind::Local(key) => key.sign_tx_hash(tx_hash)?, SignerKind::Lab => Lab::sign_tx_env(tx_env, network, &self.print)?, - SignerKind::Keychain(entry) => entry.sign_tx_env(tx_env)?, + SignerKind::SecureStore(entry) => entry.sign_tx_env(tx_env)?, }; let mut sigs = signatures.clone().into_vec(); sigs.push(decorated_signature); @@ -292,11 +292,11 @@ impl Lab { } } -pub struct KeychainEntry { +pub struct SecureStoreEntry { pub name: String, } -impl KeychainEntry { +impl SecureStoreEntry { pub fn sign_tx_env(&self, tx_env: &TransactionEnvelope) -> Result { let entry = StellarEntry::new(&self.name)?; let signed_tx_env = entry.sign_data(tx_env.to_xdr_base64(Limits::none())?.as_bytes())?; From f98b709c3a09d22bdfeddba0a1d77fb1b5d64110 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 2 Dec 2024 12:00:44 -0500 Subject: [PATCH 19/42] Use print for new fns in generate --- cmd/soroban-cli/src/commands/keys/generate.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 492d17bd4..0395a593c 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -97,7 +97,7 @@ impl Cmd { warning. It can be suppressed with -q flag.", ); } - let secret = self.secret()?; + let secret = self.secret(print)?; self.config_locator.write_identity(&self.name, &secret)?; if !self.no_fund { @@ -115,7 +115,7 @@ impl Cmd { Ok(()) } - fn secret(&self) -> Result { + fn secret(&self, print: Print) -> Result { let seed_phrase = self.seed_phrase()?; Ok(if self.as_secret { seed_phrase.private_key(self.hd_path)?.into() @@ -132,7 +132,7 @@ impl Cmd { let secret: Secret = entry_name_with_prefix.parse()?; if let Secret::SecureStore { entry_name } = &secret { - self.write_to_secure_store(entry_name.clone(), seed_phrase)?; + self.write_to_secure_store(entry_name.clone(), seed_phrase, print)?; } secret @@ -149,13 +149,20 @@ impl Cmd { }?) } - fn write_to_secure_store(&self, entry_name: String, seed_phrase: Secret) -> Result<(), Error> { + fn write_to_secure_store( + &self, + entry_name: String, + seed_phrase: Secret, + print: Print, + ) -> Result<(), Error> { println!("Writing to secure store: {entry_name}"); let entry = StellarEntry::new(&entry_name)?; if let Ok(key) = entry.get_public_key() { - println!("A key for {entry_name} already exists in your operating system's secure store: {key}"); + print.warnln(format!("A key for {entry_name} already exists in your operating system's secure store: {key}")); } else { - println!("Saving a new key to your operating system's secure store: {entry_name}"); + print.infoln(format!( + "Saving a new key to your operating system's secure store: {entry_name}" + )); let key_pair = seed_phrase.key_pair(None)?; entry.set_password(key_pair.as_bytes())?; } From 0f3106b2cc3d84d6fc6d28e1b29b66010e03f52e Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 2 Dec 2024 12:08:09 -0500 Subject: [PATCH 20/42] Return error when trying to get Secure Store secret --- cmd/soroban-cli/src/config/secret.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index 47ac9f401..49887074c 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -29,6 +29,8 @@ pub enum Error { Signer(#[from] signer::Error), #[error(transparent)] Keyring(#[from] keyring::Error), + #[error("Secure Store does not reveal secret key")] + SecureStoreDoesNotRevealSecretKey, } #[derive(Debug, clap::Args, Clone)] @@ -123,7 +125,9 @@ impl Secret { .private() .0, )?, - Secret::SecureStore { .. } => panic!("Secure Store does not reveal secret key"), + Secret::SecureStore { .. } => { + return Err(Error::SecureStoreDoesNotRevealSecretKey); + } }) } From e120595a440760cc8b7c737928d56cbf37929a5d Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:58:49 -0500 Subject: [PATCH 21/42] Cleanup tests --- cmd/soroban-cli/src/config/secret.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index 49887074c..b0070b28c 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -205,7 +205,7 @@ mod tests { } #[test] - fn test_from_str_for_seed_phrase() { + fn test_secret_from_seed_phrase() { let secret = Secret::from_str(TEST_SEED_PHRASE).unwrap(); let public_key = secret.public_key(None).unwrap(); let private_key = secret.private_key(None).unwrap(); @@ -216,18 +216,17 @@ mod tests { } #[test] - fn test_from_str_for_secure_store_secret() { + fn test_secret_from_secure_store() { //todo: add assertion for getting public key - will need to mock the keychain and add the keypair to the keychain let secret = Secret::from_str("secure_store:org.stellar.cli-alice").unwrap(); - assert!(matches!(secret, Secret::SecureStore { .. })); - } - #[test] - #[should_panic] - fn test_secure_store_will_not_reveal_private_key() { - let secret = Secret::from_str("secure_store").unwrap(); - secret.private_key(None).unwrap(); + let private_key_result = secret.private_key(None); + assert!(private_key_result.is_err()); + assert!(matches!( + private_key_result.unwrap_err(), + Error::SecureStoreDoesNotRevealSecretKey + )); } #[test] From 57ba3a489a8294421aae822eb8f2942013c3f98a Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:19:02 -0500 Subject: [PATCH 22/42] Install libdbus for rpc-tests and bindings-ts workflows required for keyring crate --- .github/workflows/bindings-ts.yml | 1 + .github/workflows/rpc-tests.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/bindings-ts.yml b/.github/workflows/bindings-ts.yml index 957661c4c..10cf23007 100644 --- a/.github/workflows/bindings-ts.yml +++ b/.github/workflows/bindings-ts.yml @@ -35,6 +35,7 @@ jobs: target/ key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - run: rustup update + - run: sudo apt install -y libdbus-1-dev - run: cargo build - run: rustup target add wasm32-unknown-unknown - run: make build-test-wasms diff --git a/.github/workflows/rpc-tests.yml b/.github/workflows/rpc-tests.yml index 75b6d7760..5392d9830 100644 --- a/.github/workflows/rpc-tests.yml +++ b/.github/workflows/rpc-tests.yml @@ -39,6 +39,7 @@ jobs: target/ key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - run: rustup update + - run: sudo apt install -y libdbus-1-dev - run: cargo build - run: rustup target add wasm32-unknown-unknown - run: make build-test-wasms From 0814e4b1ea4cca9258de3b615fb1a3b539f971bc Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:14:03 -0500 Subject: [PATCH 23/42] Update generated docs --- FULL_HELP_DOCS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index a755ef18a..70e0d19ff 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -933,7 +933,7 @@ Create and manage identities including keys and addresses ###### **Subcommands:** -* `add` — Add a new identity (keypair, ledger, macOS keychain) +* `add` — Add a new identity (keypair, ledger, OS specific secure store) * `address` — Given an identity return its address (public key) * `fund` — Fund an identity on a test network * `generate` — Generate a new identity with a seed phrase, currently 12 words @@ -946,7 +946,7 @@ Create and manage identities including keys and addresses ## `stellar keys add` -Add a new identity (keypair, ledger, macOS keychain) +Add a new identity (keypair, ledger, OS specific secure store) **Usage:** `stellar keys add [OPTIONS] ` @@ -1018,6 +1018,7 @@ Generate a new identity with a seed phrase, currently 12 words * `--no-fund` — Do not fund address * `--seed ` — Optional seed to use when generating seed phrase. Random otherwise * `-s`, `--as-secret` — Output the generated identity as a secret key +* `--secure-store` — Save in OS-specific secure store * `--global` — Use global config * `--config-dir ` — Location of config directory, default is "." * `--hd-path ` — When generating a secret key, which `hd_path` should be used from the original `seed_phrase` From 74fa05b75e093fd3c1449ab890c31c2692026a6d Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:17:23 -0500 Subject: [PATCH 24/42] Install libdbus for binaries workflow when target aarch64-unknown-linux-gnu --- .github/workflows/binaries.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index d913cf23d..f22865da8 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -46,7 +46,7 @@ jobs: - run: rustup target add ${{ matrix.sys.target }} - if: matrix.sys.target == 'aarch64-unknown-linux-gnu' - run: sudo apt-get update && sudo apt-get -y install gcc-aarch64-linux-gnu g++-aarch64-linux-gnu libudev-dev + run: sudo apt-get update && sudo apt-get -y install gcc-aarch64-linux-gnu g++-aarch64-linux-gnu libudev-dev libdbus-1-dev - name: Setup vars run: | From 7ff1f6a4907b7f7c76969952bc4a16f0ea6babcc Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:45:53 -0500 Subject: [PATCH 25/42] Clippy --- cmd/soroban-cli/src/commands/keys/generate.rs | 28 +++++++++---------- cmd/soroban-cli/src/config/secret.rs | 19 ++++++------- cmd/soroban-cli/src/signer.rs | 2 +- cmd/soroban-cli/src/signer/keyring.rs | 2 +- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 0395a593c..72597aff3 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -97,7 +97,7 @@ impl Cmd { warning. It can be suppressed with -q flag.", ); } - let secret = self.secret(print)?; + let secret = self.secret(&print)?; self.config_locator.write_identity(&self.name, &secret)?; if !self.no_fund { @@ -115,14 +115,14 @@ impl Cmd { Ok(()) } - fn secret(&self, print: Print) -> Result { + fn secret(&self, print: &Print) -> Result { let seed_phrase = self.seed_phrase()?; Ok(if self.as_secret { seed_phrase.private_key(self.hd_path)?.into() } else if self.secure_store { // secure_store:org.stellar.cli: let entry_name_with_prefix = format!( - "{}{}-{}", + "{}{}-{:?}", keyring::SECURE_STORE_ENTRY_PREFIX, keyring::SECURE_STORE_ENTRY_SERVICE, self.name.to_string() @@ -132,7 +132,7 @@ impl Cmd { let secret: Secret = entry_name_with_prefix.parse()?; if let Secret::SecureStore { entry_name } = &secret { - self.write_to_secure_store(entry_name.clone(), seed_phrase, print)?; + Self::write_to_secure_store(entry_name, &seed_phrase, print)?; } secret @@ -150,13 +150,12 @@ impl Cmd { } fn write_to_secure_store( - &self, - entry_name: String, - seed_phrase: Secret, - print: Print, + entry_name: &String, + seed_phrase: &Secret, + print: &Print, ) -> Result<(), Error> { - println!("Writing to secure store: {entry_name}"); - let entry = StellarEntry::new(&entry_name)?; + print.infoln(format!("Writing to secure store: {entry_name}")); + let entry = StellarEntry::new(entry_name)?; if let Ok(key) = entry.get_public_key() { print.warnln(format!("A key for {entry_name} already exists in your operating system's secure store: {key}")); } else { @@ -191,7 +190,7 @@ mod tests { config_locator: locator.clone(), hd_path: None, default_seed: false, - network: Default::default(), + network: super::network::Args::default(), fund: false, overwrite: false, }; @@ -200,9 +199,10 @@ mod tests { } fn global_args() -> super::global::Args { - let mut global_args = super::global::Args::default(); - global_args.quiet = true; - global_args + super::global::Args { + quiet: true, + ..Default::default() + } } #[tokio::test] diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index b0070b28c..cd3bc908a 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -132,17 +132,14 @@ impl Secret { } pub fn public_key(&self, index: Option) -> Result { - match self { - Secret::SecureStore { entry_name } => { - let entry = keyring::StellarEntry::new(entry_name)?; - Ok(entry.get_public_key()?) - } - _ => { - let key = self.key_pair(index)?; - Ok(stellar_strkey::ed25519::PublicKey::from_payload( - key.verifying_key().as_bytes(), - )?) - } + if let Secret::SecureStore { entry_name } = self { + let entry = keyring::StellarEntry::new(entry_name)?; + Ok(entry.get_public_key()?) + } else { + let key = self.key_pair(index)?; + Ok(stellar_strkey::ed25519::PublicKey::from_payload( + key.verifying_key().as_bytes(), + )?) } } diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index 86db2dcea..ba9ba8667 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -301,7 +301,7 @@ impl SecureStoreEntry { let entry = StellarEntry::new(&self.name)?; let signed_tx_env = entry.sign_data(tx_env.to_xdr_base64(Limits::none())?.as_bytes())?; let hint = SignatureHint(entry.get_public_key()?.0[28..].try_into()?); - let signature = Signature(signed_tx_env.to_vec().try_into()?); + let signature = Signature(signed_tx_env.clone().try_into()?); Ok(DecoratedSignature { hint, signature }) } } diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index f92280f7e..b5a63a398 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -124,7 +124,7 @@ mod test { let entry = StellarEntry::new("test").unwrap(); entry.set_password(&key_pair.to_bytes()).unwrap(); - let tx_xdr = r#"AAAAAgAAAADh6eOnZEq1xQgKioffuH7/8D8x8+OdGFEkiYC6QKMWzQAAAGQAAACuAAAAAQAAAAAAAAAAAAAAAQAAAAAAAAAYAAAAAQAAAAAAAAAAAAAAAOHp46dkSrXFCAqKh9+4fv/wPzHz450YUSSJgLpAoxbNoFT1s8jZPCv9IJ2DsqGTA8pOtavv58JF53aDycpRPcEAAAAA+N2m5zc3EfWUmLvigYPOHKXhSy8OrWfVibc6y6PrQoYAAAAAAAAAAAAAAAA"#; + let tx_xdr = r"AAAAAgAAAADh6eOnZEq1xQgKioffuH7/8D8x8+OdGFEkiYC6QKMWzQAAAGQAAACuAAAAAQAAAAAAAAAAAAAAAQAAAAAAAAAYAAAAAQAAAAAAAAAAAAAAAOHp46dkSrXFCAqKh9+4fv/wPzHz450YUSSJgLpAoxbNoFT1s8jZPCv9IJ2DsqGTA8pOtavv58JF53aDycpRPcEAAAAA+N2m5zc3EfWUmLvigYPOHKXhSy8OrWfVibc6y6PrQoYAAAAAAAAAAAAAAAA"; let sign_tx_env_result = entry.sign_data(tx_xdr.as_bytes()); assert!(sign_tx_env_result.is_ok()); From f263d8dfcd5ed764470be40d92aef48b81abc174 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:17:52 -0500 Subject: [PATCH 26/42] Install libdbus for rust workflow --- .github/workflows/rust.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f3c9b1920..ff4f7d399 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -49,6 +49,7 @@ jobs: - uses: actions/checkout@v4 - uses: stellar/actions/rust-cache@main - run: rustup update + - run: sudo apt install -y libdbus-1-dev - run: make generate-full-help-doc - run: git add -N . && git diff HEAD --exit-code @@ -90,7 +91,7 @@ jobs: - run: rustup target add ${{ matrix.sys.target }} - run: rustup target add wasm32-unknown-unknown - if: runner.os == 'Linux' - run: sudo apt-get update && sudo apt-get -y install gcc-aarch64-linux-gnu g++-aarch64-linux-gnu libudev-dev + run: sudo apt-get update && sudo apt-get -y install gcc-aarch64-linux-gnu g++-aarch64-linux-gnu libudev-dev libdbus-1-dev - run: cargo clippy --all-targets --target ${{ matrix.sys.target }} - run: make test env: From e2b734204020df530b302eb3666e735a45fa3edb Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 13:33:12 -0500 Subject: [PATCH 27/42] Install libdbus-1-dev in binaries workflow for build step --- .github/workflows/binaries.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index f22865da8..ebfcf927c 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -69,6 +69,7 @@ jobs: env: CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER: aarch64-linux-gnu-gcc working-directory: ${{ env.BUILD_WORKING_DIR }} + run: sudo apt-get update && sudo apt-get -y install libdbus-1-dev run: cargo build --target-dir="$GITHUB_WORKSPACE/target" --package ${{ matrix.crate.name }} --features opt --release --target ${{ matrix.sys.target }} - name: Build provenance for attestation (release only) From dabbb2739f1ef63eebef22bc76ab3532a064fd63 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:30:02 -0500 Subject: [PATCH 28/42] Impl Display for KeyName this change was made so that we can concat the KeyName with secure story prefix and service --- cmd/soroban-cli/src/commands/keys/generate.rs | 4 ++-- cmd/soroban-cli/src/config/address.rs | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 72597aff3..bd9f8c2f6 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -122,10 +122,10 @@ impl Cmd { } else if self.secure_store { // secure_store:org.stellar.cli: let entry_name_with_prefix = format!( - "{}{}-{:?}", + "{}{}-{}", keyring::SECURE_STORE_ENTRY_PREFIX, keyring::SECURE_STORE_ENTRY_SERVICE, - self.name.to_string() + self.name ); //checking that the entry name is valid before writing to the secure store diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 57c229c76..961c26bb6 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -1,4 +1,7 @@ -use std::str::FromStr; +use std::{ + fmt::{self, Display, Formatter}, + str::FromStr, +}; use crate::xdr; @@ -87,6 +90,12 @@ impl std::str::FromStr for KeyName { } } +impl Display for KeyName { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + fn allowed_char(c: char) -> bool { c.is_ascii_alphanumeric() || c == '_' || c == '-' } From d9131b402fa343bd114d020fa7fb20db818b45cd Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:19:52 -0500 Subject: [PATCH 29/42] Use resolve_muxed_account in resolve_secret --- cmd/soroban-cli/src/config/address.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 961c26bb6..b75b89fe2 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -61,8 +61,8 @@ impl Address { pub fn resolve_secret(&self, locator: &locator::Args) -> Result { match &self { - Address::MuxedAccount(muxed_account) => Err(Error::CannotSign(muxed_account.clone())), Address::AliasOrSecret(alias) => Ok(locator.read_identity(alias)?), + a => Err(Error::CannotSign(a.resolve_muxed_account(locator, None)?)), } } } From 3ad6b9b482d18f3d035560d2bfe841171c71d65d Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:23:16 -0500 Subject: [PATCH 30/42] Use resolve_muxed_account to get public key --- .../src/commands/contract/arg_parsing.rs | 18 +++++----- cmd/soroban-cli/src/commands/keys/address.rs | 34 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index 21fa2f383..3681ccd88 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -14,7 +14,7 @@ use crate::xdr::{ }; use crate::commands::txn_result::TxnResult; -use crate::config::{self}; +use crate::config::{self, address, secret}; use soroban_spec_tools::Spec; #[derive(thiserror::Error, Debug)] @@ -39,6 +39,10 @@ pub enum Error { Xdr(#[from] xdr::Error), #[error(transparent)] StrVal(#[from] soroban_spec_tools::Error), + #[error(transparent)] + Address(#[from] address::Error), + #[error(transparent)] + Secret(#[from] secret::Error), #[error("Missing argument {0}")] MissingArgument(String), #[error("")] @@ -82,16 +86,12 @@ pub fn build_host_function_parameters( if let Some(mut val) = matches_.get_raw(&name) { let mut s = val.next().unwrap().to_string_lossy().to_string(); if matches!(i.type_, ScSpecTypeDef::Address) { - let cmd = crate::commands::keys::address::Cmd { - name: s.clone(), - hd_path: Some(0), - locator: config.locator.clone(), - }; - if let Ok(address) = cmd.public_key() { + let addr: address::Address = s.parse()?; + if let Ok(address) = addr.resolve_muxed_account(&config.locator, None) { s = address.to_string(); } - if let Ok(key) = cmd.private_key() { - signers.push(key); + if let Ok(key) = addr.resolve_secret(&config.locator) { + signers.push(SigningKey::from_bytes(&key.private_key(None)?.0)); } } spec.from_string(&s, &i.type_) diff --git a/cmd/soroban-cli/src/commands/keys/address.rs b/cmd/soroban-cli/src/commands/keys/address.rs index d13381b49..145d8c30a 100644 --- a/cmd/soroban-cli/src/commands/keys/address.rs +++ b/cmd/soroban-cli/src/commands/keys/address.rs @@ -1,8 +1,10 @@ -use crate::commands::config::secret; - -use super::super::config::locator; use clap::arg; +use crate::{ + commands::config::{address, locator, secret}, + xdr, +}; + #[derive(thiserror::Error, Debug)] pub enum Error { #[error(transparent)] @@ -13,13 +15,16 @@ pub enum Error { #[error(transparent)] StrKey(#[from] stellar_strkey::DecodeError), + + #[error(transparent)] + Address(#[from] address::Error), } #[derive(Debug, clap::Parser, Clone)] #[group(skip)] pub struct Cmd { /// Name of identity to lookup, default test identity used if not provided - pub name: String, + pub name: address::Address, /// If identity is a seed phrase use this hd path, default is 0 #[arg(long)] @@ -35,20 +40,15 @@ impl Cmd { Ok(()) } - pub fn private_key(&self) -> Result { - Ok(self - .locator - .read_identity(&self.name)? - .key_pair(self.hd_path)?) - } - pub fn public_key(&self) -> Result { - if let Ok(key) = stellar_strkey::ed25519::PublicKey::from_string(&self.name) { - Ok(key) - } else { - Ok(stellar_strkey::ed25519::PublicKey::from_payload( - self.private_key()?.verifying_key().as_bytes(), - )?) + match self + .name + .resolve_muxed_account(&self.locator, self.hd_path)? + { + xdr::MuxedAccount::Ed25519(pk) => Ok(stellar_strkey::ed25519::PublicKey(pk.0)), + xdr::MuxedAccount::MuxedEd25519(xdr::MuxedAccountMed25519 { ed25519, .. }) => { + Ok(stellar_strkey::ed25519::PublicKey(ed25519.0)) + } } } } From 9d02b01bda2c67c97951563c8e04d6c5cf3105bf Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:46:05 -0500 Subject: [PATCH 31/42] Clippy --- cmd/soroban-cli/src/config/address.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index b75b89fe2..ff926b60a 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -62,7 +62,9 @@ impl Address { pub fn resolve_secret(&self, locator: &locator::Args) -> Result { match &self { Address::AliasOrSecret(alias) => Ok(locator.read_identity(alias)?), - a => Err(Error::CannotSign(a.resolve_muxed_account(locator, None)?)), + a @ Address::MuxedAccount(_) => { + Err(Error::CannotSign(a.resolve_muxed_account(locator, None)?)) + } } } } From a72276dc540e3a98638756e57d96a5d0758a671d Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Wed, 4 Dec 2024 12:06:30 -0500 Subject: [PATCH 32/42] fix: Sign tx hash instead of tx env with keychain --- cmd/soroban-cli/src/signer.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index ba9ba8667..099ed6186 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -241,7 +241,7 @@ impl Signer { let decorated_signature = match &self.kind { SignerKind::Local(key) => key.sign_tx_hash(tx_hash)?, SignerKind::Lab => Lab::sign_tx_env(tx_env, network, &self.print)?, - SignerKind::SecureStore(entry) => entry.sign_tx_env(tx_env)?, + SignerKind::SecureStore(entry) => entry.sign_tx_hash(tx_hash)?, }; let mut sigs = signatures.clone().into_vec(); sigs.push(decorated_signature); @@ -297,11 +297,11 @@ pub struct SecureStoreEntry { } impl SecureStoreEntry { - pub fn sign_tx_env(&self, tx_env: &TransactionEnvelope) -> Result { + pub fn sign_tx_hash(&self, tx_hash: [u8; 32]) -> Result { let entry = StellarEntry::new(&self.name)?; - let signed_tx_env = entry.sign_data(tx_env.to_xdr_base64(Limits::none())?.as_bytes())?; let hint = SignatureHint(entry.get_public_key()?.0[28..].try_into()?); - let signature = Signature(signed_tx_env.clone().try_into()?); + let signed_tx_hash = entry.sign_data(&tx_hash)?; + let signature = Signature(signed_tx_hash.clone().try_into()?); Ok(DecoratedSignature { hint, signature }) } } From 428863026466e0f581dcbb4bbbd932d0e417c121 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 9 Dec 2024 16:19:01 -0500 Subject: [PATCH 33/42] Remove unused bin/secret --- cmd/soroban-cli/Cargo.toml | 4 ---- cmd/soroban-cli/src/bin/secret.rs | 17 ----------------- 2 files changed, 21 deletions(-) delete mode 100644 cmd/soroban-cli/src/bin/secret.rs diff --git a/cmd/soroban-cli/Cargo.toml b/cmd/soroban-cli/Cargo.toml index 0d6da167d..3d366464b 100644 --- a/cmd/soroban-cli/Cargo.toml +++ b/cmd/soroban-cli/Cargo.toml @@ -20,10 +20,6 @@ path = "src/bin/stellar.rs" name = "soroban" path = "src/bin/soroban.rs" -[[bin]] -name = "secret" -path = "src/bin/secret.rs" - [package.metadata.binstall] pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ version }-{ target }{ archive-suffix }" bin-dir = "{ bin }{ binary-ext }" diff --git a/cmd/soroban-cli/src/bin/secret.rs b/cmd/soroban-cli/src/bin/secret.rs deleted file mode 100644 index 4fc43ec6e..000000000 --- a/cmd/soroban-cli/src/bin/secret.rs +++ /dev/null @@ -1,17 +0,0 @@ -use soroban_cli::signer::keyring::StellarEntry; - -fn main() { - let entry = StellarEntry::new("test").unwrap(); - if let Ok(key) = entry.get_public_key() { - println!("{key}"); - return; - }; - - let secret = soroban_cli::config::secret::Secret::from_seed(None).unwrap(); - let pub_key = secret.public_key(None).unwrap(); - let key_pair = secret.key_pair(None).unwrap(); - entry.set_password(key_pair.as_bytes()).unwrap(); - let pub_key_2 = entry.get_public_key().unwrap(); - assert_eq!(pub_key, pub_key_2); - println!("{pub_key} == {pub_key_2}"); -} From 497efe8d9b7dc9be3d8feb1bb90b5098938e9294 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Fri, 20 Dec 2024 14:58:10 -0500 Subject: [PATCH 34/42] feat(contract invoke): support contract aliases when parsing address (#1765) * feat(contract invoke): support contract aliases when parsing address fixes: 1764 * fix: only parse with address typedefs * fix: fmt * split two logics combined into one fn * feat: rename to unresolvedX to make it names clearer * fix: fmt and add tests * feat: ban overlapping names * fix: tests * fix: tests --------- Co-authored-by: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> --- cmd/crates/soroban-test/tests/it/config.rs | 44 ++++++++++++ .../tests/it/integration/custom_types.rs | 26 ++++++- .../soroban-test/tests/it/integration/tx.rs | 2 +- .../tests/it/integration/tx/operations.rs | 2 +- .../soroban-test/tests/it/integration/util.rs | 11 +-- .../src/commands/contract/arg_parsing.rs | 60 ++++++++++++----- .../src/commands/contract/fetch.rs | 2 +- .../src/commands/contract/info/shared.rs | 2 +- .../src/commands/contract/invoke.rs | 2 +- cmd/soroban-cli/src/commands/events.rs | 2 +- .../src/commands/snapshot/create.rs | 4 +- .../src/commands/tx/op/add/args.rs | 2 +- cmd/soroban-cli/src/config/address.rs | 44 +++++++----- cmd/soroban-cli/src/config/alias.rs | 34 ++++++---- cmd/soroban-cli/src/config/locator.rs | 10 +++ cmd/soroban-cli/src/config/mod.rs | 8 ++- cmd/soroban-cli/src/config/sc_address.rs | 67 +++++++++++++++++++ cmd/soroban-cli/src/key.rs | 2 +- 18 files changed, 263 insertions(+), 61 deletions(-) create mode 100644 cmd/soroban-cli/src/config/sc_address.rs diff --git a/cmd/crates/soroban-test/tests/it/config.rs b/cmd/crates/soroban-test/tests/it/config.rs index e81233b6e..2e5bc21c1 100644 --- a/cmd/crates/soroban-test/tests/it/config.rs +++ b/cmd/crates/soroban-test/tests/it/config.rs @@ -393,3 +393,47 @@ fn set_default_network() { .stdout(predicate::str::contains("STELLAR_NETWORK=testnet")) .success(); } + +#[test] +fn cannot_create_contract_with_test_name() { + let sandbox = TestEnv::default(); + sandbox + .new_assert_cmd("keys") + .arg("generate") + .arg("--no-fund") + .arg("d") + .assert() + .success(); + sandbox + .new_assert_cmd("contract") + .arg("alias") + .arg("add") + .arg("d") + .arg("--id=CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE") + .assert() + .stderr(predicate::str::contains("cannot overlap with key")) + .failure(); +} + +#[test] +fn cannot_create_key_with_alias() { + let sandbox = TestEnv::default(); + sandbox + .new_assert_cmd("contract") + .arg("alias") + .arg("add") + .arg("t") + .arg("--id=CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE") + .assert() + .success(); + sandbox + .new_assert_cmd("keys") + .arg("generate") + .arg("--no-fund") + .arg("t") + .assert() + .stderr(predicate::str::contains( + "cannot overlap with contract alias", + )) + .failure(); +} diff --git a/cmd/crates/soroban-test/tests/it/integration/custom_types.rs b/cmd/crates/soroban-test/tests/it/integration/custom_types.rs index 6cdb61192..f4c2be61b 100644 --- a/cmd/crates/soroban-test/tests/it/integration/custom_types.rs +++ b/cmd/crates/soroban-test/tests/it/integration/custom_types.rs @@ -5,7 +5,7 @@ use soroban_test::TestEnv; use crate::integration::util::{deploy_custom, extend_contract}; -use super::util::invoke_with_roundtrip; +use super::util::{invoke, invoke_with_roundtrip}; fn invoke_custom(e: &TestEnv, id: &str, func: &str) -> assert_cmd::Command { let mut s = e.new_assert_cmd("contract"); @@ -40,7 +40,9 @@ async fn parse() { negative_i32(sandbox, id).await; negative_i64(sandbox, id).await; account_address(sandbox, id).await; + account_address_with_alias(sandbox, id).await; contract_address(sandbox, id).await; + contract_address_with_alias(sandbox, id).await; bytes(sandbox, id).await; const_enum(sandbox, id).await; number_arg_return_ok(sandbox, id); @@ -237,6 +239,12 @@ async fn account_address(sandbox: &TestEnv, id: &str) { .await; } +async fn account_address_with_alias(sandbox: &TestEnv, id: &str) { + let res = invoke(sandbox, id, "addresse", &json!("test").to_string()).await; + let test = format!("\"{}\"", super::tx::operations::test_address(sandbox)); + assert_eq!(test, res); +} + async fn contract_address(sandbox: &TestEnv, id: &str) { invoke_with_roundtrip( sandbox, @@ -247,6 +255,22 @@ async fn contract_address(sandbox: &TestEnv, id: &str) { .await; } +async fn contract_address_with_alias(sandbox: &TestEnv, id: &str) { + sandbox + .new_assert_cmd("contract") + .arg("alias") + .arg("add") + .arg("test_contract") + .arg("--id=CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE") + .assert() + .success(); + let res = invoke(sandbox, id, "addresse", &json!("test_contract").to_string()).await; + assert_eq!( + res, + "\"CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE\"" + ); +} + async fn bytes(sandbox: &TestEnv, id: &str) { invoke_with_roundtrip(sandbox, id, "bytes", json!("7374656c6c6172")).await; } diff --git a/cmd/crates/soroban-test/tests/it/integration/tx.rs b/cmd/crates/soroban-test/tests/it/integration/tx.rs index c3cd2693b..3fa85bc09 100644 --- a/cmd/crates/soroban-test/tests/it/integration/tx.rs +++ b/cmd/crates/soroban-test/tests/it/integration/tx.rs @@ -4,7 +4,7 @@ use soroban_test::{AssertExt, TestEnv}; use crate::integration::util::{deploy_contract, DeployKind, HELLO_WORLD}; -mod operations; +pub mod operations; #[tokio::test] async fn simulate() { diff --git a/cmd/crates/soroban-test/tests/it/integration/tx/operations.rs b/cmd/crates/soroban-test/tests/it/integration/tx/operations.rs index d9bc2ac63..8c894e5dc 100644 --- a/cmd/crates/soroban-test/tests/it/integration/tx/operations.rs +++ b/cmd/crates/soroban-test/tests/it/integration/tx/operations.rs @@ -11,7 +11,7 @@ use crate::integration::{ util::{deploy_contract, DeployKind, HELLO_WORLD}, }; -fn test_address(sandbox: &TestEnv) -> String { +pub fn test_address(sandbox: &TestEnv) -> String { sandbox .new_assert_cmd("keys") .arg("address") diff --git a/cmd/crates/soroban-test/tests/it/integration/util.rs b/cmd/crates/soroban-test/tests/it/integration/util.rs index 486b00a1b..fc7f824b6 100644 --- a/cmd/crates/soroban-test/tests/it/integration/util.rs +++ b/cmd/crates/soroban-test/tests/it/integration/util.rs @@ -11,16 +11,19 @@ pub const CUSTOM_TYPES: &Wasm = &Wasm::Custom("test-wasms", "test_custom_types") pub const CUSTOM_ACCOUNT: &Wasm = &Wasm::Custom("test-wasms", "test_custom_account"); pub const SWAP: &Wasm = &Wasm::Custom("test-wasms", "test_swap"); +pub async fn invoke(sandbox: &TestEnv, id: &str, func: &str, data: &str) -> String { + sandbox + .invoke_with_test(&["--id", id, "--", func, &format!("--{func}"), data]) + .await + .unwrap() +} pub async fn invoke_with_roundtrip(e: &TestEnv, id: &str, func: &str, data: D) where D: Display, { let data = data.to_string(); println!("{data}"); - let res = e - .invoke_with_test(&["--id", id, "--", func, &format!("--{func}"), &data]) - .await - .unwrap(); + let res = invoke(e, id, func, &data).await; assert_eq!(res, data); } diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index 21fa2f383..a223851ca 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -9,12 +9,14 @@ use ed25519_dalek::SigningKey; use heck::ToKebabCase; use crate::xdr::{ - self, Hash, InvokeContractArgs, ScAddress, ScSpecEntry, ScSpecFunctionV0, ScSpecTypeDef, ScVal, - ScVec, + self, Hash, InvokeContractArgs, ScSpecEntry, ScSpecFunctionV0, ScSpecTypeDef, ScVal, ScVec, }; use crate::commands::txn_result::TxnResult; -use crate::config::{self}; +use crate::config::{ + self, + sc_address::{self, UnresolvedScAddress}, +}; use soroban_spec_tools::Spec; #[derive(thiserror::Error, Debug)] @@ -43,6 +45,10 @@ pub enum Error { MissingArgument(String), #[error("")] MissingFileArg(PathBuf), + #[error(transparent)] + ScAddress(#[from] sc_address::Error), + #[error(transparent)] + Config(#[from] config::Error), } pub fn build_host_function_parameters( @@ -80,18 +86,18 @@ pub fn build_host_function_parameters( .map(|i| { let name = i.name.to_utf8_string()?; if let Some(mut val) = matches_.get_raw(&name) { - let mut s = val.next().unwrap().to_string_lossy().to_string(); + let mut s = val + .next() + .unwrap() + .to_string_lossy() + .trim_matches('"') + .to_string(); if matches!(i.type_, ScSpecTypeDef::Address) { - let cmd = crate::commands::keys::address::Cmd { - name: s.clone(), - hd_path: Some(0), - locator: config.locator.clone(), - }; - if let Ok(address) = cmd.public_key() { - s = address.to_string(); - } - if let Ok(key) = cmd.private_key() { - signers.push(key); + let addr = resolve_address(&s, config)?; + let signer = resolve_signer(&s, config); + s = addr; + if let Some(signer) = signer { + signers.push(signer); } } spec.from_string(&s, &i.type_) @@ -125,7 +131,7 @@ pub fn build_host_function_parameters( }) .collect::, Error>>()?; - let contract_address_arg = ScAddress::Contract(Hash(contract_id.0)); + let contract_address_arg = xdr::ScAddress::Contract(Hash(contract_id.0)); let function_symbol_arg = function .try_into() .map_err(|()| Error::FunctionNameTooLong(function.clone()))?; @@ -246,3 +252,27 @@ pub fn output_to_string( } Ok(TxnResult::Res(res_str)) } + +fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result { + let sc_address: UnresolvedScAddress = addr_or_alias.parse().unwrap(); + let account = match sc_address { + UnresolvedScAddress::Resolved(addr) => addr.to_string(), + addr @ UnresolvedScAddress::Alias(_) => { + let addr = addr.resolve(&config.locator, &config.get_network()?.network_passphrase)?; + match addr { + xdr::ScAddress::Account(account) => account.to_string(), + contract @ xdr::ScAddress::Contract(_) => contract.to_string(), + } + } + }; + Ok(account) +} + +fn resolve_signer(addr_or_alias: &str, config: &config::Args) -> Option { + let cmd = crate::commands::keys::address::Cmd { + name: addr_or_alias.to_string(), + hd_path: Some(0), + locator: config.locator.clone(), + }; + cmd.private_key().ok() +} diff --git a/cmd/soroban-cli/src/commands/contract/fetch.rs b/cmd/soroban-cli/src/commands/contract/fetch.rs index 31ed191ff..d73aac3b7 100644 --- a/cmd/soroban-cli/src/commands/contract/fetch.rs +++ b/cmd/soroban-cli/src/commands/contract/fetch.rs @@ -22,7 +22,7 @@ use crate::{ pub struct Cmd { /// Contract ID to fetch #[arg(long = "id", env = "STELLAR_CONTRACT_ID")] - pub contract_id: config::ContractAddress, + pub contract_id: config::UnresolvedContract, /// Where to write output otherwise stdout is used #[arg(long, short = 'o')] pub out_file: Option, diff --git a/cmd/soroban-cli/src/commands/contract/info/shared.rs b/cmd/soroban-cli/src/commands/contract/info/shared.rs index 6023b03cf..13355268f 100644 --- a/cmd/soroban-cli/src/commands/contract/info/shared.rs +++ b/cmd/soroban-cli/src/commands/contract/info/shared.rs @@ -47,7 +47,7 @@ pub struct Args { conflicts_with = "wasm", conflicts_with = "wasm_hash" )] - pub contract_id: Option, + pub contract_id: Option, #[command(flatten)] pub network: network::Args, #[command(flatten)] diff --git a/cmd/soroban-cli/src/commands/contract/invoke.rs b/cmd/soroban-cli/src/commands/contract/invoke.rs index c7b631343..bd069698d 100644 --- a/cmd/soroban-cli/src/commands/contract/invoke.rs +++ b/cmd/soroban-cli/src/commands/contract/invoke.rs @@ -40,7 +40,7 @@ use soroban_spec_tools::contract; pub struct Cmd { /// Contract ID to invoke #[arg(long = "id", env = "STELLAR_CONTRACT_ID")] - pub contract_id: config::ContractAddress, + pub contract_id: config::UnresolvedContract, // For testing only #[arg(skip)] pub wasm: Option, diff --git a/cmd/soroban-cli/src/commands/events.rs b/cmd/soroban-cli/src/commands/events.rs index 48d79c1b7..16ef410bd 100644 --- a/cmd/soroban-cli/src/commands/events.rs +++ b/cmd/soroban-cli/src/commands/events.rs @@ -42,7 +42,7 @@ pub struct Cmd { num_args = 1..=6, help_heading = "FILTERS" )] - contract_ids: Vec, + contract_ids: Vec, /// A set of (up to 4) topic filters to filter event topics on. A single /// topic filter can contain 1-4 different segment filters, separated by /// commas, with an asterisk (`*` character) indicating a wildcard segment. diff --git a/cmd/soroban-cli/src/commands/snapshot/create.rs b/cmd/soroban-cli/src/commands/snapshot/create.rs index a3ba865fa..9ad39953f 100644 --- a/cmd/soroban-cli/src/commands/snapshot/create.rs +++ b/cmd/soroban-cli/src/commands/snapshot/create.rs @@ -34,7 +34,7 @@ use crate::{ tx::builder, utils::get_name_from_stellar_asset_contract_storage, }; -use crate::{config::address::Address, utils::http}; +use crate::{config::address::UnresolvedMuxedAccount, utils::http}; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, ValueEnum)] pub enum Output { @@ -413,7 +413,7 @@ impl Cmd { // Resolve an account address to an account id. The address can be a // G-address or a key name (as in `stellar keys address NAME`). fn resolve_account(&self, address: &str) -> Option { - let address: Address = address.parse().ok()?; + let address: UnresolvedMuxedAccount = address.parse().ok()?; Some(AccountId(xdr::PublicKey::PublicKeyTypeEd25519( match address.resolve_muxed_account(&self.locator, None).ok()? { diff --git a/cmd/soroban-cli/src/commands/tx/op/add/args.rs b/cmd/soroban-cli/src/commands/tx/op/add/args.rs index f1858e0b0..51ee4d476 100644 --- a/cmd/soroban-cli/src/commands/tx/op/add/args.rs +++ b/cmd/soroban-cli/src/commands/tx/op/add/args.rs @@ -23,7 +23,7 @@ pub struct Args { visible_alias = "op-source", env = "STELLAR_OPERATION_SOURCE_ACCOUNT" )] - pub operation_source_account: Option, + pub operation_source_account: Option, } impl Args { diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 066bc8d91..356c7a991 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -6,14 +6,14 @@ use super::{locator, secret}; /// Address can be either a public key or eventually an alias of a address. #[derive(Clone, Debug)] -pub enum Address { - MuxedAccount(xdr::MuxedAccount), +pub enum UnresolvedMuxedAccount { + Resolved(xdr::MuxedAccount), AliasOrSecret(String), } -impl Default for Address { +impl Default for UnresolvedMuxedAccount { fn default() -> Self { - Address::AliasOrSecret(String::default()) + UnresolvedMuxedAccount::AliasOrSecret(String::default()) } } @@ -27,37 +27,49 @@ pub enum Error { CannotSign(xdr::MuxedAccount), } -impl FromStr for Address { +impl FromStr for UnresolvedMuxedAccount { type Err = Error; fn from_str(value: &str) -> Result { Ok(xdr::MuxedAccount::from_str(value).map_or_else( - |_| Address::AliasOrSecret(value.to_string()), - Address::MuxedAccount, + |_| UnresolvedMuxedAccount::AliasOrSecret(value.to_string()), + UnresolvedMuxedAccount::Resolved, )) } } -impl Address { +impl UnresolvedMuxedAccount { pub fn resolve_muxed_account( &self, locator: &locator::Args, hd_path: Option, ) -> Result { match self { - Address::MuxedAccount(muxed_account) => Ok(muxed_account.clone()), - Address::AliasOrSecret(alias) => alias.parse().or_else(|_| { - Ok(xdr::MuxedAccount::Ed25519( - locator.read_identity(alias)?.public_key(hd_path)?.0.into(), - )) - }), + UnresolvedMuxedAccount::Resolved(muxed_account) => Ok(muxed_account.clone()), + UnresolvedMuxedAccount::AliasOrSecret(alias) => { + Self::resolve_muxed_account_with_alias(alias, locator, hd_path) + } } } + pub fn resolve_muxed_account_with_alias( + alias: &str, + locator: &locator::Args, + hd_path: Option, + ) -> Result { + alias.parse().or_else(|_| { + Ok(xdr::MuxedAccount::Ed25519( + locator.read_identity(alias)?.public_key(hd_path)?.0.into(), + )) + }) + } + pub fn resolve_secret(&self, locator: &locator::Args) -> Result { match &self { - Address::MuxedAccount(muxed_account) => Err(Error::CannotSign(muxed_account.clone())), - Address::AliasOrSecret(alias) => Ok(locator.read_identity(alias)?), + UnresolvedMuxedAccount::Resolved(muxed_account) => { + Err(Error::CannotSign(muxed_account.clone())) + } + UnresolvedMuxedAccount::AliasOrSecret(alias) => Ok(locator.read_identity(alias)?), } } } diff --git a/cmd/soroban-cli/src/config/alias.rs b/cmd/soroban-cli/src/config/alias.rs index 9d1d8c11b..734925c4e 100644 --- a/cmd/soroban-cli/src/config/alias.rs +++ b/cmd/soroban-cli/src/config/alias.rs @@ -11,39 +11,49 @@ pub struct Data { /// Address can be either a contract address, C.. or eventually an alias of a contract address. #[derive(Clone, Debug)] -pub enum ContractAddress { - ContractId(stellar_strkey::Contract), +pub enum UnresolvedContract { + Resolved(stellar_strkey::Contract), Alias(String), } -impl Default for ContractAddress { +impl Default for UnresolvedContract { fn default() -> Self { - ContractAddress::Alias(String::default()) + UnresolvedContract::Alias(String::default()) } } -impl FromStr for ContractAddress { +impl FromStr for UnresolvedContract { type Err = Infallible; fn from_str(value: &str) -> Result { Ok(stellar_strkey::Contract::from_str(value).map_or_else( - |_| ContractAddress::Alias(value.to_string()), - ContractAddress::ContractId, + |_| UnresolvedContract::Alias(value.to_string()), + UnresolvedContract::Resolved, )) } } -impl ContractAddress { +impl UnresolvedContract { pub fn resolve_contract_id( &self, locator: &locator::Args, network_passphrase: &str, ) -> Result { match self { - ContractAddress::ContractId(muxed_account) => Ok(*muxed_account), - ContractAddress::Alias(alias) => locator - .get_contract_id(alias, network_passphrase)? - .ok_or_else(|| locator::Error::ContractNotFound(alias.to_owned())), + UnresolvedContract::Resolved(contract) => Ok(*contract), + UnresolvedContract::Alias(alias) => { + Self::resolve_alias(alias, locator, network_passphrase) + } } } + + pub fn resolve_alias( + alias: &str, + locator: &locator::Args, + network_passphrase: &str, + ) -> Result { + locator + .get_contract_id(alias, network_passphrase)? + .ok_or_else(|| locator::Error::ContractNotFound(alias.to_owned())) + } } diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index 7e97f6796..1e26bbf7f 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -83,6 +83,10 @@ pub enum Error { UpgradeCheckReadFailed { path: PathBuf, error: io::Error }, #[error("Failed to write upgrade check file: {path}: {error}")] UpgradeCheckWriteFailed { path: PathBuf, error: io::Error }, + #[error("Contract alias {0}, cannot overlap with key")] + ContractAliasCannotOverlapWithKey(String), + #[error("Key cannot {0} cannot overlap with contract alias")] + KeyCannotOverlapWithContractAlias(String), } #[derive(Debug, clap::Args, Default, Clone)] @@ -163,6 +167,9 @@ impl Args { } pub fn write_identity(&self, name: &str, secret: &Secret) -> Result { + if let Ok(Some(_)) = self.load_contract_from_alias(name) { + return Err(Error::KeyCannotOverlapWithContractAlias(name.to_owned())); + } KeyType::Identity.write(name, secret, &self.config_dir()?) } @@ -286,6 +293,9 @@ impl Args { contract_id: &stellar_strkey::Contract, alias: &str, ) -> Result<(), Error> { + if self.read_identity(alias).is_ok() { + return Err(Error::ContractAliasCannotOverlapWithKey(alias.to_owned())); + } let path = self.alias_path(alias)?; let dir = path.parent().ok_or(Error::CannotAccessConfigDir)?; diff --git a/cmd/soroban-cli/src/config/mod.rs b/cmd/soroban-cli/src/config/mod.rs index a429ff434..1188d3bfa 100644 --- a/cmd/soroban-cli/src/config/mod.rs +++ b/cmd/soroban-cli/src/config/mod.rs @@ -1,4 +1,3 @@ -use address::Address; use clap::{arg, command}; use serde::{Deserialize, Serialize}; use std::{ @@ -19,11 +18,14 @@ pub mod alias; pub mod data; pub mod locator; pub mod network; +pub mod sc_address; pub mod secret; pub mod sign_with; pub mod upgrade_check; -pub use alias::ContractAddress; +pub use address::UnresolvedMuxedAccount; +pub use alias::UnresolvedContract; +pub use sc_address::UnresolvedScAddress; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -56,7 +58,7 @@ pub struct Args { /// or a seed phrase (--source "kite urban…"). /// If `--build-only` or `--sim-only` flags were NOT provided, this key will also be used to /// sign the final transaction. In that case, trying to sign with public key will fail. - pub source_account: Address, + pub source_account: UnresolvedMuxedAccount, #[arg(long)] /// If using a seed phrase, which hierarchical deterministic path to use, e.g. `m/44'/148'/{hd_path}`. Example: `--hd-path 1`. Default: `0` diff --git a/cmd/soroban-cli/src/config/sc_address.rs b/cmd/soroban-cli/src/config/sc_address.rs new file mode 100644 index 000000000..fc9c168f2 --- /dev/null +++ b/cmd/soroban-cli/src/config/sc_address.rs @@ -0,0 +1,67 @@ +use std::str::FromStr; + +use crate::xdr; + +use super::{address, locator, UnresolvedContract}; + +/// `ScAddress` can be either a resolved `xdr::ScAddress` or an alias of a `Contract` or `MuxedAccount`. +#[allow(clippy::module_name_repetitions)] +#[derive(Clone, Debug)] +pub enum UnresolvedScAddress { + Resolved(xdr::ScAddress), + Alias(String), +} + +impl Default for UnresolvedScAddress { + fn default() -> Self { + UnresolvedScAddress::Alias(String::default()) + } +} + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error(transparent)] + Locator(#[from] locator::Error), + #[error(transparent)] + Address(#[from] address::Error), + #[error("Account alias not Found{0}")] + AccountAliasNotFound(String), +} + +impl FromStr for UnresolvedScAddress { + type Err = Error; + + fn from_str(value: &str) -> Result { + Ok(xdr::ScAddress::from_str(value).map_or_else( + |_| UnresolvedScAddress::Alias(value.to_string()), + UnresolvedScAddress::Resolved, + )) + } +} + +impl UnresolvedScAddress { + pub fn resolve( + self, + locator: &locator::Args, + network_passphrase: &str, + ) -> Result { + let alias = match self { + UnresolvedScAddress::Resolved(addr) => return Ok(addr), + UnresolvedScAddress::Alias(alias) => alias, + }; + let contract = UnresolvedContract::resolve_alias(&alias, locator, network_passphrase); + let muxed_account = + super::UnresolvedMuxedAccount::resolve_muxed_account_with_alias(&alias, locator, None); + match (contract, muxed_account) { + (Ok(contract), Ok(_)) => { + eprintln!( + "Warning: ScAddress alias {alias} is ambiguous, assuming it is a contract" + ); + Ok(xdr::ScAddress::Contract(xdr::Hash(contract.0))) + } + (Ok(contract), _) => Ok(xdr::ScAddress::Contract(xdr::Hash(contract.0))), + (_, Ok(muxed_account)) => Ok(xdr::ScAddress::Account(muxed_account.account_id())), + _ => Err(Error::AccountAliasNotFound(alias)), + } + } +} diff --git a/cmd/soroban-cli/src/key.rs b/cmd/soroban-cli/src/key.rs index b704541c4..c3fd7ed89 100644 --- a/cmd/soroban-cli/src/key.rs +++ b/cmd/soroban-cli/src/key.rs @@ -34,7 +34,7 @@ pub struct Args { required_unless_present = "wasm", required_unless_present = "wasm_hash" )] - pub contract_id: Option, + pub contract_id: Option, /// Storage key (symbols only) #[arg(long = "key", conflicts_with = "key_xdr")] pub key: Option>, From 24269ce1db89b88ace0bff395e0f0d105606f106 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Fri, 20 Dec 2024 16:34:55 -0500 Subject: [PATCH 35/42] Fix after merging with main --- cmd/soroban-cli/src/commands/keys/address.rs | 28 +++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/address.rs b/cmd/soroban-cli/src/commands/keys/address.rs index 145d8c30a..aa5a2940d 100644 --- a/cmd/soroban-cli/src/commands/keys/address.rs +++ b/cmd/soroban-cli/src/commands/keys/address.rs @@ -1,9 +1,6 @@ use clap::arg; -use crate::{ - commands::config::{address, locator, secret}, - xdr, -}; +use crate::commands::config::{address, locator, secret}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -24,7 +21,7 @@ pub enum Error { #[group(skip)] pub struct Cmd { /// Name of identity to lookup, default test identity used if not provided - pub name: address::Address, + pub name: String, /// If identity is a seed phrase use this hd path, default is 0 #[arg(long)] @@ -40,15 +37,20 @@ impl Cmd { Ok(()) } + pub fn private_key(&self) -> Result { + Ok(self + .locator + .read_identity(&self.name)? + .key_pair(self.hd_path)?) + } + pub fn public_key(&self) -> Result { - match self - .name - .resolve_muxed_account(&self.locator, self.hd_path)? - { - xdr::MuxedAccount::Ed25519(pk) => Ok(stellar_strkey::ed25519::PublicKey(pk.0)), - xdr::MuxedAccount::MuxedEd25519(xdr::MuxedAccountMed25519 { ed25519, .. }) => { - Ok(stellar_strkey::ed25519::PublicKey(ed25519.0)) - } + if let Ok(key) = stellar_strkey::ed25519::PublicKey::from_string(&self.name) { + Ok(key) + } else { + Ok(stellar_strkey::ed25519::PublicKey::from_payload( + self.private_key()?.verifying_key().as_bytes(), + )?) } } } From e95fa3a38b53cfb09045f70de0adc6c91c09773d Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Fri, 20 Dec 2024 16:59:25 -0500 Subject: [PATCH 36/42] Apply suggestion from code review Co-authored-by: Willem Wyndham --- cmd/soroban-cli/src/signer/keyring.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index b5a63a398..4e6b19c99 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -31,7 +31,7 @@ impl StellarEntry { Ok(()) } - pub fn get_password(&self) -> Result, Error> { + fn get_password(&self) -> Result, Error> { Ok(base64.decode(self.keyring.get_password()?)?) } From 95d8b329a233b0fb7be7e74cb1daebe47b2c78d6 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 23 Dec 2024 09:47:44 -0500 Subject: [PATCH 37/42] Limit key name length --- cmd/soroban-cli/src/config/address.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 36cc90f42..dadd8853f 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -28,7 +28,11 @@ pub enum Error { Secret(#[from] secret::Error), #[error("Address cannot be used to sign {0}")] CannotSign(xdr::MuxedAccount), - #[error("Invalid key name: {0}\n only alphanumeric characters, `_`and `-` are allowed")] + #[error("Invalid key name: {0}\n only alphanumeric characters, underscores (_), and hyphens (-) are allowed.")] + InvalidKeyNameCharacters(String), + #[error("Invalid key name: {0}\n keys cannot exceed 250 characters")] + InvalidKeyNameLength(String), + #[error("Invalid key name: {0}\n keys cannot be the word \"ledger\"")] InvalidKeyName(String), } @@ -93,11 +97,14 @@ impl std::str::FromStr for KeyName { type Err = Error; fn from_str(s: &str) -> Result { if !s.chars().all(allowed_char) { - return Err(Error::InvalidKeyName(s.to_string())); + return Err(Error::InvalidKeyNameCharacters(s.to_string())); } if s == "ledger" { return Err(Error::InvalidKeyName(s.to_string())); } + if s.len() > 250 { + return Err(Error::InvalidKeyNameLength(s.to_string())); + } Ok(KeyName(s.to_string())) } } From 01c87538c6e9c79ad32737bead38a82e7591bf68 Mon Sep 17 00:00:00 2001 From: Elizabeth Engelman <4752801+elizabethengelman@users.noreply.github.com> Date: Mon, 23 Dec 2024 12:06:36 -0500 Subject: [PATCH 38/42] Update public_key to work with secure storage keys --- cmd/soroban-cli/src/commands/keys/address.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/soroban-cli/src/commands/keys/address.rs b/cmd/soroban-cli/src/commands/keys/address.rs index aa5a2940d..1a7ab21cd 100644 --- a/cmd/soroban-cli/src/commands/keys/address.rs +++ b/cmd/soroban-cli/src/commands/keys/address.rs @@ -1,6 +1,9 @@ use clap::arg; -use crate::commands::config::{address, locator, secret}; +use crate::{ + commands::config::{address, locator, secret}, + config::UnresolvedMuxedAccount, +}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -47,6 +50,11 @@ impl Cmd { pub fn public_key(&self) -> Result { if let Ok(key) = stellar_strkey::ed25519::PublicKey::from_string(&self.name) { Ok(key) + } else if let Ok(unresolved) = self.name.parse::() { + let muxed = unresolved.resolve_muxed_account(&self.locator, self.hd_path)?; + Ok(stellar_strkey::ed25519::PublicKey::from_string( + &muxed.to_string(), + )?) } else { Ok(stellar_strkey::ed25519::PublicKey::from_payload( self.private_key()?.verifying_key().as_bytes(), From ec01a79e070c8063d7726c8fb2e47c24fc4a6a9d Mon Sep 17 00:00:00 2001 From: Gleb Date: Fri, 3 Jan 2025 14:33:35 -0800 Subject: [PATCH 39/42] Fix TS tests (#1820) --- cmd/crates/soroban-spec-typescript/ts-tests/src/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/crates/soroban-spec-typescript/ts-tests/src/util.ts b/cmd/crates/soroban-spec-typescript/ts-tests/src/util.ts index eacedbaec..47504c368 100644 --- a/cmd/crates/soroban-spec-typescript/ts-tests/src/util.ts +++ b/cmd/crates/soroban-spec-typescript/ts-tests/src/util.ts @@ -3,7 +3,7 @@ import { Address, Keypair } from "@stellar/stellar-sdk"; import { basicNodeSigner } from "@stellar/stellar-sdk/contract"; const rootKeypair = Keypair.fromSecret( - spawnSync("./soroban", ["keys", "secret", "root"], { + spawnSync("./stellar", ["keys", "secret", "root"], { shell: true, encoding: "utf8", }).stdout.trim(), From 085317fdbfdaf9ca91702d2466ab46128768cfea Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Mon, 6 Jan 2025 16:31:37 -0500 Subject: [PATCH 40/42] fix(address): remove private key function & use unresolved Address This simplifies the lookup of the address. --- .../src/commands/contract/arg_parsing.rs | 7 +---- cmd/soroban-cli/src/commands/keys/address.rs | 29 ++++++------------- cmd/soroban-cli/src/config/address.rs | 2 +- 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index b522fc91e..98a6d3004 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -274,10 +274,5 @@ fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result Option { - let cmd = crate::commands::keys::address::Cmd { - name: addr_or_alias.to_string(), - hd_path: Some(0), - locator: config.locator.clone(), - }; - cmd.private_key().ok() + config.locator.key(addr_or_alias).ok()?.key_pair(None).ok() } diff --git a/cmd/soroban-cli/src/commands/keys/address.rs b/cmd/soroban-cli/src/commands/keys/address.rs index 1a7ab21cd..9727988e0 100644 --- a/cmd/soroban-cli/src/commands/keys/address.rs +++ b/cmd/soroban-cli/src/commands/keys/address.rs @@ -24,7 +24,7 @@ pub enum Error { #[group(skip)] pub struct Cmd { /// Name of identity to lookup, default test identity used if not provided - pub name: String, + pub name: UnresolvedMuxedAccount, /// If identity is a seed phrase use this hd path, default is 0 #[arg(long)] @@ -40,25 +40,14 @@ impl Cmd { Ok(()) } - pub fn private_key(&self) -> Result { - Ok(self - .locator - .read_identity(&self.name)? - .key_pair(self.hd_path)?) - } - pub fn public_key(&self) -> Result { - if let Ok(key) = stellar_strkey::ed25519::PublicKey::from_string(&self.name) { - Ok(key) - } else if let Ok(unresolved) = self.name.parse::() { - let muxed = unresolved.resolve_muxed_account(&self.locator, self.hd_path)?; - Ok(stellar_strkey::ed25519::PublicKey::from_string( - &muxed.to_string(), - )?) - } else { - Ok(stellar_strkey::ed25519::PublicKey::from_payload( - self.private_key()?.verifying_key().as_bytes(), - )?) - } + let muxed = self + .name + .resolve_muxed_account(&self.locator, self.hd_path)?; + let bytes = match muxed { + soroban_sdk::xdr::MuxedAccount::Ed25519(uint256) => uint256.0, + soroban_sdk::xdr::MuxedAccount::MuxedEd25519(muxed_account) => muxed_account.ed25519.0, + }; + Ok(stellar_strkey::ed25519::PublicKey(bytes)) } } diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index dadd8853f..24a40a62a 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -78,7 +78,7 @@ impl UnresolvedMuxedAccount { UnresolvedMuxedAccount::Resolved(muxed_account) => { Err(Error::CannotSign(muxed_account.clone())) } - UnresolvedMuxedAccount::AliasOrSecret(alias) => Ok(locator.read_identity(alias)?), + UnresolvedMuxedAccount::AliasOrSecret(alias) => Ok(locator.key(alias)?), } } } From aecc5ac4c40f6f52c15f51fb4947952aa8b2d597 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Tue, 7 Jan 2025 14:18:45 -0500 Subject: [PATCH 41/42] feat: store seedphrase instead of private key This will allow for exporting the phrase later --- cmd/soroban-cli/src/commands/keys/generate.rs | 28 ++--- cmd/soroban-cli/src/config/secret.rs | 38 +++++-- cmd/soroban-cli/src/signer.rs | 5 +- cmd/soroban-cli/src/signer/keyring.rs | 107 ++++++++++-------- 4 files changed, 106 insertions(+), 72 deletions(-) diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index 1e3727478..8ec0158bb 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -1,4 +1,5 @@ use clap::{arg, command}; +use sep5::SeedPhrase; use super::super::config::{ locator, network, @@ -122,9 +123,7 @@ impl Cmd { fn secret(&self, print: &Print) -> Result { let seed_phrase = self.seed_phrase()?; - Ok(if self.as_secret { - seed_phrase.private_key(self.hd_path)?.into() - } else if self.secure_store { + if self.secure_store { // secure_store:org.stellar.cli: let entry_name_with_prefix = format!( "{}{}-{}", @@ -137,38 +136,41 @@ impl Cmd { let secret: Secret = entry_name_with_prefix.parse()?; if let Secret::SecureStore { entry_name } = &secret { - Self::write_to_secure_store(entry_name, &seed_phrase, print)?; + Self::write_to_secure_store(entry_name, seed_phrase, print)?; } - secret + return Ok(secret); + } + let secret: Secret = seed_phrase.into(); + Ok(if self.as_secret { + secret.private_key(self.hd_path)?.into() } else { - seed_phrase + secret }) } - fn seed_phrase(&self) -> Result { + fn seed_phrase(&self) -> Result { Ok(if self.default_seed { - Secret::test_seed_phrase() + secret::test_seed_phrase() } else { - Secret::from_seed(self.seed.as_deref()) + secret::seed_phrase_from_seed(self.seed.as_deref()) }?) } fn write_to_secure_store( entry_name: &String, - seed_phrase: &Secret, + seed_phrase: SeedPhrase, print: &Print, ) -> Result<(), Error> { print.infoln(format!("Writing to secure store: {entry_name}")); let entry = StellarEntry::new(entry_name)?; - if let Ok(key) = entry.get_public_key() { + if let Ok(key) = entry.get_public_key(None) { print.warnln(format!("A key for {entry_name} already exists in your operating system's secure store: {key}")); } else { print.infoln(format!( "Saving a new key to your operating system's secure store: {entry_name}" )); - let key_pair = seed_phrase.key_pair(None)?; - entry.set_password(key_pair.as_bytes())?; + entry.set_seed_phrase(seed_phrase)?; } Ok(()) } diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index 8f4b20c60..f32b291e8 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -1,6 +1,8 @@ use clap::arg; use serde::{Deserialize, Serialize}; use std::{io::Write, str::FromStr}; + +use sep5::SeedPhrase; use stellar_strkey::ed25519::{PrivateKey, PublicKey}; use crate::{ @@ -94,6 +96,14 @@ impl From for Secret { } } +impl From for Secret { + fn from(value: SeedPhrase) -> Self { + Secret::SeedPhrase { + seed_phrase: value.seed_phrase.into_phrase(), + } + } +} + impl Secret { pub fn private_key(&self, index: Option) -> Result { Ok(match self { @@ -113,7 +123,7 @@ impl Secret { pub fn public_key(&self, index: Option) -> Result { if let Secret::SecureStore { entry_name } = self { let entry = keyring::StellarEntry::new(entry_name)?; - Ok(entry.get_public_key()?) + Ok(entry.get_public_key(index)?) } else { let key = self.key_pair(index)?; Ok(stellar_strkey::ed25519::PublicKey::from_payload( @@ -122,14 +132,15 @@ impl Secret { } } - pub fn signer(&self, index: Option, print: Print) -> Result { + pub fn signer(&self, hd_path: Option, print: Print) -> Result { let kind = match self { Secret::SecretKey { .. } | Secret::SeedPhrase { .. } => { - let key = self.key_pair(index)?; + let key = self.key_pair(hd_path)?; SignerKind::Local(LocalKey { key }) } Secret::SecureStore { entry_name } => SignerKind::SecureStore(SecureStoreEntry { name: entry_name.to_string(), + hd_path, }), }; Ok(Signer { kind, print }) @@ -140,14 +151,7 @@ impl Secret { } pub fn from_seed(seed: Option<&str>) -> Result { - let seed_phrase = if let Some(seed) = seed.map(str::as_bytes) { - sep5::SeedPhrase::from_entropy(seed) - } else { - sep5::SeedPhrase::random(sep5::MnemonicType::Words24) - }? - .seed_phrase - .into_phrase(); - Ok(Secret::SeedPhrase { seed_phrase }) + Ok(seed_phrase_from_seed(seed)?.into()) } pub fn test_seed_phrase() -> Result { @@ -155,6 +159,18 @@ impl Secret { } } +pub fn seed_phrase_from_seed(seed: Option<&str>) -> Result { + Ok(if let Some(seed) = seed.map(str::as_bytes) { + sep5::SeedPhrase::from_entropy(seed)? + } else { + sep5::SeedPhrase::random(sep5::MnemonicType::Words24)? + }) +} + +pub fn test_seed_phrase() -> Result { + Ok("0000000000000000".parse()?) +} + fn read_password() -> Result { std::io::stdout().flush().map_err(|_| Error::PasswordRead)?; rpassword::read_password().map_err(|_| Error::PasswordRead) diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index 099ed6186..ebd650d40 100644 --- a/cmd/soroban-cli/src/signer.rs +++ b/cmd/soroban-cli/src/signer.rs @@ -294,13 +294,14 @@ impl Lab { pub struct SecureStoreEntry { pub name: String, + pub hd_path: Option, } impl SecureStoreEntry { pub fn sign_tx_hash(&self, tx_hash: [u8; 32]) -> Result { let entry = StellarEntry::new(&self.name)?; - let hint = SignatureHint(entry.get_public_key()?.0[28..].try_into()?); - let signed_tx_hash = entry.sign_data(&tx_hash)?; + let hint = SignatureHint(entry.get_public_key(self.hd_path)?.0[28..].try_into()?); + let signed_tx_hash = entry.sign_data(&tx_hash, self.hd_path)?; let signature = Signature(signed_tx_hash.clone().try_into()?); Ok(DecoratedSignature { hint, signature }) } diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index 4e6b19c99..0e6c49137 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -1,6 +1,6 @@ -use base64::{engine::general_purpose::STANDARD as base64, Engine as _}; use ed25519_dalek::Signer; use keyring::Entry; +use sep5::seed_phrase::SeedPhrase; use zeroize::Zeroize; pub(crate) const SECURE_STORE_ENTRY_PREFIX: &str = "secure_store:"; @@ -11,7 +11,7 @@ pub enum Error { #[error(transparent)] Keyring(#[from] keyring::Error), #[error(transparent)] - Base64(#[from] base64::DecodeError), + Sep5(#[from] sep5::error::Error), } pub struct StellarEntry { @@ -25,46 +25,60 @@ impl StellarEntry { }) } - pub fn set_password(&self, password: &[u8]) -> Result<(), Error> { - let data = base64.encode(password); + pub fn set_seed_phrase(&self, seed_phrase: SeedPhrase) -> Result<(), Error> { + let mut data = seed_phrase.seed_phrase.into_phrase(); self.keyring.set_password(&data)?; + data.zeroize(); Ok(()) } - fn get_password(&self) -> Result, Error> { - Ok(base64.decode(self.keyring.get_password()?)?) + fn get_seed_phrase(&self) -> Result { + Ok(self.keyring.get_password()?.parse()?) } fn use_key( &self, f: impl FnOnce(ed25519_dalek::SigningKey) -> Result, + hd_path: Option, ) -> Result { - let mut key_vec = self.get_password()?; - let mut key_bytes: [u8; 32] = key_vec.as_slice().try_into().unwrap(); - + // The underlying Mnemonic type is zeroized when dropped + let mut key_bytes: [u8; 32] = { + self.get_seed_phrase()? + .from_path_index(hd_path.unwrap_or_default(), None)? + .private() + .0 + }; let result = { - // Use this scope to ensure the keypair is zeroized + // Use this scope to ensure the keypair is zeroized when dropped let keypair = ed25519_dalek::SigningKey::from_bytes(&key_bytes); f(keypair)? }; - key_vec.zeroize(); key_bytes.zeroize(); Ok(result) } - pub fn get_public_key(&self) -> Result { - self.use_key(|keypair| { - Ok(stellar_strkey::ed25519::PublicKey( - *keypair.verifying_key().as_bytes(), - )) - }) + pub fn get_public_key( + &self, + hd_path: Option, + ) -> Result { + self.use_key( + |keypair| { + Ok(stellar_strkey::ed25519::PublicKey( + *keypair.verifying_key().as_bytes(), + )) + }, + hd_path, + ) } - pub fn sign_data(&self, data: &[u8]) -> Result, Error> { - self.use_key(|keypair| { - let signature = keypair.sign(data); - Ok(signature.to_bytes().to_vec()) - }) + pub fn sign_data(&self, data: &[u8], hd_path: Option) -> Result, Error> { + self.use_key( + |keypair| { + let signature = keypair.sign(data); + Ok(signature.to_bytes().to_vec()) + }, + hd_path, + ) } } @@ -77,56 +91,57 @@ mod test { fn test_get_password() { set_default_credential_builder(mock::default_credential_builder()); - let secret = crate::config::secret::Secret::from_seed(None).unwrap(); - let key_pair = secret.key_pair(None).unwrap(); + let seed_phrase = crate::config::secret::seed_phrase_from_seed(None).unwrap(); + let seed_phrase_clone = seed_phrase.clone(); let entry = StellarEntry::new("test").unwrap(); - // set the password - let set_password_result = entry.set_password(&key_pair.to_bytes()); - assert!(set_password_result.is_ok()); - - // get_password should return the same password we set - let get_password_result = entry.get_password(); - assert!(get_password_result.is_ok()); - assert_eq!(key_pair.to_bytes().to_vec(), get_password_result.unwrap()); + // set the seed phrase + let set_seed_phrase_result = entry.set_seed_phrase(seed_phrase); + assert!(set_seed_phrase_result.is_ok()); + + // get_seed_phrase should return the same seed phrase we set + let get_seed_phrase_result = entry.get_seed_phrase(); + assert!(get_seed_phrase_result.is_ok()); + assert_eq!( + seed_phrase_clone.phrase(), + get_seed_phrase_result.unwrap().phrase() + ); } #[test] fn test_get_public_key() { set_default_credential_builder(mock::default_credential_builder()); - let secret = crate::config::secret::Secret::from_seed(None).unwrap(); - let public_key = secret.public_key(None).unwrap(); - let key_pair = secret.key_pair(None).unwrap(); + let seed_phrase = crate::config::secret::seed_phrase_from_seed(None).unwrap(); + let public_key = seed_phrase.from_path_index(0, None).unwrap().public().0; let entry = StellarEntry::new("test").unwrap(); - // set the password - let set_password_result = entry.set_password(&key_pair.to_bytes()); - assert!(set_password_result.is_ok()); + // set the seed_phrase + let set_seed_phrase_result = entry.set_seed_phrase(seed_phrase); + assert!(set_seed_phrase_result.is_ok()); // confirm that we can get the public key from the entry and that it matches the one we set - let get_public_key_result = entry.get_public_key(); + let get_public_key_result = entry.get_public_key(None); assert!(get_public_key_result.is_ok()); - assert_eq!(public_key, get_public_key_result.unwrap()); + assert_eq!(public_key, get_public_key_result.unwrap().0); } #[test] fn test_sign_data() { set_default_credential_builder(mock::default_credential_builder()); - //create a secret - let secret = crate::config::secret::Secret::from_seed(None).unwrap(); - let key_pair = secret.key_pair(None).unwrap(); + //create a seed phrase + let seed_phrase = crate::config::secret::seed_phrase_from_seed(None).unwrap(); - // create a keyring entry and set the password + // create a keyring entry and set the seed_phrase let entry = StellarEntry::new("test").unwrap(); - entry.set_password(&key_pair.to_bytes()).unwrap(); + entry.set_seed_phrase(seed_phrase).unwrap(); let tx_xdr = r"AAAAAgAAAADh6eOnZEq1xQgKioffuH7/8D8x8+OdGFEkiYC6QKMWzQAAAGQAAACuAAAAAQAAAAAAAAAAAAAAAQAAAAAAAAAYAAAAAQAAAAAAAAAAAAAAAOHp46dkSrXFCAqKh9+4fv/wPzHz450YUSSJgLpAoxbNoFT1s8jZPCv9IJ2DsqGTA8pOtavv58JF53aDycpRPcEAAAAA+N2m5zc3EfWUmLvigYPOHKXhSy8OrWfVibc6y6PrQoYAAAAAAAAAAAAAAAA"; - let sign_tx_env_result = entry.sign_data(tx_xdr.as_bytes()); + let sign_tx_env_result = entry.sign_data(tx_xdr.as_bytes(), None); assert!(sign_tx_env_result.is_ok()); } } From 63f247e5e3f11e858a0637b547a16e2ebae814c9 Mon Sep 17 00:00:00 2001 From: Willem Wyndham Date: Tue, 7 Jan 2025 15:15:59 -0500 Subject: [PATCH 42/42] fix: clean up --- cmd/soroban-cli/src/commands/contract/arg_parsing.rs | 7 +------ cmd/soroban-cli/src/commands/keys/add.rs | 8 ++------ cmd/soroban-cli/src/commands/keys/address.rs | 11 +---------- cmd/soroban-cli/src/config/address.rs | 8 +++++--- 4 files changed, 9 insertions(+), 25 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index 98a6d3004..bb2d2aa76 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -14,9 +14,8 @@ use crate::xdr::{ use crate::commands::txn_result::TxnResult; use crate::config::{ - self, address, + self, sc_address::{self, UnresolvedScAddress}, - secret, }; use soroban_spec_tools::Spec; @@ -42,10 +41,6 @@ pub enum Error { Xdr(#[from] xdr::Error), #[error(transparent)] StrVal(#[from] soroban_spec_tools::Error), - #[error(transparent)] - Address(#[from] address::Error), - #[error(transparent)] - Secret(#[from] secret::Error), #[error("Missing argument {0}")] MissingArgument(String), #[error("")] diff --git a/cmd/soroban-cli/src/commands/keys/add.rs b/cmd/soroban-cli/src/commands/keys/add.rs index 57a218ede..4c5ddbd9b 100644 --- a/cmd/soroban-cli/src/commands/keys/add.rs +++ b/cmd/soroban-cli/src/commands/keys/add.rs @@ -2,10 +2,7 @@ use clap::command; use crate::{ commands::global, - config::{ - address::{self, KeyName}, - locator, secret, - }, + config::{address::KeyName, locator, secret}, print::Print, }; @@ -13,10 +10,9 @@ use crate::{ pub enum Error { #[error(transparent)] Secret(#[from] secret::Error), + #[error(transparent)] Config(#[from] locator::Error), - #[error(transparent)] - Address(#[from] address::Error), } #[derive(Debug, clap::Parser, Clone)] diff --git a/cmd/soroban-cli/src/commands/keys/address.rs b/cmd/soroban-cli/src/commands/keys/address.rs index 9727988e0..51ce90ed2 100644 --- a/cmd/soroban-cli/src/commands/keys/address.rs +++ b/cmd/soroban-cli/src/commands/keys/address.rs @@ -1,21 +1,12 @@ use clap::arg; use crate::{ - commands::config::{address, locator, secret}, + commands::config::{address, locator}, config::UnresolvedMuxedAccount, }; #[derive(thiserror::Error, Debug)] pub enum Error { - #[error(transparent)] - Config(#[from] locator::Error), - - #[error(transparent)] - Secret(#[from] secret::Error), - - #[error(transparent)] - StrKey(#[from] stellar_strkey::DecodeError), - #[error(transparent)] Address(#[from] address::Error), } diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 24a40a62a..f86f88ecb 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -55,8 +55,8 @@ impl UnresolvedMuxedAccount { ) -> Result { match self { UnresolvedMuxedAccount::Resolved(muxed_account) => Ok(muxed_account.clone()), - UnresolvedMuxedAccount::AliasOrSecret(alias) => { - Self::resolve_muxed_account_with_alias(alias, locator, hd_path) + UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => { + Self::resolve_muxed_account_with_alias(alias_or_secret, locator, hd_path) } } } @@ -78,7 +78,9 @@ impl UnresolvedMuxedAccount { UnresolvedMuxedAccount::Resolved(muxed_account) => { Err(Error::CannotSign(muxed_account.clone())) } - UnresolvedMuxedAccount::AliasOrSecret(alias) => Ok(locator.key(alias)?), + UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => { + Ok(locator.key(alias_or_secret)?) + } } } }