diff --git a/src/cbmpc/crypto/base_bn.cpp b/src/cbmpc/crypto/base_bn.cpp index 661e4b6d..da956b72 100644 --- a/src/cbmpc/crypto/base_bn.cpp +++ b/src/cbmpc/crypto/base_bn.cpp @@ -503,6 +503,8 @@ std::vector bn_t::vector_from_bin(mem_t mem, int n, int size, const mod_t& bn_t bn_t::from_bin_bitlen(mem_t mem, int bits) { // static cb_assert(mem.size == coinbase::bits_to_bytes(bits)); + // Handle the 0-bit / empty-input case without indexing into `mem`. + if (mem.size == 0) return from_bin(mem); int unused_bits = bytes_to_bits(mem.size) - bits; byte_t mask = 0xff >> unused_bits; if (mem[0] == (mem[0] & mask)) return from_bin(mem); diff --git a/src/cbmpc/crypto/base_ecc_secp256k1.cpp b/src/cbmpc/crypto/base_ecc_secp256k1.cpp index 27f62667..01f3dc00 100644 --- a/src/cbmpc/crypto/base_ecc_secp256k1.cpp +++ b/src/cbmpc/crypto/base_ecc_secp256k1.cpp @@ -117,13 +117,22 @@ void ecurve_secp256k1_t::add(const ecc_point_t& P1, const ecc_point_t& P2, ecc_p // 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) { - cb_assert(!a->infinity); - cb_assert(!b->infinity); - 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); @@ -137,8 +146,16 @@ static void secp256k1_gej_add_const(secp256k1_gej* r, const secp256k1_gej* a, co secp256k1_fe_negate(&i, &s2, 1); secp256k1_fe_add(&i, &s1); - cb_assert(!secp256k1_fe_normalizes_to_zero(&h)); - cb_assert(!secp256k1_fe_normalizes_to_zero(&i)); + 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); @@ -209,6 +226,14 @@ void ecurve_secp256k1_t::mul_add(const bn_t& n, const ecc_point_t& P, const bn_t secp256k1_scalar_set_b32(&scalar_n, bin_n.data(), nullptr); secp256k1_scalar_set_b32(&scalar_m, bin_m.data(), nullptr); + if (is_vartime_scope()) { + // Verifier code paths allow variable-time operations and require robustness for all inputs. + secp256k1_ecmult((secp256k1_gej*)R.secp256k1, (const secp256k1_gej*)P.secp256k1, &scalar_m, &scalar_n); + secp256k1_scalar_clear(&scalar_m); + secp256k1_scalar_clear(&scalar_n); + return; + } + secp256k1_gej Rn; secp256k1_ecmult_gen(&secp256k1_ecmult_gen_ctx, &Rn, &scalar_n); diff --git a/src/cbmpc/crypto/base_hash.h b/src/cbmpc/crypto/base_hash.h index 16499577..57f951f6 100644 --- a/src/cbmpc/crypto/base_hash.h +++ b/src/cbmpc/crypto/base_hash.h @@ -167,7 +167,11 @@ T& update_state(T& state, const V (&v)[N]) { } template T& update_state(T& state, const std::vector& v) { - for (std::size_t i = 0; i < (int)v.size(); i++) update_state(state, v[i]); + update_state(state, uint64_t(v.size())); + for (std::size_t i = 0; i < v.size(); i++) { + update_state(state, uint64_t(get_bin_size(v[i]))); + update_state(state, v[i]); + } return state; } template @@ -178,7 +182,11 @@ T& update_state(T& state, const V& v) { template T& update_state(T& state, const coinbase::array_view_t& v) { - for (int i = 0; i < v.count; i++) update_state(state, v.ptr[i]); + update_state(state, uint64_t(v.count)); + for (int i = 0; i < v.count; i++) { + update_state(state, uint64_t(get_bin_size(v.ptr[i]))); + update_state(state, v.ptr[i]); + } return state; } diff --git a/src/cbmpc/crypto/base_mod.cpp b/src/cbmpc/crypto/base_mod.cpp index 6e24012d..c4ecc9ad 100644 --- a/src/cbmpc/crypto/base_mod.cpp +++ b/src/cbmpc/crypto/base_mod.cpp @@ -24,7 +24,7 @@ void mod_t::convert(coinbase::converter_t& converter) { converter.convert(m); if (!converter.is_write()) { if (converter.is_error()) return; - if (m <= 0) { + if (m <= 1) { converter.set_error(); return; } diff --git a/src/cbmpc/protocol/ec_dkg.cpp b/src/cbmpc/protocol/ec_dkg.cpp index 9702b5b4..9ebc013f 100644 --- a/src/cbmpc/protocol/ec_dkg.cpp +++ b/src/cbmpc/protocol/ec_dkg.cpp @@ -223,6 +223,8 @@ error_t key_share_mp_t::refresh(job_mp_t& job, buf_t& sid, const key_share_mp_t& // Curve check of R._j[l] is done inside the zk verify function further below if (h._j != h) return coinbase::error(E_CRYPTO); + if (R._j.size() != size_t(n)) return coinbase::error(E_CRYPTO, "ec_dkg: inconsistent batch size (R)"); + if (pi_r._j.size() != size_t(n)) return coinbase::error(E_CRYPTO, "ec_dkg: inconsistent batch size (pi_r)"); if (rv = com_R.id(sid, job.get_pid(j)).set(rho._j, c._j).open(R._j, pi_r._j)) return rv; for (int l = 0; l < n; l++) { diff --git a/src/cbmpc/protocol/ecdsa_2p.cpp b/src/cbmpc/protocol/ecdsa_2p.cpp index 398517b1..fc1c6a7b 100644 --- a/src/cbmpc/protocol/ecdsa_2p.cpp +++ b/src/cbmpc/protocol/ecdsa_2p.cpp @@ -102,6 +102,7 @@ error_t dkg(job_2p_t& job, ecurve_t curve, key_t& key) { } if (rv = job.p1_to_p2(ec_dkg.msg1, paillier_gen.msg1, key.c_key)) return rv; + if (paillier_gen.c_key != key.c_key) return coinbase::error(E_CRYPTO, "paillier_gen.c_key != key.c_key"); if (job.is_p2()) { ec_dkg.step2_p2_to_p1(key.x_share); @@ -268,6 +269,7 @@ error_t sign_batch_impl(job_2p_t& job, buf_t& sid, const key_t& key, const std:: } if (rv = job.p2_to_p1(R2, pi_2)) return rv; + if (job.is_p1() && R2.size() != n_sigs) return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (R2)"); std::vector R(n_sigs); @@ -289,6 +291,7 @@ error_t sign_batch_impl(job_2p_t& job, buf_t& sid, const key_t& key, const std:: if (job.is_p2()) { const mod_t& N = key.paillier.get_N(); + if (R1.size() != n_sigs) return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (R1)"); if (rv = com.open(msgs, R1, pi_1)) return rv; // Checking that R1 values are valid is done in the verify function. @@ -331,6 +334,9 @@ error_t sign_batch_impl(job_2p_t& job, buf_t& sid, const key_t& key, const std:: } if (job.is_p1()) { + if (c.size() != n_sigs) return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (c)"); + if (!global_abort_mode && zk_ecdsa.size() != n_sigs) + return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (zk_ecdsa)"); for (int i = 0; i < n_sigs; i++) { r[i] = R[i].get_x() % q; diff --git a/src/cbmpc/protocol/schnorr_2p.cpp b/src/cbmpc/protocol/schnorr_2p.cpp index 4176df0d..2fb8dfc0 100644 --- a/src/cbmpc/protocol/schnorr_2p.cpp +++ b/src/cbmpc/protocol/schnorr_2p.cpp @@ -50,6 +50,8 @@ error_t sign_batch(job_2p_t& job, key_t& key, const std::vector& msgs, st zk_dl2.prove(R2, k2, sid, 2); } if (rv = job.p2_to_p1(R2, zk_dl2, sid2)) return rv; + if (job.is_p1() && R2.size() != size_t(n_sigs)) + return coinbase::error(E_CRYPTO, "schnorr_2p: inconsistent batch size (R2)"); if (job.is_p1()) { // point checks are covered by the zk proof @@ -60,6 +62,7 @@ error_t sign_batch(job_2p_t& job, key_t& key, const std::vector& msgs, st if (rv = job.p1_to_p2(zk_dl1, R1, com.rand)) return rv; if (job.is_p2()) { + if (R1.size() != size_t(n_sigs)) return coinbase::error(E_CRYPTO, "schnorr_2p: inconsistent batch size (R1)"); // point checks are covered by the zk proof if (rv = com.id(sid1, job.get_pid(party_t::p1)).open(R1)) return rv; if (rv = zk_dl1.verify(R1, sid, 1)) return rv; @@ -102,6 +105,7 @@ error_t sign_batch(job_2p_t& job, key_t& key, const std::vector& msgs, st if (rv = job.p2_to_p1(s2)) return rv; if (job.is_p1()) { + if (s2.size() != size_t(n_sigs)) return coinbase::error(E_CRYPTO, "schnorr_2p: inconsistent batch size (s2)"); for (int i = 0; i < n_sigs; i++) { bn_t s, s1; MODULO(q) { diff --git a/src/cbmpc/protocol/schnorr_mp.cpp b/src/cbmpc/protocol/schnorr_mp.cpp index b5791b18..4861e9ee 100644 --- a/src/cbmpc/protocol/schnorr_mp.cpp +++ b/src/cbmpc/protocol/schnorr_mp.cpp @@ -104,6 +104,8 @@ error_t sign_batch(job_mp_t& job, key_t& key, const std::vector& msgs, pa if (sid._j != sid._i) return coinbase::error(E_CRYPTO); if (h._j != h._i) return coinbase::error(E_CRYPTO); + if (Ri._j.size() != msgs.size()) + return coinbase::error(E_CRYPTO, "schnorrmp::sign_batch: inconsistent batch size (Ri)"); // Verification of Ri._j is done in the zk verify function if (rv = pi._j.verify(Ri._j, sid._i, j)) return coinbase::error(rv, "schnorr_mp_t::sign_batch: verify pi failed"); @@ -151,6 +153,8 @@ error_t sign_batch(job_mp_t& job, key_t& key, const std::vector& msgs, pa if (job.is_party_idx(sig_receiver)) { std::vector ss(msgs.size(), 0); for (int j = 0; j < n; j++) { + if (ssi._j.size() != msgs.size()) + return coinbase::error(E_CRYPTO, "schnorrmp::sign_batch: inconsistent batch size (ssi)"); for (size_t l = 0; l < msgs.size(); l++) MODULO(q) ss[l] += ssi._j[l]; } diff --git a/src/cbmpc/zk/fischlin.h b/src/cbmpc/zk/fischlin.h index f82f2bb1..308b0dca 100644 --- a/src/cbmpc/zk/fischlin.h +++ b/src/cbmpc/zk/fischlin.h @@ -43,13 +43,31 @@ struct fischlin_params_t { int rho, b, t; int e_max() const { - assert(t < 32); + cb_assert(t < 32); return 1 << t; } uint32_t b_mask() const { - assert(b < 32); + cb_assert(b < 32); return (1 << b) - 1; } + error_t check() const { + if (rho <= 0) return coinbase::error(E_CRYPTO, "rho <= 0"); + if (b <= 0) return coinbase::error(E_CRYPTO, "b <= 0"); + if (b >= 32) return coinbase::error(E_CRYPTO, "b >= 32"); + if (int64_t(b) * int64_t(rho) < SEC_P_COM) return coinbase::error(E_CRYPTO, "b * rho < SEC_P_COM"); + return SUCCESS; + } + + error_t check_with_effective_b(int effective_b) const { + if (rho <= 0) return coinbase::error(E_CRYPTO, "rho <= 0"); + if (b <= 0) return coinbase::error(E_CRYPTO, "b <= 0"); + if (b >= 32) return coinbase::error(E_CRYPTO, "b >= 32"); + + if (effective_b <= 0) return coinbase::error(E_CRYPTO, "effective_b <= 0"); + if (int64_t(rho) * int64_t(effective_b) < SEC_P_COM) + return coinbase::error(E_CRYPTO, "rho * effective_b < SEC_P_COM"); + return SUCCESS; + } void convert(coinbase::converter_t& c) { c.convert(rho, b); } // t is not sent }; diff --git a/src/cbmpc/zk/zk_ec.cpp b/src/cbmpc/zk/zk_ec.cpp index 8df7c969..3347e2cf 100644 --- a/src/cbmpc/zk/zk_ec.cpp +++ b/src/cbmpc/zk/zk_ec.cpp @@ -56,8 +56,8 @@ void uc_dl_t::prove(const ecc_point_t& Q, const bn_t& w, mem_t session_id, uint6 error_t uc_dl_t::verify(const ecc_point_t& Q, mem_t session_id, uint64_t aux) const { error_t rv = UNINITIALIZED_ERROR; crypto::vartime_scope_t vartime_scope; + if (rv = params.check()) return rv; int rho = params.rho; - if (params.b * rho < SEC_P_COM) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: b * rho < SEC_P_COM"); if (int(A.size()) != rho) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: A.size() != rho"); if (int(e.size()) != rho) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: e.size() != rho"); if (int(z.size()) != rho) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: z.size() != rho"); @@ -214,11 +214,11 @@ error_t uc_batch_dl_finite_difference_impl_t::verify(const std::vector PQ(n + 1); @@ -243,9 +247,6 @@ error_t uc_batch_dl_finite_difference_impl_t::verify(const std::vector>= param::log_alpha; - crypto::paillier_t::elem_t c_tag(paillier, c_inv.pow_mod(ei, NN)); c_tilde[i] = c_tag + paillier.enc(di[i], D[i]); } - if (D_prod == 0) return coinbase::error(E_CRYPTO); - if (!mod_t::coprime(D_prod, N)) return coinbase::error(E_CRYPTO); buf_t e_buf = crypto::ro::hash_string(N, c, p, q, g, h, Com, c_tilde, Com_tilde, session_id, aux).bitlen(param::lambda); @@ -451,6 +460,9 @@ error_t paillier_pedersen_equal_interactive_t::verify(const crypto::paillier_t& paillier_no_small_factors = zk_flag::verified; } + if (e < 0) return coinbase::error(E_CRYPTO, "paillier_pedersen_equal_interactive_t::verify: e < 0"); + if (nu < 0) return coinbase::error(E_CRYPTO, "paillier_pedersen_equal_interactive_t::verify: nu < 0"); + const pedersen_commitment_params_t& params = pedersen_commitment_params_t::get(); const mod_t& p = params.p; const bn_t& g = params.g; @@ -473,15 +485,20 @@ error_t paillier_pedersen_equal_interactive_t::verify(const crypto::paillier_t& bn_t d = 0; bn_t CD = c; - bn_t e_temp = e; - bn_t radix = 1 << param::log_alpha; bn_t q_with_slack = q << (param::log_alpha + SEC_P_STAT); for (int i = 0; i < param::t; i++) { + if (rv = coinbase::crypto::check_right_open_range(0, di[i], q_with_slack)) return rv; + if (rv = coinbase::crypto::check_open_range(0, Di[i], N)) return rv; MODULO(N) CD *= Di[i] * c_tilde[i]; + } - if (rv = coinbase::crypto::check_right_open_range(0, di[i], q_with_slack)) return rv; + if (CD == 0) return coinbase::error(E_CRYPTO); + if (!mod_t::coprime(CD, N)) return coinbase::error(E_CRYPTO); + bn_t e_temp = e; + bn_t radix = 1 << param::log_alpha; + for (int i = 0; i < param::t; i++) { bn_t ei = mod_t::mod(e_temp, radix); e_temp >>= param::log_alpha; @@ -492,9 +509,6 @@ error_t paillier_pedersen_equal_interactive_t::verify(const crypto::paillier_t& d += di[i] << (i * param::log_alpha); } - if (CD == 0) return coinbase::error(E_CRYPTO); - if (!mod_t::coprime(CD, N)) return coinbase::error(E_CRYPTO); - bn_t C1, C2; MODULO(p) { C1 = Com.pow(e) * Com_tilde; diff --git a/src/cbmpc/zk/zk_unknown_order.cpp b/src/cbmpc/zk/zk_unknown_order.cpp index 0266618d..c908cd5b 100644 --- a/src/cbmpc/zk/zk_unknown_order.cpp +++ b/src/cbmpc/zk/zk_unknown_order.cpp @@ -29,6 +29,28 @@ void unknown_order_dl_t::prove(const bn_t& a, const bn_t& b, const mod_t& N, con error_t unknown_order_dl_t::verify(const bn_t& a, const bn_t& b, const mod_t& N, const int l, mem_t sid, uint64_t aux) const { crypto::vartime_scope_t vartime_scope; + error_t rv = UNINITIALIZED_ERROR; + + if (l <= 0) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: l <= 0"); + if (e.size() != coinbase::bits_to_bytes(SEC_P_COM)) + return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: invalid e size"); + + // Ensure `b` is invertible mod N before attempting BN_mod_inverse (which asserts on failure). + if (rv = coinbase::crypto::check_open_range(0, b, N)) + return coinbase::error(rv, "unknown_order_dl_t::verify: invalid b"); + if (!mod_t::coprime(b, N)) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: gcd(b, N) != 1"); + + const int max_z_bits = l + SEC_P_STAT + 2; + for (int i = 0; i < SEC_P_COM; i++) { + if (z[i] < 0) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: z[i] < 0"); + if (z[i].get_bits_count() > max_z_bits) + return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: z[i] too large"); + } + + bn_t gcd_test; + MODULO(N) gcd_test = a * b; + if (!mod_t::coprime(gcd_test, N)) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: gcd(a*b, N) != 1"); + bn_t b_inv = N.inv(b); bn_t R_tag; diff --git a/tests/unit/crypto/test_base_hash.cpp b/tests/unit/crypto/test_base_hash.cpp new file mode 100644 index 00000000..f4526bbf --- /dev/null +++ b/tests/unit/crypto/test_base_hash.cpp @@ -0,0 +1,25 @@ +#include +#include + +#include + +namespace { + +using namespace coinbase; +using namespace coinbase::crypto; + +TEST(BaseHash, MemTVectorEncodesBoundariesAndLength) { + const std::vector msgs_a = {mem_t("a"), mem_t("bc")}; // concat: "abc" + const std::vector msgs_b = {mem_t("ab"), mem_t("c")}; // concat: "abc" + const std::vector msgs_c = {mem_t("abc")}; // concat: "abc" + + const auto ha = sha256_t::hash(msgs_a); + const auto hb = sha256_t::hash(msgs_b); + const auto hc = sha256_t::hash(msgs_c); + + EXPECT_NE(ha, hb); + EXPECT_NE(ha, hc); + EXPECT_NE(hb, hc); +} + +} // namespace