Skip to content

Commit

Permalink
feat: add support for OS specific keychains (#1703)
Browse files Browse the repository at this point in the history
* feat: initial work into system keychain

* chore: clean up

* Add KeyName struct in address

* Add Secret::Keychain

* keys generate: allow for generating keys that are stored in keychain

* keys generate: Namespace keychain entry to identity name

* keys generate: don't allow 'keychain:' as a key name

* keys address: use keychain entry in secret to get the pub key

* tx sign: allow a keychain identity sign a tx

* Cleanup

* Use keyring mock for generate tests

* 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

* Add tests for keyring

* Update config/secret tests

* Cleanup

* Rename keychain arg to secure_store in generate

* Rename Secret::Keychain to Secret::SecureStore

* Rename SignerKind::Keychain to SignerKind::SecureStore

* Use print for new fns in generate

* Return error when trying to get Secure Store secret

* Cleanup tests

* Install libdbus for rpc-tests and bindings-ts workflows

required for keyring crate

* Update generated docs

* Install libdbus for binaries workflow when target aarch64-unknown-linux-gnu

* Clippy

* Install libdbus for rust workflow

* Install libdbus-1-dev in binaries workflow for build step

* Impl Display for KeyName

this change was made so that we can concat the KeyName with secure story
prefix and service

* Use resolve_muxed_account in resolve_secret

* Use resolve_muxed_account to get public key

* Clippy

* fix: Sign tx hash instead of tx env with keychain

* Remove unused bin/secret

* Fix after merging with main

* Apply suggestion from code review

Co-authored-by: Willem Wyndham <[email protected]>

* Limit key name length

* Update public_key to work with secure storage keys

* fix(address): remove private key function & use unresolved Address

This simplifies the lookup of the address.

* feat: store seedphrase instead of private key

This will allow for exporting the phrase later

* fix: clean up

---------

Co-authored-by: Elizabeth Engelman <[email protected]>
  • Loading branch information
willemneal and elizabethengelman authored Jan 9, 2025
1 parent c928006 commit c055eac
Show file tree
Hide file tree
Showing 16 changed files with 615 additions and 75 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/binaries.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/bindings-ts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/rpc-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
111 changes: 111 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,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
Expand All @@ -951,7 +951,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] <NAME>`

Expand Down Expand Up @@ -1023,6 +1023,7 @@ Generate a new identity with a seed phrase, currently 12 words
* `--no-fund` — Do not fund address
* `--seed <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 <CONFIG_DIR>` — Location of config directory, default is "."
* `--hd-path <HD_PATH>` — When generating a secret key, which `hd_path` should be used from the original `seed_phrase`
Expand Down
7 changes: 6 additions & 1 deletion cmd/soroban-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
Expand Down
7 changes: 1 addition & 6 deletions cmd/soroban-cli/src/commands/contract/arg_parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,5 @@ fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result<String,
}

fn resolve_signer(addr_or_alias: &str, config: &config::Args) -> Option<SigningKey> {
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()
}
4 changes: 2 additions & 2 deletions cmd/soroban-cli/src/commands/keys/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clap::command;

use crate::{
commands::global,
config::{locator, secret},
config::{address::KeyName, locator, secret},
print::Print,
};

Expand All @@ -19,7 +19,7 @@ pub enum Error {
#[group(skip)]
pub struct Cmd {
/// Name of identity
pub name: String,
pub name: KeyName,

#[command(flatten)]
pub secrets: secret::Args,
Expand Down
40 changes: 15 additions & 25 deletions cmd/soroban-cli/src/commands/keys/address.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,21 @@
use crate::commands::config::secret;

use super::super::config::locator;
use clap::arg;

use crate::{
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),
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: UnresolvedMuxedAccount,

/// If identity is a seed phrase use this hd path, default is 0
#[arg(long)]
Expand All @@ -35,20 +31,14 @@ impl Cmd {
Ok(())
}

pub fn private_key(&self) -> Result<ed25519_dalek::SigningKey, Error> {
Ok(self
.locator
.read_identity(&self.name)?
.key_pair(self.hd_path)?)
}

pub fn public_key(&self) -> Result<stellar_strkey::ed25519::PublicKey, Error> {
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(),
)?)
}
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))
}
}
Loading

0 comments on commit c055eac

Please sign in to comment.