Merge branch 'reduce-sc-ops'

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;