Merge pull request #276 from LedgerHQ/perf-opts

Performance optimizations

src/common/wallet.c

@@ -424,7 +424,12 @@ int parse_policy_map_key_info(buffer_t *buffer, policy_map_key_info_t *out, int
     return 0;
 }
 
-static int parse_placeholder(buffer_t *in_buf, int version, policy_node_key_placeholder_t *out) {
+// parses a placeholder from in_buf, storing it in out. On success, the pointed placeholder_index is
+// stored in out->placeholder_index, and then it's incremented.
+static int parse_placeholder(buffer_t *in_buf,
+                             int version,
+                             policy_node_key_placeholder_t *out,
+                             uint16_t *placeholder_index) {
     char c;
     if (!buffer_read_u8(in_buf, (uint8_t *) &c) || c != '@') {
         return WITH_ERROR(-1, "Expected key placeholder starting with '@'");
@@ -489,6 +494,8 @@ static int parse_placeholder(buffer_t *in_buf, int version, policy_node_key_plac
         return WITH_ERROR(-1, "Invalid version number");
     }
 
+    out->placeholder_index = *placeholder_index;
+    ++(*placeholder_index);
     return 0;
 }
 
@@ -544,6 +551,15 @@ static int parse_script(buffer_t *in_buf,
                         unsigned int context_flags) {
     int n_wrappers = 0;
 
+    // Keep track of how many key placeholders have been created while parsing
+    // This allows to know the counter even in recursive calls
+    static uint16_t key_placeholder_count = 0;
+
+    if (depth == 0) {
+        // reset the counter on function entry, but not in recursive calls
+        key_placeholder_count = 0;
+    }
+
     policy_node_t *outermost_node = (policy_node_t *) buffer_get_cur(out_buf);
     policy_node_with_script_t *inner_wrapper = NULL;  // pointer to the inner wrapper, if any
 
@@ -1396,7 +1412,7 @@ static int parse_script(buffer_t *in_buf,
 
             node->base.type = token;
 
-            if (0 > parse_placeholder(in_buf, version, key_placeholder)) {
+            if (0 > parse_placeholder(in_buf, version, key_placeholder, &key_placeholder_count)) {
                 return WITH_ERROR(-1, "Couldn't parse key placeholder");
             }
 
@@ -1466,7 +1482,7 @@ static int parse_script(buffer_t *in_buf,
             }
             i_policy_node_key_placeholder(&node->key_placeholder, key_placeholder);
 
-            if (0 > parse_placeholder(in_buf, version, key_placeholder)) {
+            if (0 > parse_placeholder(in_buf, version, key_placeholder, &key_placeholder_count)) {
                 return WITH_ERROR(-1, "Couldn't parse key placeholder");
             }
 
@@ -1606,7 +1622,8 @@ static int parse_script(buffer_t *in_buf,
                     return WITH_ERROR(-1, "Out of memory");
                 }
 
-                if (0 > parse_placeholder(in_buf, version, key_placeholder)) {
+                if (0 >
+                    parse_placeholder(in_buf, version, key_placeholder, &key_placeholder_count)) {
                     return WITH_ERROR(-1, "Error parsing key placeholder");
                 }
 

src/common/wallet.h

@@ -300,6 +300,8 @@ typedef struct {
 
     // common between V1 and V2
     int16_t key_index;  // index of the key
+    int16_t
+        placeholder_index;  // index of the placeholder in the descriptor template, in parsing order
 } policy_node_key_placeholder_t;
 
 DEFINE_REL_PTR(policy_node_key_placeholder, policy_node_key_placeholder_t)

src/crypto.c

@@ -78,6 +78,28 @@ static const uint8_t BIP0341_taptweak_tag[] = {'T', 'a', 'p', 'T', 'w', 'e', 'a'
 static const uint8_t BIP0341_tapbranch_tag[] = {'T', 'a', 'p', 'B', 'r', 'a', 'n', 'c', 'h'};
 static const uint8_t BIP0341_tapleaf_tag[] = {'T', 'a', 'p', 'L', 'e', 'a', 'f'};
 
+// Copy of cx_ecfp_scalar_mult_no_throw, but without using randomization for the scalar
+// multiplication. Therefore, it is faster, but not safe to use on private data, as it is vulnerable
+// to timing attacks.
+cx_err_t cx_ecfp_scalar_mult_unsafe(cx_curve_t curve, uint8_t *P, const uint8_t *k, size_t k_len) {
+    size_t size;
+    cx_ecpoint_t ecP;
+    cx_err_t error;
+
+    CX_CHECK(cx_ecdomain_parameters_length(curve, &size));
+    CX_CHECK(cx_bn_lock(size, 0));
+
+    CX_CHECK(cx_ecpoint_alloc(&ecP, curve));
+    CX_CHECK(cx_ecpoint_init(&ecP, P + 1, size, P + 1 + size, size));
+    CX_CHECK(cx_ecpoint_scalarmul(&ecP, k, k_len));
+    P[0] = 0x04;
+    CX_CHECK(cx_ecpoint_export(&ecP, &P[1], size, &P[1 + size], size));
+
+end:
+    cx_bn_unlock();
+    return error;
+}
+
 /**
  * Gets the point on the SECP256K1 that corresponds to kG, where G is the curve's generator point.
  * Returns -1 if point is Infinity or any error occurs; 0 otherwise.
@@ -88,6 +110,16 @@ static int secp256k1_point(const uint8_t k[static 32], uint8_t out[static 65]) {
     return 0;
 }
 
+/**
+ * Equivalent to secp256k1_point, but it does not use randomization; it is faster, but only to be
+ * used with public data, as it is vulnerable to timing attacks.
+ */
+static int secp256k1_point_unsafe(const uint8_t k[static 32], uint8_t out[static 65]) {
+    memcpy(out, secp256k1_generator, 65);
+    if (CX_OK != cx_ecfp_scalar_mult_unsafe(CX_CURVE_SECP256K1, out, k, 32)) return -1;
+    return 0;
+}
+
 int bip32_CKDpub(const serialized_extended_pubkey_t *parent,
                  uint32_t index,
                  serialized_extended_pubkey_t *child) {
@@ -126,7 +158,9 @@ int bip32_CKDpub(const serialized_extended_pubkey_t *parent,
     {  // make sure that heavy memory allocations are freed as soon as possible
         // compute point(I_L)
         uint8_t P[65];
-        if (0 > secp256k1_point(I_L, P)) return -1;
+        // as the arguments of bip32_CKDpub are public keys, we do not need to use math functions
+        // hardened against side channels attacks, which are slower
+        if (0 > secp256k1_point_unsafe(I_L, P)) return -1;
 
         uint8_t K_par[65];
         crypto_get_uncompressed_pubkey(parent->compressed_pubkey, K_par);
@@ -543,7 +577,9 @@ int crypto_tr_tweak_pubkey(const uint8_t pubkey[static 32],
         return -1;
     }
 
-    if (0 > secp256k1_point(t, Q)) {
+    // as the arguments of bip32_CKDpub are public keys, we do not need to use math functions
+    // hardened against side channels attacks, which are slower
+    if (0 > secp256k1_point_unsafe(t, Q)) {
         // point at infinity, or error
         return -1;
     }

src/handler/lib/policy.c

@@ -469,10 +469,16 @@ __attribute__((warn_unused_result)) static int get_derived_pubkey(
 
     // we derive the /<change>/<address_index> child of this pubkey
     // we reuse the same memory of ext_pubkey
-    bip32_CKDpub(&ext_pubkey,
-                 wdi->change ? key_placeholder->num_second : key_placeholder->num_first,
-                 &ext_pubkey);
-    bip32_CKDpub(&ext_pubkey, wdi->address_index, &ext_pubkey);
+    if (0 > derive_first_step_for_pubkey(&ext_pubkey,
+                                         key_placeholder,
+                                         wdi->sign_psbt_cache,
+                                         wdi->change,
+                                         &ext_pubkey)) {
+        return -1;
+    }
+    if (0 > bip32_CKDpub(&ext_pubkey, wdi->address_index, &ext_pubkey)) {
+        return -1;
+    }
 
     memcpy(out, ext_pubkey.compressed_pubkey, 33);
 

src/handler/lib/policy.h

@@ -2,6 +2,7 @@
 
 #include "../../boilerplate/dispatcher.h"
 #include "../../common/wallet.h"
+#include "../../handler/sign_psbt/sign_psbt_cache.h"
 
 /**
  * Parses a serialized wallet policy, saving the wallet header, the policy map descriptor and the
@@ -48,6 +49,8 @@ typedef struct {
     uint32_t n_keys;        // The number of key information placeholders in the policy
     size_t address_index;   // The address index to use in the derivation
     bool change;            // whether a change address or a receive address is derived
+    sign_psbt_cache_t
+        *sign_psbt_cache;  // If not NULL, the cache for key derivations used during signing
 } wallet_derivation_info_t;
 
 /**