Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bdd7341
Add an additional libs feature for keyring crate
elizabethengelman Mar 17, 2025
30fc05e
Move src/signer.rs to src/signer/mod.rs
elizabethengelman Mar 18, 2025
df082ff
Return an error if a user tries to create a StellarEntry without addi…
elizabethengelman Mar 18, 2025
9d78b43
handle add-libs feature flagging in keyring.rs
elizabethengelman Mar 18, 2025
f35699a
Handle keyring specific errors in keyring.rs
elizabethengelman Mar 18, 2025
0cc7f00
Fix keyring tests
elizabethengelman Mar 19, 2025
b3b30fb
Start to move additional-libs feature to secure_store.rs
elizabethengelman Mar 20, 2025
6d1f398
Remove dependencies to secret.rs and locator.rs from secure_store.rs
elizabethengelman Mar 20, 2025
14e8d53
Use secure_store in locator remove_identity instead of keyring directly
elizabethengelman Mar 20, 2025
5dd755c
Use secure_store in secret to get pub key instead of keyring directly
elizabethengelman Mar 20, 2025
bef9a37
Use secure_store to get pub key in signer/mod
elizabethengelman Mar 20, 2025
3f0960d
Use secure_store to sign tx data instead of keyring directly
elizabethengelman Mar 20, 2025
a91569b
Remove feature flag from keyring.rs
elizabethengelman Mar 20, 2025
f3e2444
Refactor to cleanup unreachable code warnings
elizabethengelman Mar 20, 2025
c681fc5
Fix secure_store write
elizabethengelman Mar 20, 2025
d299684
Update generate tests
elizabethengelman Mar 20, 2025
ac6013d
Cleanup clippy and fmt
elizabethengelman Mar 20, 2025
956c488
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Mar 25, 2025
640bb3a
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Mar 31, 2025
9100472
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Apr 8, 2025
421c82a
Address feedback
elizabethengelman Apr 8, 2025
8b9e16d
Add additional-libs feature to stellar-cli/Cargo.toml
elizabethengelman Apr 9, 2025
1695dc3
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Apr 9, 2025
8467f0d
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Apr 10, 2025
5626fb0
Update Makefile to include additional-libs feature
elizabethengelman Apr 10, 2025
f731762
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Apr 11, 2025
b741a33
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Apr 14, 2025
e9643ad
Merge branch 'main' into feat/additional-libs-feature
elizabethengelman Apr 15, 2025
a7e96f5
additional-libs feature for ledger-stellar dependencies (ledger signi…
elizabethengelman Apr 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/binaries.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:

- name: Build
run: |
cargo build --package ${{ matrix.crate.name }} --features opt --release --target ${{ matrix.sys.target }}
cargo build --package ${{ matrix.crate.name }} --features opt,additional-libs --release --target ${{ matrix.sys.target }}
- name: Build provenance for binary attestation (release only)
if: github.event_name == 'release'
uses: actions/attest-build-provenance@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ledger-emulator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ jobs:
sudo apt update && sudo apt install -y libudev-dev libdbus-1-dev
- run: |
cargo test --manifest-path cmd/crates/stellar-ledger/Cargo.toml --features "emulator-tests" -- --nocapture
- run: cargo build --features emulator-tests
- run: cargo build --features emulator-tests,additional-libs
- run: |
cargo test --features emulator-tests --package soroban-test --test it -- emulator
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ generate-full-help-doc:

test: build-test
cargo test --workspace --exclude soroban-test
cargo test --workspace --exclude soroban-test --features additional-libs
Comment thread
elizabethengelman marked this conversation as resolved.
cargo test -p soroban-test -- --skip integration::

e2e-test:
Expand Down
5 changes: 3 additions & 2 deletions cmd/soroban-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ version_lt_23 = []
version_gte_23 = []
opt = ["dep:wasm-opt"]
emulator-tests = ["stellar-ledger/emulator-tests"]
additional-libs = ["dep:keyring", "dep:stellar-ledger"]

[dependencies]
stellar-xdr = { workspace = true, features = ["cli"] }
Expand All @@ -52,7 +53,7 @@ soroban-ledger-snapshot = { workspace = true }
stellar-strkey = { workspace = true }
soroban-sdk = { workspace = true }
soroban-rpc = { workspace = true }
stellar-ledger = { workspace = true }
stellar-ledger = { workspace = true, optional = true }

clap = { workspace = true, features = [
"derive",
Expand Down Expand Up @@ -131,7 +132,7 @@ 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"] }
keyring = { version = "3", features = ["apple-native", "windows-native", "sync-secret-service"], optional = true }
whoami = "1.5.2"
serde_with = "3.11.0"

Expand Down
3 changes: 2 additions & 1 deletion cmd/soroban-cli/src/commands/keys/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ impl Cmd {

let seed_phrase: SeedPhrase = secret_key.parse()?;

Ok(secure_store::save_secret(print, &self.name, seed_phrase)?)
let secret = secure_store::save_secret(print, &self.name, &seed_phrase)?;
Ok(secret.parse()?)
} else {
let prompt = "Type a secret key or 12/24 word seed phrase:";
let secret_key = read_password(print, prompt)?;
Expand Down
28 changes: 26 additions & 2 deletions cmd/soroban-cli/src/commands/keys/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ impl Cmd {
fn secret(&self, print: &Print) -> Result<Secret, Error> {
let seed_phrase = self.seed_phrase()?;
if self.secure_store {
Ok(secure_store::save_secret(print, &self.name, seed_phrase)?)
let secret = secure_store::save_secret(print, &self.name, &seed_phrase)?;
Ok(secret.parse()?)
} else if self.as_secret {
let secret: Secret = seed_phrase.into();
Ok(secret.private_key(self.hd_path)?.into())
Expand All @@ -144,7 +145,6 @@ impl Cmd {
#[cfg(test)]
mod tests {
use crate::config::{address::KeyName, key::Key, secret::Secret};
use keyring::{mock, set_default_credential_builder};

fn set_up_test() -> (super::locator::Args, super::Cmd) {
let temp_dir = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -200,8 +200,10 @@ mod tests {
assert!(matches!(identity, Key::Secret(Secret::SecretKey { .. })));
}

#[cfg(feature = "additional-libs")]
#[tokio::test]
async fn test_storing_secret_in_secure_store() {
use keyring::{mock, set_default_credential_builder};
set_default_credential_builder(mock::default_credential_builder());
let (test_locator, mut cmd) = set_up_test();
cmd.secure_store = true;
Expand All @@ -212,4 +214,26 @@ mod tests {
let identity = test_locator.read_identity("test_name").unwrap();
assert!(matches!(identity, Key::Secret(Secret::SecureStore { .. })));
}

#[cfg(not(feature = "additional-libs"))]
#[tokio::test]
Comment thread
elizabethengelman marked this conversation as resolved.
async fn test_storing_in_secure_store_returns_error_when_additional_libs_not_enabled() {
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_err());
assert_eq!(
result.unwrap_err().to_string(),
format!("Secure Store keys are not allowed: additional-libs feature must be enabled")
);

let identity_result = test_locator.read_identity("test_name");
assert!(identity_result.is_err());
assert_eq!(
identity_result.unwrap_err().to_string(),
format!("Failed to find config identity for test_name")
);
}
}
4 changes: 3 additions & 1 deletion cmd/soroban-cli/src/config/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ pub enum Error {
InvalidKeyName(String),
#[error("Ledger not supported in this context")]
LedgerNotSupported,
#[error(transparent)]
Ledger(#[from] signer::ledger::Error),
}

impl FromStr for UnresolvedMuxedAccount {
Expand Down Expand Up @@ -85,7 +87,7 @@ impl UnresolvedMuxedAccount {
) -> Result<xdr::MuxedAccount, Error> {
match self {
UnresolvedMuxedAccount::Ledger(hd_path) => Ok(xdr::MuxedAccount::Ed25519(
ledger(*hd_path).await?.public_key().await?.0.into(),
ledger::new(*hd_path).await?.public_key().await?.0.into(),
)),
UnresolvedMuxedAccount::Resolved(_) | UnresolvedMuxedAccount::AliasOrSecret(_) => {
self.resolve_muxed_account_sync(locator, hd_path)
Expand Down
18 changes: 4 additions & 14 deletions cmd/soroban-cli/src/config/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use stellar_strkey::{Contract, DecodeError};
use crate::{
commands::{global, HEADING_GLOBAL},
print::Print,
signer::{self, keyring::StellarEntry},
signer::secure_store,
utils::find_config_dir,
xdr, Pwd,
};
Expand Down Expand Up @@ -95,7 +95,7 @@ pub enum Error {
#[error("Key cannot {0} cannot overlap with contract alias")]
KeyCannotOverlapWithContractAlias(String),
#[error(transparent)]
Keyring(#[from] signer::keyring::Error),
SecureStore(#[from] secure_store::Error),
#[error("Only private keys and seed phrases are supported for getting private keys {0}")]
SecretKeyOnly(String),
#[error(transparent)]
Expand Down Expand Up @@ -301,20 +301,10 @@ impl Args {
let identity = self.read_identity(name)?;

if let Key::Secret(Secret::SecureStore { entry_name }) = identity {
let entry = StellarEntry::new(&entry_name)?;
match entry.delete_seed_phrase() {
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));
}
},
}
secure_store::delete_secret(&print, &entry_name)?;
}

print.infoln("Removing the key's cli config file");
KeyType::Identity.remove(name, &self.config_dir()?)
}

Expand Down
15 changes: 7 additions & 8 deletions cmd/soroban-cli/src/config/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use stellar_strkey::ed25519::{PrivateKey, PublicKey};

use crate::{
print::Print,
signer::{self, keyring, ledger, LocalKey, SecureStoreEntry, Signer, SignerKind},
signer::{self, ledger, secure_store, LocalKey, SecureStoreEntry, Signer, SignerKind},
utils,
};

Expand All @@ -25,14 +25,14 @@ pub enum Error {
InvalidSecretOrSeedPhrase,
#[error(transparent)]
Signer(#[from] signer::Error),

#[error("Ledger does not reveal secret key")]
LedgerDoesNotRevealSecretKey,

#[error(transparent)]
Keyring(#[from] keyring::Error),
SecureStore(#[from] secure_store::Error),
#[error("Secure Store does not reveal secret key")]
SecureStoreDoesNotRevealSecretKey,
#[error(transparent)]
Ledger(#[from] signer::ledger::Error),
}

#[derive(Debug, clap::Args, Clone)]
Expand Down Expand Up @@ -72,7 +72,7 @@ impl FromStr for Secret {
})
} else if s == "ledger" {
Ok(Secret::Ledger)
} else if s.starts_with(keyring::SECURE_STORE_ENTRY_PREFIX) {
} else if s.starts_with(secure_store::ENTRY_PREFIX) {
Ok(Secret::SecureStore {
entry_name: s.to_string(),
})
Expand Down Expand Up @@ -123,8 +123,7 @@ impl Secret {

pub fn public_key(&self, index: Option<usize>) -> Result<PublicKey, Error> {
if let Secret::SecureStore { entry_name } = self {
let entry = keyring::StellarEntry::new(entry_name)?;
Ok(entry.get_public_key(index)?)
Ok(secure_store::get_public_key(entry_name, index)?)
} else {
let key = self.key_pair(index)?;
Ok(stellar_strkey::ed25519::PublicKey::from_payload(
Expand All @@ -144,7 +143,7 @@ impl Secret {
.unwrap_or_default()
.try_into()
.expect("uszie bigger than u32");
SignerKind::Ledger(ledger(hd_path).await?)
SignerKind::Ledger(ledger::new(hd_path).await?)
}
Secret::SecureStore { entry_name } => SignerKind::SecureStore(SecureStoreEntry {
name: entry_name.to_string(),
Expand Down
4 changes: 3 additions & 1 deletion cmd/soroban-cli/src/config/sign_with.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub enum Error {
StrKey(#[from] stellar_strkey::DecodeError),
#[error(transparent)]
Xdr(#[from] xdr::Error),
#[error(transparent)]
Ledger(#[from] signer::ledger::Error),
}

#[derive(Debug, clap::Args, Clone, Default)]
Expand Down Expand Up @@ -72,7 +74,7 @@ impl Args {
print,
}
} else if self.sign_with_ledger {
let ledger = ledger(
let ledger = ledger::new(
self.hd_path
.unwrap_or_default()
.try_into()
Expand Down
50 changes: 43 additions & 7 deletions cmd/soroban-cli/src/signer/keyring.rs
Original file line number Diff line number Diff line change
@@ -1,39 +1,71 @@
use crate::print::Print;
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:";
pub(crate) const SECURE_STORE_ENTRY_SERVICE: &str = "org.stellar.cli";

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[cfg(feature = "additional-libs")]
#[error(transparent)]
Keyring(#[from] keyring::Error),

#[error(transparent)]
Sep5(#[from] sep5::error::Error),

#[error("Secure Store keys are not allowed: additional-libs feature must be enabled")]
FeatureNotEnabled,
}

pub struct StellarEntry {
name: String,
#[cfg(feature = "additional-libs")]
keyring: Entry,
}

impl StellarEntry {
pub fn new(name: &str) -> Result<Self, Error> {
Ok(StellarEntry {
name: name.to_string(),
keyring: Entry::new(name, &whoami::username())?,
})
}

pub fn set_seed_phrase(&self, seed_phrase: SeedPhrase) -> Result<(), Error> {
pub fn write(&self, seed_phrase: SeedPhrase, print: &Print) -> Result<(), Error> {
if let Ok(key) = self.get_public_key(None) {
print.warnln(format!(
"A key for {0} already exists in your operating system's secure store: {1}",
self.name, key
));
} else {
print.infoln(format!(
"Saving a new key to your operating system's secure store: {0}",
self.name
));
self.set_seed_phrase(seed_phrase)?;
};
Ok(())
}

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(())
}

pub fn delete_seed_phrase(&self) -> Result<(), Error> {
Ok(self.keyring.delete_credential()?)
pub fn delete_seed_phrase(&self, print: &Print) -> Result<(), Error> {
match self.keyring.delete_credential() {
Ok(()) => Ok(()),
Err(e) => match e {
keyring::Error::NoEntry => {
print.infoln("This key was already removed from the secure store.");
Ok(())
}
_ => Err(Error::Keyring(e)),
},
}
}

fn get_seed_phrase(&self) -> Result<SeedPhrase, Error> {
Expand Down Expand Up @@ -86,8 +118,11 @@ impl StellarEntry {
}
}

#[cfg(feature = "additional-libs")]
#[cfg(test)]
mod test {
use crate::print;

use super::*;
use keyring::{mock, set_default_credential_builder};

Expand Down Expand Up @@ -165,7 +200,8 @@ mod test {
assert!(get_seed_phrase_result.is_ok());

// delete the password
let delete_seed_phrase_result = entry.delete_seed_phrase();
let print = print::Print::new(true);
let delete_seed_phrase_result = entry.delete_seed_phrase(&print);
assert!(delete_seed_phrase_result.is_ok());

// confirm the entry is gone
Expand Down
Loading