diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 829447d1f..16ed660f9 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -959,6 +959,8 @@ Add a new identity (keypair, ledger, OS specific secure store) * `--secret-key` — Add using `secret_key` Can provide with `SOROBAN_SECRET_KEY` * `--seed-phrase` — Add using 12 word seed phrase to generate `secret_key` +* `--secure-store` — Add using a key saved in a secure store entry. Requires the entry name to be provided with `--entry_name` +* `--entry-name ` — Name of the secure store entry, to be used with `--secure_store` * `--global` — Use global config * `--config-dir ` — Location of config directory, default is "." @@ -1006,7 +1008,9 @@ Fund an identity on a test network ## `stellar keys generate` -Generate a new identity with a seed phrase, currently 12 words +Generate a new identity with a seed phrase, currently 12 words. + +The identity's secret can be stored in a config file (default), in an OS-specific secure store, or be printed out to the console. **Usage:** `stellar keys generate [OPTIONS] ` @@ -2014,7 +2018,7 @@ Sign a transaction envelope appending the signature to the envelope ###### **Options:** -* `--sign-with-key ` — Sign with a local key. Can be an identity (--sign-with-key alice), a secret key (--sign-with-key SC36…), or a seed phrase (--sign-with-key "kite urban…"). If using seed phrase, `--hd-path` defaults to the `0` path +* `--sign-with-key ` — Sign with a local key or a key saved in OS's secure storage. Can be an identity (--sign-with-key alice), a secret key (--sign-with-key SC36…), or a seed phrase (--sign-with-key "kite urban…"). If using seed phrase, `--hd-path` defaults to the `0` path * `--hd-path ` — If using a seed phrase to sign, sets which hierarchical deterministic path to use, e.g. `m/44'/148'/{hd_path}`. Example: `--hd-path 1`. Default: `0` * `--sign-with-lab` — Sign with https://lab.stellar.org * `--rpc-url ` — RPC server endpoint diff --git a/cmd/soroban-cli/src/commands/keys/mod.rs b/cmd/soroban-cli/src/commands/keys/mod.rs index b3309fa02..ab0884a1b 100644 --- a/cmd/soroban-cli/src/commands/keys/mod.rs +++ b/cmd/soroban-cli/src/commands/keys/mod.rs @@ -21,7 +21,9 @@ pub enum Cmd { /// Fund an identity on a test network Fund(fund::Cmd), - /// Generate a new identity with a seed phrase, currently 12 words + /// Generate a new identity with a seed phrase, currently 12 words. + /// + /// The identity's secret can be stored in a config file (default), in an OS-specific secure store, or be printed out to the console. Generate(generate::Cmd), /// List identities @@ -75,7 +77,7 @@ impl Cmd { Cmd::Fund(cmd) => cmd.run().await?, Cmd::Generate(cmd) => cmd.run(global_args).await?, Cmd::Ls(cmd) => cmd.run()?, - Cmd::Rm(cmd) => cmd.run()?, + Cmd::Rm(cmd) => cmd.run(global_args)?, Cmd::Show(cmd) => cmd.run()?, Cmd::Default(cmd) => cmd.run(global_args)?, }; diff --git a/cmd/soroban-cli/src/commands/keys/rm.rs b/cmd/soroban-cli/src/commands/keys/rm.rs index df48108d3..c551d05e8 100644 --- a/cmd/soroban-cli/src/commands/keys/rm.rs +++ b/cmd/soroban-cli/src/commands/keys/rm.rs @@ -1,5 +1,7 @@ use clap::command; +use crate::commands::global; + use super::super::config::locator; #[derive(thiserror::Error, Debug)] @@ -19,7 +21,7 @@ pub struct Cmd { } impl Cmd { - pub fn run(&self) -> Result<(), Error> { - Ok(self.config.remove_identity(&self.name)?) + pub fn run(&self, global_args: &global::Args) -> Result<(), Error> { + Ok(self.config.remove_identity(&self.name, global_args)?) } } diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index b6f5c75c1..2f6f36f0e 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -12,7 +12,13 @@ use std::{ }; use stellar_strkey::{Contract, DecodeError}; -use crate::{commands::HEADING_GLOBAL, utils::find_config_dir, Pwd}; +use crate::{ + commands::{global, HEADING_GLOBAL}, + print::Print, + signer::{self, keyring::StellarEntry}, + utils::find_config_dir, + Pwd, +}; use super::{ alias, @@ -83,6 +89,8 @@ 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(transparent)] + Keyring(#[from] signer::keyring::Error), } #[derive(Debug, clap::Args, Default, Clone)] @@ -253,7 +261,24 @@ impl Args { res } - pub fn remove_identity(&self, name: &str) -> Result<(), Error> { + pub fn remove_identity(&self, name: &str, global_args: &global::Args) -> Result<(), Error> { + let print = Print::new(global_args.quiet); + let identity = self.read_identity(name)?; + if let Secret::SecureStore { entry_name } = identity { + let entry = StellarEntry::new(&entry_name)?; + match entry.delete_password() { + Ok(()) => {} + Err(e) => match e { + signer::keyring::Error::Keyring(keyring::Error::NoEntry) => { + print.infoln("This key was already removed from the secure store. Removing the cli config file."); + } + _ => { + return Err(Error::Keyring(e)); + } + }, + } + } + KeyType::Identity.remove(name, &self.config_dir()?) } diff --git a/cmd/soroban-cli/src/config/secret.rs b/cmd/soroban-cli/src/config/secret.rs index cd3bc908a..1e89097ad 100644 --- a/cmd/soroban-cli/src/config/secret.rs +++ b/cmd/soroban-cli/src/config/secret.rs @@ -38,11 +38,25 @@ 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 = "seed_phrase", conflicts_with = "secure_store")] pub secret_key: bool, + /// Add using 12 word seed phrase to generate `secret_key` - #[arg(long, conflicts_with = "secret_key")] + #[arg(long, conflicts_with = "secret_key", conflicts_with = "secure_store")] pub seed_phrase: bool, + + /// Add using a key saved in a secure store entry. Requires the entry name to be provided with `--entry_name` + #[arg( + long, + requires = "entry_name", + conflicts_with = "seed_phrase", + conflicts_with = "secret_key" + )] + pub secure_store: bool, + + /// Name of the secure store entry, to be used with `--secure_store` + #[arg(long, requires = "secure_store")] + pub entry_name: Option, } impl Args { @@ -71,6 +85,17 @@ impl Args { .collect::>() .join(" "), }) + } else if self.secure_store { + let entry_name_with_prefix = format!( + "{}{}-{}", + keyring::SECURE_STORE_ENTRY_PREFIX, + keyring::SECURE_STORE_ENTRY_SERVICE, + self.entry_name.as_ref().unwrap() + ); + + Ok(Secret::SecureStore { + entry_name: entry_name_with_prefix, + }) } else { Err(Error::PasswordRead {}) } diff --git a/cmd/soroban-cli/src/config/sign_with.rs b/cmd/soroban-cli/src/config/sign_with.rs index 475013bc8..40512fc48 100644 --- a/cmd/soroban-cli/src/config/sign_with.rs +++ b/cmd/soroban-cli/src/config/sign_with.rs @@ -34,7 +34,7 @@ pub enum Error { #[derive(Debug, clap::Args, Clone, Default)] #[group(skip)] pub struct Args { - /// Sign with a local key. Can be an identity (--sign-with-key alice), a secret key (--sign-with-key SC36…), or a seed phrase (--sign-with-key "kite urban…"). If using seed phrase, `--hd-path` defaults to the `0` path. + /// Sign with a local key or a key saved in OS's secure storage. Can be an identity (--sign-with-key alice), a secret key (--sign-with-key SC36…), or a seed phrase (--sign-with-key "kite urban…"). If using seed phrase, `--hd-path` defaults to the `0` path. #[arg(long, env = "STELLAR_SIGN_WITH_KEY")] pub sign_with_key: Option, diff --git a/cmd/soroban-cli/src/signer/keyring.rs b/cmd/soroban-cli/src/signer/keyring.rs index b5a63a398..0a07e9c33 100644 --- a/cmd/soroban-cli/src/signer/keyring.rs +++ b/cmd/soroban-cli/src/signer/keyring.rs @@ -35,6 +35,10 @@ impl StellarEntry { Ok(base64.decode(self.keyring.get_password()?)?) } + pub fn delete_password(&self) -> Result<(), Error> { + Ok(self.keyring.delete_credential()?) + } + fn use_key( &self, f: impl FnOnce(ed25519_dalek::SigningKey) -> Result, @@ -116,7 +120,7 @@ mod test { fn test_sign_data() { set_default_credential_builder(mock::default_credential_builder()); - //create a secret + // create a secret let secret = crate::config::secret::Secret::from_seed(None).unwrap(); let key_pair = secret.key_pair(None).unwrap(); @@ -129,4 +133,29 @@ mod test { let sign_tx_env_result = entry.sign_data(tx_xdr.as_bytes()); assert!(sign_tx_env_result.is_ok()); } + + #[test] + fn test_delete_password() { + set_default_credential_builder(mock::default_credential_builder()); + + // add a keyring entry + let entry = StellarEntry::new("test").unwrap(); + entry.set_password("test password".as_bytes()).unwrap(); + + // assert it is there + let get_password_result = entry.get_password(); + assert!(get_password_result.is_ok()); + + // delete the password + let delete_password_result = entry.delete_password(); + assert!(delete_password_result.is_ok()); + + // confirm the entry is gone + let get_password_result = entry.get_password(); + assert!(get_password_result.is_err()); + assert!(matches!( + get_password_result.unwrap_err(), + Error::Keyring(_) + )); + } }