diff --git a/src/keystore.c b/src/keystore.c index ca4e066d9..83d206cb7 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 eabcb54fa..268338917 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 9a9a2aafc..cad89869a 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 = 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 ceb201192..504bf767d 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 901ad4296..01d68a687 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 = 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 1fe9e4edc..cc818005a 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 = 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 9d626ae6e..6cdeb7509 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 = 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 12880cbfe..5acd5616d 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 { @@ -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 6858fc861..c3d6010fe 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;