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.

Author: benma

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(

src/keystore.h

@@ -157,24 +157,22 @@ 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
  * `secp256k1_ecdsa_anti_exfil_host_commit()`.
  * @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,

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);
 

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();
         }

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);
 

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);
 

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);
 

src/rust/bitbox02/src/keystore.rs

@@ -218,16 +218,15 @@ pub struct SignResult {
 }
 
 pub fn secp256k1_sign(
-    keypath: &[u32],
+    private_key: &[u8; 32],
     msg: &[u8; 32],
     host_nonce: &[u8; 32],
 ) -> Result<SignResult, ()> {
     let mut signature = [0u8; 64];
     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",

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;