← Watch feed
keystore: pass private key to nonce_commit() and sign()
Public commit record
78/100 · Adequate
What the developer wrote
Authored by Marko Bencun
keystore: pass private key to nonce_commit() and sign()
End-goal: reduce the number of secure chip ops when signing a BTC
transaction, to reduce the chance of going over the Optiga chip's
"rate limit", which induces throttling.
With antiklepto, we derived the private key twice for each input that
is signed: once to commit to the nonce, and after that to sign.
This commit decouples the nonce commit and sign functions from the
underlying keystore, and allows reusing a private key instead of
re-deriving it, which requires secure chip operations.
This halves the number of secure chip ops needed per input when
signing a BTC transaction.
We do not reuse the privkey for the other instances of
antiklepto (signing a msg, signing an ETH tx), as there it's one
commit/sign pair only and unlikely to cause secure chip throttling.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit is a performance optimization for the BitBox02 hardware wallet. It changes how Bitcoin transaction signing uses the secure chip, so the device only derives a private key once per transaction input instead of twice. This reduces the chance of hitting the secure chip's rate limit and avoids throttling. The change does not appear to introduce a security flaw; it is a refactoring to reuse an already-derived key for two related signing steps.
Recommended action
No immediate action required. Reviewers should verify that all callers of keystore_secp256k1_sign() and keystore_secp256k1_nonce_commit() now perform keystore unlock checks and derive the private key securely before calling, and that the private key is zeroized promptly after use (e.g., the bitcoin/signtx.rs caller uses drop(private_key)).
Security signals we found
01
Removal of keystore_is_locked() check from keystore_secp256k1_sign()
02
Private key now passed by pointer into signing/nonce-commit functions instead of being derived inside them
03
Callers now responsible for key derivation and lock-state validation
04
Performance optimization to reduce Optiga secure chip operations, not a cryptographic change
Evidence from the diff
The commit refactors keystore_secp256k1_nonce_commit() and keystore_secp256k1_sign() to accept a 32-byte private key instead of a BIP32 keypath. Previously, each function derived the private key internally via rust_secp256k1_get_private_key(), which involves operations on the Optiga secure chip. With the Anti-Klepto protocol, both functions were called per input, causing two secure chip derivations per input. The new code derives the key once in the caller (e.g., bitcoin/signtx.rs) and passes it to both functions, halving secure chip operations for Bitcoin transaction signing. The C functions no longer check keystore_is_locked() because key derivation is now the caller’s responsibility. The Rust callers for message/ETH signing still derive twice because they only have one commit/sign pair each and are not throttling-sensitive. Unit tests were updated to pass raw private keys instead of keypaths.
Changed components
src/keystore.csrc/keystore.hsrc/rust/bitbox02/src/keystore.rssrc/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rssrc/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rssrc/rust/bitbox02-rust/src/hww/api/ethereum/sign.rssrc/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rssrc/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rstest/unit-test/test_keystore_antiklepto.cInspect captured patch +82 / −68
diff --git a/src/keystore.c b/src/keystore.c
index ca4e066d..83d206cb 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -528,19 +528,11 @@ bool keystore_get_bip39_word(uint16_t idx, char** word_out)
}
bool keystore_secp256k1_nonce_commit(
- const uint32_t* keypath,
- size_t keypath_len,
+ const uint8_t* private_key,
const uint8_t* msg32,
const uint8_t* host_commitment,
uint8_t* signer_commitment_out)
{
- uint8_t private_key[32] = {0};
- UTIL_CLEANUP_32(private_key);
- if (!rust_secp256k1_get_private_key(
- keypath, keypath_len, rust_util_bytes_mut(private_key, sizeof(private_key)))) {
- return false;
- }
-
const secp256k1_context* ctx = wally_get_secp_context();
secp256k1_ecdsa_s2c_opening signer_commitment;
if (!secp256k1_ecdsa_anti_exfil_signer_commit(
@@ -555,23 +547,12 @@ bool keystore_secp256k1_nonce_commit(
}
bool keystore_secp256k1_sign(
- const uint32_t* keypath,
- size_t keypath_len,
+ const uint8_t* private_key,
const uint8_t* msg32,
const uint8_t* host_nonce32,
uint8_t* sig_compact_out,
int* recid_out)
{
- if (keystore_is_locked()) {
- return false;
- }
- uint8_t private_key[32] = {0};
- UTIL_CLEANUP_32(private_key);
- if (!rust_secp256k1_get_private_key(
- keypath, keypath_len, rust_util_bytes_mut(private_key, sizeof(private_key)))) {
- return false;
- }
-
const secp256k1_context* ctx = wally_get_secp_context();
secp256k1_ecdsa_signature secp256k1_sig = {0};
if (!secp256k1_anti_exfil_sign(
diff --git a/src/keystore.h b/src/keystore.h
index eabcb54f..26833891 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -157,8 +157,7 @@ USE_RESULT bool keystore_get_bip39_word(uint16_t idx, char** word_out);
* Get a commitment to the original nonce before tweaking it with the host nonce. This is part of
* the ECDSA Anti-Klepto Protocol. For more details, check the docs of
* `secp256k1_ecdsa_anti_exfil_signer_commit`.
- * @param[in] keypath derivation keypath
- * @param[in] keypath_len size of keypath buffer
+ * @param[in] private_key 32 byte private key
* @param[in] msg32 32 byte message which will be signed by `keystore_secp256k1_sign`.
* @param[in] host_commitment must be `sha256(sha256(tag)||shas256(tag)||host_nonce)` where
* host_nonce is passed to `keystore_secp256k1_sign()`. See
@@ -166,15 +165,14 @@ USE_RESULT bool keystore_get_bip39_word(uint16_t idx, char** word_out);
* @param[out] client_commitment_out EC_PUBLIC_KEY_LEN bytes compressed signer nonce pubkey.
*/
USE_RESULT bool keystore_secp256k1_nonce_commit(
- const uint32_t* keypath,
- size_t keypath_len,
+ const uint8_t* private_key,
const uint8_t* msg32,
const uint8_t* host_commitment,
uint8_t* client_commitment_out);
// clang-format off
/**
- * Sign message with private key at the given keypath. Keystore must be unlocked.
+ * Sign message with private key using the given private key.
*
* Details about `host_nonce32`, the host nonce contribution.
* Instead of using plain rfc6979 to generate the nonce in this signature, the following formula is used:
@@ -187,8 +185,7 @@ USE_RESULT bool keystore_secp256k1_nonce_commit(
* This is part of the ECSDA Anti-Klepto protocol, preventing this function to leak any secrets via
* the signatures (see the ecdsa-s2c module in secp256k1-zpk for more details).
*
- * @param[in] keypath derivation keypath
- * @param[in] keypath_len size of keypath buffer
+ * @param[in] private_key 32 byte private key
* @param[in] msg32 32 byte message to sign
* @param[in] host_nonce32 32 byte nonce contribution. Cannot be NULL.
* Intended to be a contribution by the host. If there is none available, use 32 zero bytes.
@@ -199,8 +196,7 @@ USE_RESULT bool keystore_secp256k1_nonce_commit(
*/
// clang-format on
USE_RESULT bool keystore_secp256k1_sign(
- const uint32_t* keypath,
- size_t keypath_len,
+ const uint8_t* private_key,
const uint8_t* msg32,
const uint8_t* host_nonce32,
uint8_t* sig_compact_out,
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 9a9a2aaf..cad89869 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signmsg.rs
@@ -98,7 +98,10 @@ pub async fn process(
// Engage in the anti-klepto protocol if the host sends a host nonce commitment.
Some(pb::AntiKleptoHostNonceCommitment { ref commitment }) => {
let signer_commitment = keystore::secp256k1_nonce_commit(
- keypath,
+ crate::keystore::secp256k1_get_private_key(keypath)?
+ .as_slice()
+ .try_into()
+ .unwrap(),
&sighash,
commitment
.as_slice()
@@ -114,8 +117,14 @@ pub async fn process(
None => [0; 32],
};
- let sign_result = bitbox02::keystore::secp256k1_sign(keypath, &sighash, &host_nonce)?;
-
+ let sign_result = bitbox02::keystore::secp256k1_sign(
+ crate::keystore::secp256k1_get_private_key(keypath)?
+ .as_slice()
+ .try_into()
+ .unwrap(),
+ &sighash,
+ &host_nonce,
+ )?;
let mut signature: Vec<u8> = sign_result.signature.to_vec();
signature.push(sign_result.recid);
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 ceb20119..504bf767 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
@@ -1203,11 +1203,12 @@ async fn _process(
sighash_flags: SIGHASH_ALL,
});
+ let private_key = crate::keystore::secp256k1_get_private_key(&tx_input.keypath)?;
// Engage in the Anti-Klepto protocol if the host sends a host nonce commitment.
let host_nonce: [u8; 32] = match tx_input.host_nonce_commitment {
Some(pb::AntiKleptoHostNonceCommitment { ref commitment }) => {
let signer_commitment = bitbox02::keystore::secp256k1_nonce_commit(
- &tx_input.keypath,
+ private_key.as_slice().try_into().unwrap(),
&sighash,
commitment
.as_slice()
@@ -1230,8 +1231,12 @@ async fn _process(
None => [0; 32],
};
- let sign_result =
- bitbox02::keystore::secp256k1_sign(&tx_input.keypath, &sighash, &host_nonce)?;
+ let sign_result = bitbox02::keystore::secp256k1_sign(
+ private_key.as_slice().try_into().unwrap(),
+ &sighash,
+ &host_nonce,
+ )?;
+ drop(private_key);
next_response.next.has_signature = true;
next_response.next.signature = sign_result.signature.to_vec();
}
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 901ad429..01d68a68 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
@@ -389,7 +389,10 @@ pub async fn _process(
// Engage in the anti-klepto protocol if the host sends a host nonce commitment.
Some(pb::AntiKleptoHostNonceCommitment { ref commitment }) => {
let signer_commitment = keystore::secp256k1_nonce_commit(
- request.keypath(),
+ &crate::keystore::secp256k1_get_private_key(request.keypath())?
+ .as_slice()
+ .try_into()
+ .unwrap(),
&hash,
commitment
.as_slice()
@@ -404,8 +407,14 @@ pub async fn _process(
// Return signature directly without the anti-klepto protocol, for backwards compatibility.
None => [0; 32],
};
- let sign_result = keystore::secp256k1_sign(request.keypath(), &hash, &host_nonce)?;
-
+ let sign_result = keystore::secp256k1_sign(
+ &crate::keystore::secp256k1_get_private_key(request.keypath())?
+ .as_slice()
+ .try_into()
+ .unwrap(),
+ &hash,
+ &host_nonce,
+ )?;
let mut signature: Vec<u8> = sign_result.signature.to_vec();
signature.push(sign_result.recid);
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 1fe9e4ed..cc818005 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
@@ -563,7 +563,10 @@ pub async fn process(
let host_nonce = match request.host_nonce_commitment {
Some(pb::AntiKleptoHostNonceCommitment { ref commitment }) => {
let signer_commitment = keystore::secp256k1_nonce_commit(
- &request.keypath,
+ crate::keystore::secp256k1_get_private_key(&request.keypath)?
+ .as_slice()
+ .try_into()
+ .unwrap(),
&sighash,
commitment
.as_slice()
@@ -578,8 +581,14 @@ pub async fn process(
_ => return Err(Error::InvalidInput),
};
- let sign_result = bitbox02::keystore::secp256k1_sign(&request.keypath, &sighash, &host_nonce)?;
-
+ let sign_result = bitbox02::keystore::secp256k1_sign(
+ crate::keystore::secp256k1_get_private_key(&request.keypath)?
+ .as_slice()
+ .try_into()
+ .unwrap(),
+ &sighash,
+ &host_nonce,
+ )?;
let mut signature: Vec<u8> = sign_result.signature.to_vec();
signature.push(sign_result.recid);
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 9d626ae6..6cdeb750 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
@@ -67,7 +67,10 @@ pub async fn process(
// Engage in the anti-klepto protocol if the host sends a host nonce commitment.
Some(pb::AntiKleptoHostNonceCommitment { ref commitment }) => {
let signer_commitment = keystore::secp256k1_nonce_commit(
- &request.keypath,
+ crate::keystore::secp256k1_get_private_key(&request.keypath)?
+ .as_slice()
+ .try_into()
+ .unwrap(),
&sighash,
commitment
.as_slice()
@@ -83,8 +86,14 @@ pub async fn process(
None => [0; 32],
};
- let sign_result = bitbox02::keystore::secp256k1_sign(&request.keypath, &sighash, &host_nonce)?;
-
+ let sign_result = bitbox02::keystore::secp256k1_sign(
+ crate::keystore::secp256k1_get_private_key(&request.keypath)?
+ .as_slice()
+ .try_into()
+ .unwrap(),
+ &sighash,
+ &host_nonce,
+ )?;
let mut signature: Vec<u8> = sign_result.signature.to_vec();
signature.push(sign_result.recid);
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
index 12880cbf..5acd5616 100644
--- a/src/rust/bitbox02/src/keystore.rs
+++ b/src/rust/bitbox02/src/keystore.rs
@@ -218,7 +218,7 @@ pub struct SignResult {
}
pub fn secp256k1_sign(
- keypath: &[u32],
+ private_key: &[u8; 32],
msg: &[u8; 32],
host_nonce: &[u8; 32],
) -> Result<SignResult, ()> {
@@ -226,8 +226,7 @@ pub fn secp256k1_sign(
let mut recid: util::c_types::c_int = 0;
match unsafe {
bitbox02_sys::keystore_secp256k1_sign(
- keypath.as_ptr(),
- keypath.len() as _,
+ private_key.as_ptr(),
msg.as_ptr(),
host_nonce.as_ptr(),
signature.as_mut_ptr(),
@@ -243,15 +242,14 @@ pub fn secp256k1_sign(
}
pub fn secp256k1_nonce_commit(
- keypath: &[u32],
+ private_key: &[u8; 32],
msg: &[u8; 32],
host_commitment: &[u8; 32],
) -> Result<[u8; EC_PUBLIC_KEY_LEN], ()> {
let mut signer_commitment = [0u8; EC_PUBLIC_KEY_LEN];
match unsafe {
bitbox02_sys::keystore_secp256k1_nonce_commit(
- keypath.as_ptr(),
- keypath.len() as _,
+ private_key.as_ptr(),
msg.as_ptr(),
host_commitment.as_ptr(),
signer_commitment.as_mut_ptr(),
@@ -338,21 +336,19 @@ mod tests {
use super::*;
use bitcoin::secp256k1;
- use crate::testing::{mock_memory, mock_unlocked, mock_unlocked_using_mnemonic};
+ use crate::testing::{mock_memory, mock_unlocked_using_mnemonic};
use util::bip32::HARDENED;
#[test]
fn test_secp256k1_sign() {
- lock();
- let keypath = [44 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 5];
+ let private_key =
+ hex::decode("a2d8cf543c60d65162b5a06f0cef9760c883f8aa09f31236859faa85d0b74c7c")
+ .unwrap();
let msg = [0x88u8; 32];
let host_nonce = [0x56u8; 32];
- // Fails because keystore is locked.
- assert!(secp256k1_sign(&keypath, &msg, &host_nonce).is_err());
-
- mock_unlocked();
- let sign_result = secp256k1_sign(&keypath, &msg, &host_nonce).unwrap();
+ let sign_result =
+ secp256k1_sign(&private_key.try_into().unwrap(), &msg, &host_nonce).unwrap();
// Verify signature against expected pubkey.
let secp = secp256k1::Secp256k1::new();
@@ -426,16 +422,15 @@ mod tests {
#[test]
fn test_secp256k1_nonce_commit() {
- lock();
- let keypath = [44 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 5];
+ let private_key =
+ hex::decode("a2d8cf543c60d65162b5a06f0cef9760c883f8aa09f31236859faa85d0b74c7c")
+ .unwrap();
let msg = [0x88u8; 32];
let host_commitment = [0xabu8; 32];
- // Fails because keystore is locked.
- assert!(secp256k1_nonce_commit(&keypath, &msg, &host_commitment).is_err());
-
- mock_unlocked();
- let client_commitment = secp256k1_nonce_commit(&keypath, &msg, &host_commitment).unwrap();
+ let client_commitment =
+ secp256k1_nonce_commit(&private_key.try_into().unwrap(), &msg, &host_commitment)
+ .unwrap();
assert_eq!(
hex::encode(client_commitment),
"0381e4136251c87f2947b735159c6dd644a7b58d35b437e20c878e5129f1320e5e",
diff --git a/test/unit-test/test_keystore_antiklepto.c b/test/unit-test/test_keystore_antiklepto.c
index 6858fc86..c3d6010f 100644
--- a/test/unit-test/test_keystore_antiklepto.c
+++ b/test/unit-test/test_keystore_antiklepto.c
@@ -93,10 +93,11 @@ static void _test_keystore_antiklepto(void** state)
// Commit - protocol step 2.
assert_true(keystore_secp256k1_nonce_commit(
- keypath, 5, msg, host_nonce_commitment, signer_commitment));
+ xprv_derived.priv_key + 1, msg, host_nonce_commitment, signer_commitment));
// Protocol step 3: host_nonce sent from host to signer to be used in step 4
// Sign - protocol step 4.
- assert_true(keystore_secp256k1_sign(keypath, 5, msg, host_nonce, sig, &recid));
+ assert_true(
+ keystore_secp256k1_sign(xprv_derived.priv_key + 1, msg, host_nonce, sig, &recid));
// Protocol step 5: host verification.
secp256k1_ecdsa_signature parsed_signature;
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.