fix: constant-time hardening and input validation across crypto primitives

demos-go/cb-mpc-go/internal/cgobinding/network.cpp

@@ -135,10 +135,19 @@ class callback_data_transport_t : public data_transport_interface_t {
       c_senders.push_back(sender);
     }
 
-    cmems_t cmsgs;
-    int result = callbacks.receive_all_fun(go_impl_ptr, const_cast<int*>(c_senders.data()), n, &cmsgs);
-    msgs = coinbase::ffi::bufs_from_cmems(cmsgs);
+    // Ensure cmsgs is initialized so that error paths never leave us with
+    // uninitialized pointers/count.
+    cmems_t cmsgs{0, nullptr, nullptr};
+    const int result = callbacks.receive_all_fun(go_impl_ptr, const_cast<int*>(c_senders.data()), n, &cmsgs);
+    if (error_t rv = error_t(result)) {
+      msgs.clear();
+      return rv;
+    }
 
+    // Copy results out of the cgo-owned buffers, then free them (Go side uses C.malloc).
+    msgs = coinbase::ffi::bufs_from_cmems(cmsgs);
+    cgo_free(cmsgs.data);
+    cgo_free(cmsgs.sizes);
     return SUCCESS;
   }
 };

src/cbmpc/core/utils.h

@@ -12,6 +12,7 @@ static std::mutex coutMutex;
   } while (0)
 #endif
 
+#include <cbmpc/core/buf.h>
 #include <cbmpc/core/macros.h>
 
 namespace coinbase {
@@ -112,4 +113,25 @@ static inline uint64_t constant_time_select_u64(bool flag, uint64_t y, uint64_t
   return MASKED_SELECT(mask, y, z);
 }
 
+// Constant-time selection for a byte array.
+// Writes `y` to `out` when flag == true, else writes `z` to `out`.
+static inline void constant_time_select_bytes(bool flag, const_byte_ptr y, const_byte_ptr z, byte_ptr out, int size) {
+  // `size` is public; `flag` is assumed potentially secret.
+  cb_assert(size >= 0);
+  uint8_t mask = static_cast<uint8_t>(constant_time_mask_64(static_cast<uint64_t>(flag)));
+  uint8_t inv_mask = static_cast<uint8_t>(~mask);
+  for (int i = 0; i < size; i++) {
+    out[i] = static_cast<uint8_t>((y[i] & mask) | (z[i] & inv_mask));
+  }
+}
+
+// Constant-time selection between two same-sized buffers.
+// Returns `y` when flag == true, else returns `z`.
+static inline buf_t ct_select_buf(bool flag, mem_t y, mem_t z) {
+  cb_assert(y.size == z.size);
+  buf_t out(y.size);
+  constant_time_select_bytes(flag, y.data, z.data, out.data(), y.size);
+  return out;
+}
+
 }  // namespace coinbase

src/cbmpc/crypto/base_bn.cpp

@@ -651,7 +651,9 @@ static int bn_cmp_ct(const BIGNUM& a, const BIGNUM& b) {
     lt |= xlt;
     gt |= xgt;
   }
-  return int(lt - gt);
+  int res = int(lt - gt);
+  int m = 1 - 2 * int(a.neg & b.neg);
+  return m * res;
 }
 
 extern "C" int BN_cmpCT(const BIGNUM* a, const BIGNUM* b) {

src/cbmpc/crypto/base_ec_core.h

@@ -34,6 +34,16 @@ struct edwards_projective_t {
 
   using fe_t = FE;
   static fe_t get_d();
+
+  // Edwards identity in projective coords is (X=0, Y=Z) (affine (0,1)).
+  // Do NOT use X==0 alone: there exist non-identity torsion points with X==0
+  // (e.g., (0,-1) of order 2 on Ed25519).
+  static bool is_infinity(const fe_t& x, const fe_t& y, const fe_t& z) { return x.is_zero() & (y == z); }
+  static void set_infinity(fe_t& x, fe_t& y, fe_t& z) {
+    x = fe_t::zero();
+    y = fe_t::one();
+    z = fe_t::one();
+  }
   struct precomp_t  // affine
   {
     fe_t y_minus_x, y_plus_x, kt;
@@ -242,8 +252,8 @@ struct edwards_projective_t {
 
   static void add(fe_t& rx, fe_t& ry, fe_t& rz, const fe_t& ax, const fe_t& ay, const fe_t& az, const fe_t& bx,
                   const fe_t& by, const fe_t& bz) {
-    bool a_is_inf = ax.is_zero();
-    bool b_is_inf = bx.is_zero();
+    bool a_is_inf = is_infinity(ax, ay, az);
+    bool b_is_inf = is_infinity(bx, by, bz);
 
     fe_t save_ax = ax;
     fe_t save_ay = ay;
@@ -295,7 +305,7 @@ struct edwards_projective_t {
     const fe_t& X1 = X3;
     const fe_t& Y1 = Y3;
     const fe_t& Z1 = Z3;
-    bool a_is_inf = X1.is_zero();
+    bool a_is_inf = is_infinity(X1, Y1, Z1);
     static const fe_t d = get_d();
 
     fe_t B, C, D, E, F, G, H;
@@ -326,6 +336,14 @@ struct edwards_projective_t {
   }
 };
 
+// NOTE(future-proofing): `ecurve_core_t::point_t` delegates identity handling to the formula:
+// - FORMULA::is_infinity(x, y, z)
+// - FORMULA::set_infinity(x, y, z)
+//
+// Edwards uses a non-standard identity check (X=0 && Y==Z) to avoid confusing torsion points with identity.
+// If/when we add a short-Weierstrass backend based on Jacobian/projective coords, it should implement
+// these hooks too (commonly: infinity iff Z==0).
+
 template <typename FORMULA, bool USE_GLV = false>
 struct ecurve_core_t {
   using fe_t = typename FORMULA::fe_t;
@@ -343,8 +361,8 @@ struct ecurve_core_t {
 
     fe_t x, y, z;
 
-    bool is_infinity() const { return z.is_zero(); }
-    void set_infinity() { x = y = z = fe_t::zero(); }
+    bool is_infinity() const { return FORMULA::is_infinity(x, y, z); }
+    void set_infinity() { FORMULA::set_infinity(x, y, z); }
 
     void get_xy(fe_t& affine_x, fe_t& affine_y) const { FORMULA::get_xy(x, y, z, affine_x, affine_y); }
 
@@ -387,7 +405,12 @@ struct ecurve_core_t {
 
     void cnd_negate(bool flag) { FORMULA::cnd_neg(flag, x, y, z); }
 
-    bool operator==(const point_t& P) const { return FORMULA::equ(x, y, z, P.x, P.y, P.z); }
+    bool operator==(const point_t& P) const {
+      // Avoid degenerate projective representations accidentally comparing equal to all points.
+      if (is_infinity()) return P.is_infinity();
+      if (P.is_infinity()) return false;
+      return FORMULA::equ(x, y, z, P.x, P.y, P.z);
+    }
     bool operator!=(const point_t& P) const { return !(*this == P); }
   };
 
@@ -646,7 +669,14 @@ struct ecurve_core_t {
     }
 
     A.get_xyz(R.x, R.y, R.z);
-    R.z.cnd_assign(r_is_inf, fe_t::zero());
+    // Preserve a *valid* Edwards identity representation when the scalar is zero.
+    // Using Z=0 as a sentinel breaks downstream is_infinity() checks (which intentionally
+    // use X=0 && Y==Z for Edwards to avoid confusing torsion points with the identity).
+    fe_t inf_x, inf_y, inf_z;
+    FORMULA::set_infinity(inf_x, inf_y, inf_z);
+    R.x.cnd_assign(r_is_inf, inf_x);
+    R.y.cnd_assign(r_is_inf, inf_y);
+    R.z.cnd_assign(r_is_inf, inf_z);
   }
 
   static point_t mul_to_generator(const bn_t& x) {

src/cbmpc/crypto/base_ecc.cpp

@@ -837,16 +837,6 @@ ecc_point_t ecc_point_t::weighted_sum(const bn_t& x0, const ecc_point_t& P0, con
         crypto::consttime_point_add_scope_t consttime_point_add_scope;
         return x0 * P0 + x1 * P1;
       }
-      case ct_add_support_e::Conditional: {
-        const mod_t& q = curve.order();
-        bn_t bias0 = bn_t::rand(q);
-        bn_t bias1 = bn_t::rand(q);
-        ecc_point_t Bias = bias0 * P0 + bias1 * P1;
-        bias0 = q.add(bias0, x0);
-        bias1 = q.add(bias1, x1);
-        crypto::consttime_point_add_scope_t consttime_point_add_scope;
-        return bias0 * P0 + bias1 * P1 - Bias;
-      }
       case ct_add_support_e::None:
       default: {
         bn_t bias = bn_t::rand(curve.order());

src/cbmpc/crypto/base_ecc.h

@@ -18,9 +18,8 @@ typedef struct point_t* point_ptr_t;
 
 // Capability of constant-time point addition provided by a curve backend
 enum class ct_add_support_e {
-  None = 0,         // No constant-time add available
-  Conditional = 1,  // Constant-time for non-degenerate cases only
-  Full = 2          // Constant-time for all inputs
+  None = 0,  // No constant-time add available
+  Full = 1   // Constant-time for all inputs
 };
 
 enum class ecurve_type_e {
@@ -63,6 +62,11 @@ class ecurve_interface_t {
   virtual void add_consttime(const ecc_point_t& P, const ecc_point_t& x, ecc_point_t& R) const = 0;
   // Report the support level for constant-time point addition
   virtual ct_add_support_e ct_add_support() const { return ct_add_support_e::None; }
+  // Constant-time conditional copy:
+  // - if flag is true, assign dst = src
+  // - if flag is false, dst is unchanged
+  // Returns true iff the operation is supported and implemented constant-time (w.r.t. flag and point values).
+  virtual bool cnd_copy_point(bool /*flag*/, const ecc_point_t& /*src*/, ecc_point_t& /*dst*/) const { return false; }
   virtual void mul(const ecc_point_t& P, const bn_t& x, ecc_point_t& R) const = 0;
   virtual void mul_vartime(const ecc_point_t& P, const bn_t& x, ecc_point_t& R) const = 0;
   virtual void mul_add(const bn_t& n, const ecc_point_t& P, const bn_t& m, ecc_point_t& R) const;  // R = G*n + P*m
@@ -118,6 +122,9 @@ class ecurve_t {
 
   // Report the support level for constant-time point addition
   ct_add_support_e ct_add_support() const { return ptr ? ptr->ct_add_support() : ct_add_support_e::None; }
+  bool cnd_copy_point(bool flag, const ecc_point_t& src, ecc_point_t& dst) const {
+    return ptr && ptr->cnd_copy_point(flag, src, dst);
+  }
 
   void get_params(bn_t& p, bn_t& a, bn_t& b) const;
   const mod_t& p() const;

src/cbmpc/crypto/base_ecc_secp256k1.cpp

@@ -33,6 +33,36 @@ namespace secp256k1 {
 point_ptr_t new_point(const point_ptr_t src) { return point_ptr_t(new secp256k1_gej(*(const secp256k1_gej*)src)); }
 }  // namespace secp256k1
 
+// Constant-time addition for secp256k1_gej points for all inputs (including infinity).
+// Implementation strategy:
+// - Use libsecp256k1's `secp256k1_gej_add_ge` (side-channel resistant, handles degeneracy),
+//   which requires the second operand in affine form and non-infinity.
+// - If the second operand is infinity, cmov it to generator (any fixed non-infinity point),
+//   perform the addition, then cmov the result back to `a`.
+static void secp256k1_gej_add_consttime_full(secp256k1_gej* r, const secp256k1_gej* a, const secp256k1_gej* b) {
+  SECP256K1_GEJ_VERIFY(a);
+  SECP256K1_GEJ_VERIFY(b);
+
+  const int b_is_inf = b->infinity;
+
+  secp256k1_gej b_noninf = *b;
+  secp256k1_gej_cmov(&b_noninf, &G, b_is_inf);  // ensure non-infinity for affine conversion
+
+  // Convert a copy of b_noninf to affine (constant-time inversion via secp256k1_fe_inv).
+  secp256k1_ge b_ge;
+  secp256k1_gej b_tmp = b_noninf;
+  secp256k1_ge_set_gej(&b_ge, &b_tmp);
+  VERIFY_CHECK(!b_ge.infinity);
+
+  // Add in (side-channel resistant) constant-time.
+  secp256k1_gej_add_ge(r, a, &b_ge);
+
+  // If original `b` was infinity, the result should be `a`.
+  secp256k1_gej_cmov(r, a, b_is_inf);
+
+  SECP256K1_GEJ_VERIFY(r);
+}
+
 ecurve_secp256k1_t::ecurve_secp256k1_t() noexcept {
   name = "SECP256K1";
   type = ecurve_type_e::bitcoin;
@@ -81,6 +111,11 @@ void ecurve_secp256k1_t::copy_point(ecc_point_t& Dst, const ecc_point_t& Src) co
   Dst.secp256k1 = secp256k1::new_point(Src.secp256k1);
 }
 
+bool ecurve_secp256k1_t::cnd_copy_point(bool flag, const ecc_point_t& Src, ecc_point_t& Dst) const {
+  secp256k1_gej_cmov((secp256k1_gej*)Dst.secp256k1, (const secp256k1_gej*)Src.secp256k1, static_cast<int>(flag));
+  return true;
+}
+
 bool ecurve_secp256k1_t::is_on_curve(const ecc_point_t& P) const {
   secp256k1_ge ge;
   secp256k1_ge_set_gej(&ge, (secp256k1_gej*)P.secp256k1);
@@ -114,74 +149,9 @@ void ecurve_secp256k1_t::add(const ecc_point_t& P1, const ecc_point_t& P2, ecc_p
                         (const secp256k1_gej*)P2.secp256k1, nullptr);
 }
 
-// This function does not work for some special cases, like when a or b is infinity, or a and b have the same z
-// coordinate. When points are random, the probability of these cases is negligible.
-static void secp256k1_gej_add_const(secp256k1_gej* r, const secp256k1_gej* a, const secp256k1_gej* b) {
-  secp256k1_fe z22, z12, u1, u2, s1, s2, h, i, h2, h3, t;
-  SECP256K1_GEJ_VERIFY(a);
-  SECP256K1_GEJ_VERIFY(b);
-
-  const bool vartime = is_vartime_scope();
-  if (!vartime) {
-    cb_assert(!a->infinity);
-    cb_assert(!b->infinity);
-  } else {
-    // In verifier code paths we intentionally allow variable-time operations for robustness.
-    if (a->infinity || b->infinity) {
-      secp256k1_gej_add_var(r, a, b, nullptr);
-      return;
-    }
-  }
-
-  secp256k1_fe_sqr(&z22, &b->z);
-  secp256k1_fe_sqr(&z12, &a->z);
-  secp256k1_fe_mul(&u1, &a->x, &z22);
-  secp256k1_fe_mul(&u2, &b->x, &z12);
-  secp256k1_fe_mul(&s1, &a->y, &z22);
-  secp256k1_fe_mul(&s1, &s1, &b->z);
-  secp256k1_fe_mul(&s2, &b->y, &z12);
-  secp256k1_fe_mul(&s2, &s2, &a->z);
-  secp256k1_fe_negate(&h, &u1, 1);
-  secp256k1_fe_add(&h, &u2);
-  secp256k1_fe_negate(&i, &s2, 1);
-  secp256k1_fe_add(&i, &s1);
-
-  if (!vartime) {
-    cb_assert(!secp256k1_fe_normalizes_to_zero(&h));
-    cb_assert(!secp256k1_fe_normalizes_to_zero(&i));
-  } else {
-    // In verifier code paths we can handle degenerate inputs (rare) via the generic addition.
-    if (secp256k1_fe_normalizes_to_zero(&h) || secp256k1_fe_normalizes_to_zero(&i)) {
-      secp256k1_gej_add_var(r, a, b, nullptr);
-      return;
-    }
-  }
-
-  r->infinity = 0;
-  secp256k1_fe_mul(&t, &h, &b->z);
-  secp256k1_fe_mul(&r->z, &a->z, &t);
-
-  secp256k1_fe_sqr(&h2, &h);
-  secp256k1_fe_negate(&h2, &h2, 1);
-  secp256k1_fe_mul(&h3, &h2, &h);
-  secp256k1_fe_mul(&t, &u1, &h2);
-
-  secp256k1_fe_sqr(&r->x, &i);
-  secp256k1_fe_add(&r->x, &h3);
-  secp256k1_fe_add(&r->x, &t);
-  secp256k1_fe_add(&r->x, &t);
-
-  secp256k1_fe_add(&t, &r->x);
-  secp256k1_fe_mul(&r->y, &t, &i);
-  secp256k1_fe_mul(&h3, &h3, &s1);
-  secp256k1_fe_add(&r->y, &h3);
-
-  SECP256K1_GEJ_VERIFY(r);
-}
-
 void ecurve_secp256k1_t::add_consttime(const ecc_point_t& P1, const ecc_point_t& P2, ecc_point_t& R) const {
-  secp256k1_gej_add_const((secp256k1_gej*)R.secp256k1, (const secp256k1_gej*)P1.secp256k1,
-                          (const secp256k1_gej*)P2.secp256k1);
+  secp256k1_gej_add_consttime_full((secp256k1_gej*)R.secp256k1, (const secp256k1_gej*)P1.secp256k1,
+                                   (const secp256k1_gej*)P2.secp256k1);
 }
 
 void ecurve_secp256k1_t::mul_vartime(const ecc_point_t& P, const bn_t& x, ecc_point_t& R) const {
@@ -237,19 +207,28 @@ void ecurve_secp256k1_t::mul_add(const bn_t& n, const ecc_point_t& P, const bn_t
   secp256k1_gej Rn;
   secp256k1_ecmult_gen(&secp256k1_ecmult_gen_ctx, &Rn, &scalar_n);
 
-  // Convert P to affine coordinates for secp256k1_ecmult_const
-  // This involves a field inversion, which is variable-time. Since P is public,
-  // this is usually acceptable. If P is secret, you'll need a constant-time inversion method.
+  // Convert P to affine coordinates for secp256k1_ecmult_const.
+  // Use the constant-time conversion (secp256k1_fe_inv via secp256k1_ge_set_gej).
+  // Also ensure we never feed infinity into secp256k1_ge_set_gej by cmov'ing to generator.
+  secp256k1_gej Pj = *(const secp256k1_gej*)P.secp256k1;
+  const int P_is_inf = Pj.infinity;
+  secp256k1_gej_cmov(&Pj, &G, P_is_inf);
+
   secp256k1_ge P_ge;
-  secp256k1_ge_set_gej(&P_ge, (secp256k1_gej*)P.secp256k1);
+  secp256k1_ge_set_gej(&P_ge, &Pj);
+  VERIFY_CHECK(!P_ge.infinity);
 
   // Compute Rm = mP in constant-time
   secp256k1_gej Rm;
   secp256k1_ecmult_const(&Rm, &P_ge, &scalar_m);
 
-  secp256k1_gej R_sum;
-  secp256k1_gej_add_const(&R_sum, &Rm, &Rn);
+  // If original P was infinity, mP is infinity.
+  secp256k1_gej inf;
+  secp256k1_gej_set_infinity(&inf);
+  secp256k1_gej_cmov(&Rm, &inf, P_is_inf);
 
+  secp256k1_gej R_sum;
+  secp256k1_gej_add_consttime_full(&R_sum, &Rm, &Rn);
   memcpy(R.secp256k1, &R_sum, sizeof(R_sum));
 
   secp256k1_scalar_clear(&scalar_m);

src/cbmpc/crypto/base_ecc_secp256k1.h

@@ -14,6 +14,7 @@ class ecurve_secp256k1_t final : public ecurve_interface_t {
   void init_point(ecc_point_t& P) const override;
   void free_point(ecc_point_t& P) const override;
   void copy_point(ecc_point_t& Dst, const ecc_point_t& Src) const override;
+  bool cnd_copy_point(bool flag, const ecc_point_t& src, ecc_point_t& dst) const override;
   bool is_on_curve(const ecc_point_t& P) const override;
   bool is_in_subgroup(const ecc_point_t& P) const override;
   bool is_infinity(const ecc_point_t& P) const override;
@@ -24,7 +25,7 @@ class ecurve_secp256k1_t final : public ecurve_interface_t {
   void invert_point(ecc_point_t& P) const override;
   void add(const ecc_point_t& P1, const ecc_point_t& P2, ecc_point_t& R) const override;
   void add_consttime(const ecc_point_t& P1, const ecc_point_t& P2, ecc_point_t& R) const override;
-  ct_add_support_e ct_add_support() const override { return ct_add_support_e::Conditional; }
+  ct_add_support_e ct_add_support() const override { return ct_add_support_e::Full; }
   void mul(const ecc_point_t& P, const bn_t& x, ecc_point_t& R) const override;
   void mul_vartime(const ecc_point_t& P, const bn_t& x, ecc_point_t& R) const override;
   void mul_to_generator(const bn_t& val, ecc_point_t& P) const override;