rust/keystore: wrap secp256k1_sign() in bitbox02-rust
What changed, and why it matters
This commit is a routine internal code reorganization. It moves the function that creates secp256k1 signatures from a low-level Rust-C binding module into a higher-level Rust keystore module, and renames the original function so the compiler catches any remaining callers. The actual signing behavior, cryptographic math, and security properties are unchanged.
No security action required. Treat as normal refactoring; standard code review and CI pass are sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change wraps bitbox02::keystore::secp256k1_sign() as bitbox02_rust::keystore::secp256k1_sign(), removing the Secp256k1 context argument from the public API and forwarding to the renamed _secp256k1_sign(). Call sites in Bitcoin and Ethereum signing paths are updated. The Anti-Klepto nonce derivation and signature output remain identical; the original function is renamed (not removed) so missed references become compile errors. A unit test is relocated and adapted to use the global SECP256K1 context.
Changed components
src/rust/bitbox02-rust/src/keystore.rssrc/rust/bitbox02/src/keystore.rsBitcoin message signingBitcoin transaction signingEthereum transaction signingEthereum message signingEthereum typed-data signingInspect captured patch +77 / −49
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs
index 8995fde..20692df 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs
@@ -119,8 +119,7 @@ pub async fn process(
None => [0; 32],
};
- let sign_result = bitbox02::keystore::secp256k1_sign(
- SECP256K1,
+ let sign_result = crate::keystore::secp256k1_sign(
crate::keystore::secp256k1_get_private_key(keypath)?
.as_slice()
.try_into()
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
index f74663a..1f69d56 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
@@ -1255,8 +1255,7 @@ async fn _process(
None => [0; 32],
};
- let sign_result = bitbox02::keystore::secp256k1_sign(
- SECP256K1,
+ let sign_result = crate::keystore::secp256k1_sign(
private_key.as_slice().try_into().unwrap(),
&sighash,
&host_nonce,
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
index 723aa39..4d33243 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
@@ -409,8 +409,7 @@ pub async fn _process(
// Return signature directly without the anti-klepto protocol, for backwards compatibility.
None => [0; 32],
};
- let sign_result = keystore::secp256k1_sign(
- SECP256K1,
+ let sign_result = crate::keystore::secp256k1_sign(
&crate::keystore::secp256k1_get_private_key(request.keypath())?
.as_slice()
.try_into()
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
index 2eb81f8..4386cd8 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
@@ -583,8 +583,7 @@ pub async fn process(
_ => return Err(Error::InvalidInput),
};
- let sign_result = bitbox02::keystore::secp256k1_sign(
- SECP256K1,
+ let sign_result = crate::keystore::secp256k1_sign(
crate::keystore::secp256k1_get_private_key(&request.keypath)?
.as_slice()
.try_into()
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
index 2b6cd91..ec3fd51 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
@@ -88,8 +88,7 @@ pub async fn process(
None => [0; 32],
};
- let sign_result = bitbox02::keystore::secp256k1_sign(
- SECP256K1,
+ let sign_result = crate::keystore::secp256k1_sign(
crate::keystore::secp256k1_get_private_key(&request.keypath)?
.as_slice()
.try_into()
diff --git a/src/rust/bitbox02-rust/src/keystore.rs b/src/rust/bitbox02-rust/src/keystore.rs
index 34e257e..5fb0150 100644
--- a/src/rust/bitbox02-rust/src/keystore.rs
+++ b/src/rust/bitbox02-rust/src/keystore.rs
@@ -19,6 +19,7 @@ use alloc::string::String;
use alloc::vec::Vec;
use crate::bip32;
+pub use bitbox02::keystore::SignResult;
use bitbox02::{keystore, securechip};
use util::bip32::HARDENED;
@@ -252,6 +253,37 @@ pub fn bip85_ln(index: u32) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
Ok(entropy)
}
+/// Sign message with private key using the given private key.
+///
+/// Details about `host_nonce`, the host nonce contribution. Instead of using plain rfc6979 to
+/// generate the nonce in this signature, the following formula is used:
+///
+/// r = rfc6979(..., additional_data=Hash_d(host_nonce))
+/// R = r * G (pubkey to secret r)
+/// nonce = r + Hash_p(R, host_nonce)
+/// `Hash_d(msg)` and `Hash_p(msg)` are tagged hashes: `sha256(sha256(tag)||sha256(tag)||msg)`.
+/// Tag for `Hash_d`: "s2c/ecdsa/data".
+/// Tag for `Hash_p`: "s2c/ecdsa/point".
+/// This is part of the ECDSA Anti-Klepto protocol, preventing this function to leak any secrets via
+/// the signatures (see the ecdsa-s2c module in secp256k1-zpk for more details).
+///
+/// # Arguments
+/// * `private_key` - 32 byte private key
+/// * `msg` - 32 byte message to sign
+/// * `host_nonce` - 32 byte nonce contribution. Cannot be NULL.
+/// Intended to be a contribution by the host. If there is none available, use 32 zero bytes.
+///
+/// # Returns
+/// * `Ok(SignResult)` containing signature in compact format and recoverable id on success
+/// * `Err(())` if the keystore is locked
+pub fn secp256k1_sign(
+ private_key: &[u8; 32],
+ msg: &[u8; 32],
+ host_nonce: &[u8; 32],
+) -> Result<SignResult, ()> {
+ keystore::_secp256k1_sign(SECP256K1, private_key, msg, host_nonce)
+}
+
/// Sign a message using the private key at the keypath, which is optionally tweaked with the given
/// tweak.
pub fn secp256k1_schnorr_sign(
@@ -776,6 +808,45 @@ mod tests {
}
}
+ #[test]
+ fn test_secp256k1_sign() {
+ let private_key =
+ hex::decode("a2d8cf543c60d65162b5a06f0cef9760c883f8aa09f31236859faa85d0b74c7c")
+ .unwrap();
+ let msg = [0x88u8; 32];
+ let host_nonce = [0x56u8; 32];
+
+ let sign_result =
+ secp256k1_sign(&private_key.try_into().unwrap(), &msg, &host_nonce).unwrap();
+
+ // Verify signature against expected pubkey.
+
+ let expected_pubkey = {
+ let pubkey =
+ hex::decode("023ffb4a4e41444d40e4e1e4c6cc329bcba2be50d0ef380aea19d490c373be58fb")
+ .unwrap();
+ secp256k1::PublicKey::from_slice(&pubkey).unwrap()
+ };
+ let msg = secp256k1::Message::from_digest_slice(&msg).unwrap();
+ // Test recid by recovering the public key from the signature and checking against the
+ // expected public key.
+ let recoverable_sig = secp256k1::ecdsa::RecoverableSignature::from_compact(
+ &sign_result.signature,
+ secp256k1::ecdsa::RecoveryId::from_i32(sign_result.recid as i32).unwrap(),
+ )
+ .unwrap();
+
+ let recovered_pubkey = SECP256K1.recover_ecdsa(&msg, &recoverable_sig).unwrap();
+ assert_eq!(recovered_pubkey, expected_pubkey);
+
+ // Verify signature.
+ assert!(
+ SECP256K1
+ .verify_ecdsa(&msg, &recoverable_sig.to_standard(), &expected_pubkey)
+ .is_ok()
+ );
+ }
+
#[test]
fn test_secp256k1_schnorr_sign() {
mock_unlocked_using_mnemonic(
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
index 107c9ab..23a2141 100644
--- a/src/rust/bitbox02/src/keystore.rs
+++ b/src/rust/bitbox02/src/keystore.rs
@@ -223,7 +223,7 @@ pub struct SignResult {
pub recid: u8,
}
-pub fn secp256k1_sign(
+pub fn _secp256k1_sign(
secp: &Secp256k1<All>,
private_key: &[u8; 32],
msg: &[u8; 32],
@@ -301,44 +301,6 @@ mod tests {
use crate::testing::{mock_memory, mock_unlocked_using_mnemonic};
use util::bb02_async::block_on;
- #[test]
- fn test_secp256k1_sign() {
- let private_key =
- hex::decode("a2d8cf543c60d65162b5a06f0cef9760c883f8aa09f31236859faa85d0b74c7c")
- .unwrap();
- let msg = [0x88u8; 32];
- let host_nonce = [0x56u8; 32];
-
- let secp = secp256k1::Secp256k1::new();
- let sign_result =
- secp256k1_sign(&secp, &private_key.try_into().unwrap(), &msg, &host_nonce).unwrap();
- // Verify signature against expected pubkey.
-
- let expected_pubkey = {
- let pubkey =
- hex::decode("023ffb4a4e41444d40e4e1e4c6cc329bcba2be50d0ef380aea19d490c373be58fb")
- .unwrap();
- secp256k1::PublicKey::from_slice(&pubkey).unwrap()
- };
- let msg = secp256k1::Message::from_digest_slice(&msg).unwrap();
- // Test recid by recovering the public key from the signature and checking against the
- // expected public key.
- let recoverable_sig = secp256k1::ecdsa::RecoverableSignature::from_compact(
- &sign_result.signature,
- secp256k1::ecdsa::RecoveryId::from_i32(sign_result.recid as i32).unwrap(),
- )
- .unwrap();
-
- let recovered_pubkey = secp.recover_ecdsa(&msg, &recoverable_sig).unwrap();
- assert_eq!(recovered_pubkey, expected_pubkey);
-
- // Verify signature.
- assert!(
- secp.verify_ecdsa(&msg, &recoverable_sig.to_standard(), &expected_pubkey)
- .is_ok()
- );
- }
-
#[test]
fn test_secp256k1_nonce_commit() {
let secp = secp256k1::Secp256k1::new();
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.