diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index d913cf23d..ebfcf927c 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: | @@ -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) diff --git a/.github/workflows/bindings-ts.yml b/.github/workflows/bindings-ts.yml index 9b5f767ac..d09725323 100644 --- a/.github/workflows/bindings-ts.yml +++ b/.github/workflows/bindings-ts.yml @@ -38,6 +38,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 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: diff --git a/Cargo.lock b/Cargo.lock index 6789ea504..441d6f98b 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/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 04b144315..829447d1f 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -934,7 +934,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 @@ -947,7 +947,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] ` @@ -1019,6 +1019,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` diff --git a/cmd/soroban-cli/Cargo.toml b/cmd/soroban-cli/Cargo.toml index 2f2a26255..3d366464b 100644 --- a/cmd/soroban-cli/Cargo.toml +++ b/cmd/soroban-cli/Cargo.toml @@ -72,7 +72,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 +125,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/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/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/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)) + } } } } diff --git a/cmd/soroban-cli/src/commands/keys/generate.rs b/cmd/soroban-cli/src/commands/keys/generate.rs index c6623386c..bd9f8c2f6 100644 --- a/cmd/soroban-cli/src/commands/keys/generate.rs +++ b/cmd/soroban-cli/src/commands/keys/generate.rs @@ -4,7 +4,12 @@ use super::super::config::{ locator, network, secret::{self, Secret}, }; -use crate::{commands::global, print::Print}; +use crate::{ + commands::global, + config::address::KeyName, + print::Print, + signer::keyring::{self, StellarEntry}, +}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -19,6 +24,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)] @@ -26,10 +34,12 @@ 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, + /// Optional seed to use when generating seed phrase. /// Random otherwise. #[arg(long, conflicts_with = "default_seed")] @@ -39,6 +49,10 @@ pub struct Cmd { #[arg(long, short = 's')] pub as_secret: bool, + /// Save in OS-specific secure store + #[arg(long)] + pub secure_store: bool, + #[command(flatten)] pub config_locator: locator::Args, @@ -69,10 +83,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 { @@ -83,19 +97,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(&print)?; self.config_locator.write_identity(&self.name, &secret)?; if !self.no_fund { @@ -112,4 +114,130 @@ impl Cmd { Ok(()) } + + 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 + ); + + //checking that the entry name is valid before writing to the secure store + 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)?; + } + + secret + } 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()) + }?) + } + + fn write_to_secure_store( + entry_name: &String, + seed_phrase: &Secret, + 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() { + 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())?; + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use crate::config::{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(); + let locator = super::locator::Args { + global: false, + config_dir: Some(temp_dir.path().to_path_buf()), + }; + + let cmd = super::Cmd { + name: KeyName("test_name".to_string()), + no_fund: true, + seed: None, + as_secret: false, + secure_store: false, + config_locator: locator.clone(), + hd_path: None, + default_seed: false, + network: super::network::Args::default(), + fund: false, + overwrite: false, + }; + + (locator, cmd) + } + + fn global_args() -> super::global::Args { + super::global::Args { + quiet: true, + ..Default::default() + } + } + + #[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_secure_store() { + set_default_credential_builder(mock::default_credential_builder()); + let (test_locator, mut cmd) = set_up_test(); + cmd.secure_store = 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::SecureStore { .. })); + } } 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/address.rs b/cmd/soroban-cli/src/config/address.rs index 066bc8d91..ff926b60a 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; @@ -25,6 +28,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 { @@ -56,8 +61,43 @@ 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 @ Address::MuxedAccount(_) => { + Err(Error::CannotSign(a.resolve_muxed_account(locator, None)?)) + } } } } + +#[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())) + } +} + +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 == '-' +} diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index a7fd86fda..cd3bc908a 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, LocalKey, Signer, SignerKind}, + signer::{self, keyring, LocalKey, SecureStoreEntry, Signer, SignerKind}, utils, }; @@ -27,6 +27,10 @@ pub enum Error { InvalidAddress(String), #[error(transparent)] Signer(#[from] signer::Error), + #[error(transparent)] + Keyring(#[from] keyring::Error), + #[error("Secure Store does not reveal secret key")] + SecureStoreDoesNotRevealSecretKey, } #[derive(Debug, clap::Args, Clone)] @@ -78,6 +82,7 @@ impl Args { pub enum Secret { SecretKey { secret_key: String }, SeedPhrase { seed_phrase: String }, + SecureStore { entry_name: String }, } impl FromStr for Secret { @@ -92,6 +97,10 @@ impl FromStr for Secret { Ok(Secret::SeedPhrase { seed_phrase: s.to_string(), }) + } else if s.starts_with(keyring::SECURE_STORE_ENTRY_PREFIX) { + Ok(Secret::SecureStore { + entry_name: s.to_string(), + }) } else { Err(Error::InvalidAddress(s.to_string())) } @@ -116,14 +125,22 @@ impl Secret { .private() .0, )?, + Secret::SecureStore { .. } => { + return Err(Error::SecureStoreDoesNotRevealSecretKey); + } }) } 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(), - )?) + 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(), + )?) + } } pub fn signer(&self, index: Option, print: Print) -> Result { @@ -132,6 +149,9 @@ impl Secret { let key = self.key_pair(index)?; SignerKind::Local(LocalKey { key }) } + Secret::SecureStore { entry_name } => SignerKind::SecureStore(SecureStoreEntry { + name: entry_name.to_string(), + }), }; Ok(Signer { kind, print }) } @@ -160,3 +180,55 @@ 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_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_from_str_for_secret_key() { + let secret = Secret::from_str(TEST_SECRET_KEY).unwrap(); + 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() { + 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(); + + 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_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 { .. })); + + let private_key_result = secret.private_key(None); + assert!(private_key_result.is_err()); + assert!(matches!( + private_key_result.unwrap_err(), + Error::SecureStoreDoesNotRevealSecretKey + )); + } + + #[test] + fn test_secret_from_invalid_string() { + let secret = Secret::from_str("invalid"); + assert!(secret.is_err()); + } +} diff --git a/cmd/soroban-cli/src/signer.rs b/cmd/soroban-cli/src/signer.rs index 5bf22499c..099ed6186 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 keyring::StellarEntry; use sha2::{Digest, Sha256}; use crate::xdr::{ @@ -11,6 +12,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}")] @@ -33,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 { @@ -207,6 +212,7 @@ pub struct Signer { pub enum SignerKind { Local(LocalKey), Lab, + SecureStore(SecureStoreEntry), } impl Signer { @@ -235,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::SecureStore(entry) => entry.sign_tx_hash(tx_hash)?, }; let mut sigs = signatures.clone().into_vec(); sigs.push(decorated_signature); @@ -284,3 +291,17 @@ impl Lab { Err(Error::ReturningSignatureFromLab) } } + +pub struct SecureStoreEntry { + pub name: String, +} + +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 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 new file mode 100644 index 000000000..b5a63a398 --- /dev/null +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -0,0 +1,132 @@ +use base64::{engine::general_purpose::STANDARD as base64, Engine as _}; +use ed25519_dalek::Signer; +use keyring::Entry; +use zeroize::Zeroize; + +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 { + #[error(transparent)] + Keyring(#[from] keyring::Error), + #[error(transparent)] + Base64(#[from] base64::DecodeError), +} + +pub struct StellarEntry { + keyring: Entry, +} + +impl StellarEntry { + pub fn new(name: &str) -> Result { + Ok(StellarEntry { + keyring: Entry::new(name, &whoami::username())?, + }) + } + + pub fn set_password(&self, password: &[u8]) -> Result<(), Error> { + let data = base64.encode(password); + self.keyring.set_password(&data)?; + Ok(()) + } + + pub fn get_password(&self) -> Result, Error> { + Ok(base64.decode(self.keyring.get_password()?)?) + } + + 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 result = { + // Use this scope to ensure the keypair is zeroized + 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 sign_data(&self, data: &[u8]) -> Result, Error> { + self.use_key(|keypair| { + let signature = keypair.sign(data); + Ok(signature.to_bytes().to_vec()) + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use keyring::{mock, set_default_credential_builder}; + + #[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 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(); + + // 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!(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()); + } +}