Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 16 additions & 6 deletions cmd/soroban-cli/src/signer/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ pub enum Error {

#[error(transparent)]
Xdr(#[from] xdr::Error),

#[error(transparent)]
Validation(#[from] crate::signer::validation::Error),
}

#[cfg(feature = "additional-libs")]
Expand Down Expand Up @@ -47,13 +50,17 @@ mod ledger_impl {
Some(pk) => pk,
None => live.public_key().await?,
};
let sig_bytes = live
.signer
.sign_transaction_hash(live.index, &tx_hash)
.await?;
// Only the cached-pubkey path can drift; the cacheless path fetched the
// key live from the same device that just signed.
if self.public_key.is_some() {
crate::signer::validation::verify_signature(&key, &tx_hash, &sig_bytes)?;
}
let hint = SignatureHint(key.0[28..].try_into()?);
let signature = Signature(
live.signer
.sign_transaction_hash(live.index, &tx_hash)
.await?
.try_into()?,
);
let signature = Signature(sig_bytes.try_into()?);
Ok(DecoratedSignature { hint, signature })
}

Expand All @@ -63,6 +70,9 @@ mod ledger_impl {
.signer
.sign_transaction_hash(live.index, &payload)
.await?;
if let Some(pk) = self.public_key {
crate::signer::validation::verify_signature(&pk, &payload, &bytes)?;
}
Ok(Ed25519Signature::from_bytes(bytes.as_slice().try_into()?))
}
}
Expand Down
9 changes: 9 additions & 0 deletions cmd/soroban-cli/src/signer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub enum Error {
Ledger(#[from] ledger::Error),
#[error(transparent)]
Decode(#[from] stellar_strkey::DecodeError),
#[error(transparent)]
Validation(#[from] validation::Error),
}

/// Sign all SorobanAuthorizationEntry's in the transaction with the given signers. Returns a new
Expand Down Expand Up @@ -472,12 +474,19 @@ impl SecureStoreEntry {

let signed_tx_hash = secure_store::sign_tx_data(&self.name, self.hd_path, &tx_hash)?;

if let Some(pk) = self.public_key {
validation::verify_signature(&pk, &tx_hash, &signed_tx_hash)?;
}

let signature = Signature(signed_tx_hash.clone().try_into()?);
Ok(DecoratedSignature { hint, signature })
}

pub fn sign_payload(&self, payload: [u8; 32]) -> Result<Ed25519Signature, Error> {
let signed_bytes = secure_store::sign_tx_data(&self.name, self.hd_path, &payload)?;
if let Some(pk) = self.public_key {
validation::verify_signature(&pk, &payload, &signed_bytes)?;
}
let sig = Ed25519Signature::from_bytes(signed_bytes.as_slice().try_into()?);
Ok(sig)
}
Expand Down
80 changes: 80 additions & 0 deletions cmd/soroban-cli/src/signer/validation.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,45 @@
use crate::xdr::{HostFunction, SorobanAuthorizedFunction, SorobanAuthorizedInvocation};

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error(
"the produced signature does not match the cached public key {public_key}; \
the cached key may be stale — check the correct device is connected / the alias's \
hd-path is right, or re-add the key"
)]
SignatureMismatch { public_key: String },

#[error("the cached public key {public_key} is not a valid ed25519 public key")]
InvalidPublicKey { public_key: String },

#[error("the signer returned a malformed ed25519 signature ({len} bytes, expected 64)")]
InvalidSignature { len: usize },
}

/// Verify that `signature` over `message` was produced by the secret key
/// corresponding to `public_key`. Used as a post-sign drift guard for
/// cached-pubkey signers (Ledger / Secure Store): the signature is produced on a
/// live device/keychain while the hint and embedded public key are derived from a
/// cached value, so a stale cache yields a transaction the network silently
/// rejects. This catches that locally with a clear error.
pub fn verify_signature(
public_key: &stellar_strkey::ed25519::PublicKey,
message: &[u8; 32],
signature: &[u8],
) -> Result<(), Error> {
use ed25519_dalek::{Signature, VerifyingKey};
let vk = VerifyingKey::from_bytes(&public_key.0).map_err(|_| Error::InvalidPublicKey {
public_key: format!("{public_key}"),
})?;
let sig = Signature::from_slice(signature).map_err(|_| Error::InvalidSignature {
len: signature.len(),
})?;
vk.verify_strict(message, &sig)
.map_err(|_| Error::SignatureMismatch {
public_key: format!("{public_key}"),
})
}

/// Classification of an `Address`-credential auth entry's relationship to the
/// transaction's host function.
///
Expand Down Expand Up @@ -125,6 +165,46 @@ mod tests {
}
}

#[test]
fn test_verify_signature_roundtrip() {
use ed25519_dalek::{ed25519::signature::Signer as _, SigningKey};

let signing_key = SigningKey::from_bytes(&[7u8; 32]);
let public_key =
ed25519::PublicKey::from_payload(signing_key.verifying_key().as_bytes()).unwrap();
let message = [42u8; 32];
let signature = signing_key.sign(&message).to_bytes();

// Matching key + untampered signature verifies.
verify_signature(&public_key, &message, &signature).unwrap();

// A different public key is rejected.
let cached_pk = ed25519::PublicKey::from_payload(
SigningKey::from_bytes(&[9u8; 32])
.verifying_key()
.as_bytes(),
)
.unwrap();
assert!(matches!(
verify_signature(&cached_pk, &message, &signature),
Err(Error::SignatureMismatch { .. })
));

// A tampered signature is rejected.
let mut tampered = signature;
tampered[0] ^= 0xff;
assert!(matches!(
verify_signature(&public_key, &message, &tampered),
Err(Error::SignatureMismatch { .. })
));

// A wrong-length signature is reported as malformed, not as a mismatch.
assert!(matches!(
verify_signature(&public_key, &message, &signature[..63]),
Err(Error::InvalidSignature { len: 63 })
));
}

#[test]
fn test_matching_root_invocation_is_strict() {
let contract = [1u8; 32];
Expand Down