From c0773a323e17bf21ef22137de129990ac6f3b498 Mon Sep 17 00:00:00 2001 From: benma's agent Date: Thu, 6 Aug 2026 00:03:23 +0000 Subject: [PATCH] antiklepto: validate ECDSA signatures Use shared compact and recoverable ECDSA validators for direct signing responses and from inside Anti-Klepto verification. Reject malformed scalars, high-S encodings, and recovery IDs outside the secp256k1 range before accepting signatures. Accept recovery IDs 0..3 for all recoverable ECDSA signatures. Add regression tests and prepare Rust 0.13.1 release metadata. --- CHANGELOG-rust.md | 3 ++ Cargo.lock | 2 +- Cargo.toml | 2 +- src/antiklepto.rs | 63 ++++++++++++++++++++++++++-- src/btc.rs | 2 +- src/eth.rs | 19 ++++++--- src/lib.rs | 1 + src/secp256k1.rs | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 183 insertions(+), 11 deletions(-) create mode 100644 src/secp256k1.rs diff --git a/CHANGELOG-rust.md b/CHANGELOG-rust.md index b0977c5..03070e2 100644 --- a/CHANGELOG-rust.md +++ b/CHANGELOG-rust.md @@ -1,7 +1,10 @@ # Changelog ## [Unreleased] + +## 0.13.1 - Restrict persisted Noise config files to private permissions on Unix. +- Validate ECDSA signatures and recovery IDs in Anti-Klepto and direct signing flows. ## 0.13.0 - Add `BitBox::from_transport()` diff --git a/Cargo.lock b/Cargo.lock index a4a0770..b37e2a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -126,7 +126,7 @@ checksum = "32637268377fc7b10a8c6d51de3e7fba1ce5dd371a96e342b34e6078db558e7f" [[package]] name = "bitbox-api" -version = "0.13.0" +version = "0.13.1" dependencies = [ "async-trait", "base32", diff --git a/Cargo.toml b/Cargo.toml index c4c7de3..b46eade 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "bitbox-api" authors = ["Marko Bencun "] -version = "0.13.0" +version = "0.13.1" homepage = "https://bitbox.swiss/" repository = "https://github.com/BitBoxSwiss/bitbox-api-rs/" readme = "README-rust.md" diff --git a/src/antiklepto.rs b/src/antiklepto.rs index 6414c19..4ff0adb 100644 --- a/src/antiklepto.rs +++ b/src/antiklepto.rs @@ -14,6 +14,8 @@ pub enum Error { GenNonce, #[error("{0}")] VerificationErr(&'static str), + #[error(transparent)] + InvalidSignature(#[from] crate::secp256k1::ValidationError), #[error( "Could not verify that the host nonce was contributed to the signature. \ If this happens repeatedly, the device might be attempting to leak the \ @@ -43,13 +45,32 @@ pub fn host_commit(host_nonce: &[u8]) -> [u8; 32] { tagged_sha256(b"s2c/ecdsa/data", host_nonce) } -/// antikleptoVerify verifies that hostNonce was used to tweak the nonce during signature -/// generation according to k' = k + H(clientCommitment, hostNonce) by checking that -/// k'*G = signerCommitment + H(signerCommitment, hostNonce)*G. +/// Verifies that `host_nonce` was used to tweak the nonce during signature generation according +/// to k' = k + H(signer_commitment, host_nonce) by checking that +/// k'*G = signer_commitment + H(signer_commitment, host_nonce)*G. pub fn verify_ecdsa( host_nonce: &[u8], signer_commitment: &[u8], signature: &[u8], +) -> Result<(), Error> { + crate::secp256k1::validate_signature_compact(signature)?; + verify_ecdsa_nonce(host_nonce, signer_commitment, signature) +} + +/// Validates a recoverable ECDSA signature and verifies its Anti-Klepto nonce contribution. +pub fn verify_recoverable_ecdsa( + host_nonce: &[u8], + signer_commitment: &[u8], + signature: &[u8], +) -> Result<(), Error> { + crate::secp256k1::validate_signature_recoverable(signature)?; + verify_ecdsa_nonce(host_nonce, signer_commitment, &signature[..64]) +} + +fn verify_ecdsa_nonce( + host_nonce: &[u8], + signer_commitment: &[u8], + signature: &[u8], ) -> Result<(), Error> { let secp = Secp256k1::new(); let signer_commitment_pubkey = PublicKey::from_slice(signer_commitment) @@ -143,4 +164,40 @@ mod tests { .is_err()); } } + + #[test] + fn test_verify_ecdsa_rejects_high_s() { + let unhex = |s| FromHex::from_hex(s).unwrap(); + let host_nonce: Vec = + unhex("8b4c26aa2695a34bdbc34235f6c91be14b93037a063b13f7c814101359561092"); + let signer_commitment: Vec = + unhex("0236ff92fe02c08d0d04851e0ce1516104085215f05a178307de60ea53e207f971"); + let low_s: Vec = unhex( + "7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8f47e01eca356377870356d54853573a955076228e5044cd3dd3a049abe70d5585", + ); + let high_s: Vec = unhex( + "7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8fb81fe135ca9c8878fca92ab7aca8c5696a38ba585f03d2fdec3214e0e928ebbc", + ); + + assert!(verify_ecdsa(&host_nonce, &signer_commitment, &low_s).is_ok()); + assert!(verify_ecdsa(&host_nonce, &signer_commitment, &high_s).is_err()); + } + + #[test] + fn test_verify_recoverable_ecdsa() { + let unhex = |s| FromHex::from_hex(s).unwrap(); + let host_nonce: Vec = + unhex("8b4c26aa2695a34bdbc34235f6c91be14b93037a063b13f7c814101359561092"); + let signer_commitment: Vec = + unhex("0236ff92fe02c08d0d04851e0ce1516104085215f05a178307de60ea53e207f971"); + let mut signature: Vec = unhex( + "7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8f47e01eca356377870356d54853573a955076228e5044cd3dd3a049abe70d558500", + ); + + assert!(verify_recoverable_ecdsa(&host_nonce, &signer_commitment, &signature).is_ok()); + signature[64] = 3; + assert!(verify_recoverable_ecdsa(&host_nonce, &signer_commitment, &signature).is_ok()); + signature[64] = 4; + assert!(verify_recoverable_ecdsa(&host_nonce, &signer_commitment, &signature).is_err()); + } } diff --git a/src/btc.rs b/src/btc.rs index de0cfee..24a0a90 100644 --- a/src/btc.rs +++ b/src/btc.rs @@ -1112,7 +1112,7 @@ impl PairedBitBox { } _ => return Err(Error::UnexpectedResponse), }; - crate::antiklepto::verify_ecdsa(&host_nonce, &signer_commitment, &signature)?; + crate::antiklepto::verify_recoverable_ecdsa(&host_nonce, &signer_commitment, &signature)?; let sig = signature[..64].to_vec(); let recid = signature[64]; diff --git a/src/eth.rs b/src/eth.rs index 2742583..25479e7 100644 --- a/src/eth.rs +++ b/src/eth.rs @@ -476,7 +476,11 @@ impl PairedBitBox { .await? { pb::eth_response::Response::Sign(pb::EthSignResponse { signature }) => { - crate::antiklepto::verify_ecdsa(&host_nonce, commitment, &signature)?; + crate::antiklepto::verify_recoverable_ecdsa( + &host_nonce, + commitment, + &signature, + )?; signature.try_into().map_err(|_| Error::UnexpectedResponse) } _ => Err(Error::UnexpectedResponse), @@ -733,10 +737,15 @@ impl PairedBitBox { .await? } else { match response { - pb::eth_response::Response::Sign(pb::EthSignResponse { signature }) => signature - .as_slice() - .try_into() - .map_err(|_| Error::UnexpectedResponse)?, + pb::eth_response::Response::Sign(pb::EthSignResponse { signature }) => { + let signature: [u8; 65] = signature + .as_slice() + .try_into() + .map_err(|_| Error::UnexpectedResponse)?; + crate::secp256k1::validate_signature_recoverable(&signature) + .map_err(|_| Error::InvalidSignature)?; + signature + } _ => return Err(Error::UnexpectedResponse), } }; diff --git a/src/lib.rs b/src/lib.rs index e3968e5..7c8de19 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,6 +22,7 @@ mod antiklepto; mod communication; mod constants; mod keypath; +mod secp256k1; mod u2fframing; mod util; diff --git a/src/secp256k1.rs b/src/secp256k1.rs new file mode 100644 index 0000000..ee77d81 --- /dev/null +++ b/src/secp256k1.rs @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 + +use bitcoin::secp256k1::ecdsa::Signature; +use thiserror::Error; + +/// An invalid ECDSA signature encoding. +#[derive(Debug, Error)] +#[error("{0}")] +pub struct ValidationError(&'static str); + +/// Validates a 64-byte compact ECDSA signature encoded as `r || s`. +/// +/// Both scalars must be nonzero and in range, and `s` must use its low-S encoding. +pub(crate) fn validate_signature_compact(signature: &[u8]) -> Result<(), ValidationError> { + if signature.len() != 64 { + return Err(ValidationError("Signature must be 64 bytes")); + } + + let parsed_signature = Signature::from_compact(signature) + .map_err(|_| ValidationError("Failed to parse ECDSA signature"))?; + if signature[..32].iter().all(|byte| *byte == 0) + || signature[32..].iter().all(|byte| *byte == 0) + { + return Err(ValidationError("Invalid ECDSA signature")); + } + let mut normalized_signature = parsed_signature; + normalized_signature.normalize_s(); + if normalized_signature != parsed_signature { + return Err(ValidationError("ECDSA signature has high S")); + } + Ok(()) +} + +/// Validates a 65-byte recoverable ECDSA signature encoded as `r || s || recovery_id`. +/// +/// The compact signature must be valid and the recovery ID must be in the range 0..=3. +pub(crate) fn validate_signature_recoverable(signature: &[u8]) -> Result<(), ValidationError> { + if signature.len() != 65 { + return Err(ValidationError("Signature must be 65 bytes")); + } + validate_signature_compact(&signature[..64])?; + if signature[64] > 3 { + return Err(ValidationError("Invalid recovery ID")); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use bitcoin::hashes::hex::FromHex; + + fn valid_signature() -> Vec { + FromHex::from_hex( + "7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8f47e01eca356377870356d54853573a955076228e5044cd3dd3a049abe70d5585", + ) + .unwrap() + } + + #[test] + fn test_validate_signature_compact() { + let signature = valid_signature(); + assert!(validate_signature_compact(&signature).is_ok()); + + let high_s: Vec = FromHex::from_hex( + "7fd66b48ffea2fe048869880bbb3a1819e262af14980e8885df1e5765750cb8fb81fe135ca9c8878fca92ab7aca8c5696a38ba585f03d2fdec3214e0e928ebbc", + ) + .unwrap(); + assert!(Signature::from_compact(&high_s).is_ok()); + assert!(validate_signature_compact(&high_s).is_err()); + + assert!(validate_signature_compact(&signature[..63]).is_err()); + let mut too_long = signature.clone(); + too_long.push(0); + assert!(validate_signature_compact(&too_long).is_err()); + + for offset in [0, 32] { + let mut zero = signature.clone(); + zero[offset..offset + 32].fill(0); + assert!(validate_signature_compact(&zero).is_err()); + + let mut out_of_range = signature.clone(); + out_of_range[offset..offset + 32] + .copy_from_slice(&bitcoin::secp256k1::constants::CURVE_ORDER); + assert!(validate_signature_compact(&out_of_range).is_err()); + } + } + + #[test] + fn test_validate_signature_recoverable() { + let mut signature = valid_signature(); + signature.push(0); + assert!(validate_signature_recoverable(&signature).is_ok()); + assert!(validate_signature_recoverable(&signature[..64]).is_err()); + + signature[64] = 3; + assert!(validate_signature_recoverable(&signature).is_ok()); + + signature[64] = 4; + assert!(validate_signature_recoverable(&signature).is_err()); + } +}