Fix signing with a secure store key (#1982)

* Add keys integration tests

* Test creating a create-account tx and signing with secure store key

* Use Secret's signer fn to get the signer instead of creating one using the signing key

* Add secure-store-tests to makefile e2e-test

* Add & start gnome-keyring for linux integration tests

* Enable additional-libs for rpc-tests

Author: elizabethengelman

.github/workflows/rpc-tests.yml

@@ -32,9 +32,20 @@ jobs:
       - uses: actions/checkout@v4
       - uses: stellar/actions/rust-cache@main
       - run: rustup update
-      - run: sudo apt update && sudo apt install -y libudev-dev libdbus-1-dev
+      - run: sudo apt update && sudo apt install -y libudev-dev libdbus-1-dev gnome-keyring
         if: runner.os == 'Linux'
-      - run: cargo build
+      - name: Start gnome-keyring
+        if: runner.os == 'Linux'
+        # run gnome-keyring with 'foobar' as password for the login keyring
+        # this will create a new login keyring and unlock it
+        # the login password doesn't matter, but the keyring must be unlocked for the tests to work
+        # this is based on the ci workflow in the keyring crate repo
+        run: gnome-keyring-daemon --components=secrets --daemonize --unlock <<< 'foobar'
+      - name: Check GNOME Keyring
+        if: runner.os == 'Linux'
+        run: |
+          gnome-keyring-daemon
+      - run: cargo build --features additional-libs
       - run: rustup target add wasm32-unknown-unknown
       - run: make build-test-wasms
       - run: SOROBAN_PORT=8000 cargo test --features it --package soroban-test --test it -- integration --test-threads=1

cmd/crates/soroban-test/Cargo.toml

@@ -55,4 +55,4 @@ default = []
 it = []
 emulator-tests = ["stellar-ledger/emulator-tests"]
 version_lt_23 = []
-version_gte_23 = []
+version_gte_23 = []

cmd/crates/soroban-test/tests/it/integration.rs

@@ -6,6 +6,7 @@ mod dotenv;
 mod hello_world;
 mod init;
 mod keys;
+mod secure_store;
 mod snapshot;
 mod tx;
 mod util;

cmd/crates/soroban-test/tests/it/integration/keys.rs

@@ -33,6 +33,23 @@ async fn fund() {
         .success();
 }
 
+#[tokio::test]
+async fn secret() {
+    let sandbox = &TestEnv::new();
+    sandbox
+        .new_assert_cmd("keys")
+        .arg("generate")
+        .arg("test2")
+        .assert()
+        .success();
+    sandbox
+        .new_assert_cmd("keys")
+        .arg("secret")
+        .arg("test2")
+        .assert()
+        .success();
+}
+
 #[tokio::test]
 #[allow(clippy::too_many_lines)]
 async fn overwrite_identity() {

cmd/crates/soroban-test/tests/it/integration/secure_store.rs

@@ -0,0 +1,98 @@
+use predicates::prelude::predicate;
+use soroban_cli::tx::ONE_XLM;
+use soroban_test::{AssertExt, TestEnv};
+
+fn secure_store_key(sandbox: &TestEnv, name: &str) -> String {
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["generate", "--fund", "--secure-store", name])
+        .assert()
+        .success()
+        .stdout_as_str();
+
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["address", name])
+        .assert()
+        .success()
+        .stdout_as_str()
+}
+
+// test that we can create a create-account tx and sign it with a secure-store key
+#[tokio::test]
+async fn create_account() {
+    let sandbox = &TestEnv::new();
+    let secure_store_address = secure_store_key(sandbox, "secure-store");
+
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["generate", "--no-fund", "new"])
+        .assert()
+        .success();
+    let new_address = sandbox
+        .new_assert_cmd("keys")
+        .args(["address", "new"])
+        .assert()
+        .success()
+        .stdout_as_str();
+
+    let client = sandbox.network.rpc_client().unwrap();
+    let secure_account = client.get_account(&secure_store_address).await.unwrap();
+
+    let starting_balance = ONE_XLM * 100;
+    sandbox
+        .new_assert_cmd("tx")
+        .args([
+            "new",
+            "create-account",
+            "--destination",
+            new_address.as_str(),
+            "--starting-balance",
+            starting_balance.to_string().as_str(),
+            "--source",
+            "secure-store",
+        ])
+        .assert()
+        .success()
+        .stdout_as_str();
+
+    let secure_account_after = client.get_account(&secure_store_address).await.unwrap();
+    assert!(secure_account_after.balance < secure_account.balance);
+
+    let new_account = client.get_account(&new_address).await.unwrap();
+    assert_eq!(new_account.balance, starting_balance);
+}
+
+#[tokio::test]
+async fn get_secret_key() {
+    let sandbox = &TestEnv::new();
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["generate", "secret-key-test", "--secure-store"])
+        .assert()
+        .success();
+    sandbox
+        .new_assert_cmd("keys")
+        .arg("secret")
+        .arg("secret-key-test")
+        .assert()
+        .stderr(predicate::str::contains("does not reveal secret key"))
+        .failure();
+}
+
+#[tokio::test]
+async fn public_key_with_secure_store() {
+    let sandbox = &TestEnv::new();
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["generate", "public-key-test", "--secure-store"])
+        .assert()
+        .success();
+    sandbox
+        .new_assert_cmd("keys")
+        .arg("public-key")
+        .arg("public-key-test")
+        .assert()
+        .stdout(predicate::str::contains("G"))
+        .success();
+}

cmd/soroban-cli/src/config/mod.rs

@@ -7,7 +7,7 @@ use std::{
 
 use crate::{
     print::Print,
-    signer::{self, LocalKey, Signer, SignerKind},
+    signer,
     xdr::{self, SequenceNumber, Transaction, TransactionEnvelope},
     Pwd,
 };
@@ -89,12 +89,10 @@ impl Args {
 
     #[allow(clippy::unused_async)]
     pub async fn sign(&self, tx: Transaction) -> Result<TransactionEnvelope, Error> {
-        let key = self.key_pair()?;
+        let key = &self.source_account.resolve_secret(&self.locator)?;
+        let signer = key.signer(self.hd_path, Print::new(false)).await?;
         let network = &self.get_network()?;
-        let signer = Signer {
-            kind: SignerKind::Local(LocalKey { key }),
-            print: Print::new(false),
-        };
+
         Ok(signer.sign_tx(tx, network).await?)
     }