diff --git a/src/field.h b/src/field.h index 1f6ba7460f..e3f0234f5f 100644 --- a/src/field.h +++ b/src/field.h @@ -307,7 +307,8 @@ static void secp256k1_fe_to_storage(secp256k1_fe_storage *r, const secp256k1_fe */ static void secp256k1_fe_from_storage(secp256k1_fe *r, const secp256k1_fe_storage *a); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag); /** Conditionally move a field element in constant time. diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index ea14c27318..aa45434d9d 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -1014,6 +1014,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp25 SECP256K1_INLINE static void secp256k1_fe_impl_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint32_t)0); mask1 = ~mask0; @@ -1097,6 +1098,7 @@ static SECP256K1_INLINE void secp256k1_fe_impl_half(secp256k1_fe *r) { static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint32_t)0); mask1 = ~mask0; diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 46dca6b981..3a976135eb 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -349,6 +349,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp25 SECP256K1_INLINE static void secp256k1_fe_impl_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) { uint64_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint64_t)0); mask1 = ~mask0; @@ -416,6 +417,7 @@ static SECP256K1_INLINE void secp256k1_fe_impl_half(secp256k1_fe *r) { static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) { uint64_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint64_t)0); mask1 = ~mask0; diff --git a/src/group.h b/src/group.h index 05ae0d203c..ee3ebbbefe 100644 --- a/src/group.h +++ b/src/group.h @@ -169,10 +169,12 @@ static void secp256k1_ge_to_storage(secp256k1_ge_storage *r, const secp256k1_ge /** Convert a group element back from the storage type. */ static void secp256k1_ge_from_storage(secp256k1_ge *r, const secp256k1_ge_storage *a); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k1_gej *a, int flag); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_ge_storage_cmov(secp256k1_ge_storage *r, const secp256k1_ge_storage *a, int flag); /** Rescale a jacobian point by b which must be non-zero. Constant-time. */ diff --git a/src/group_impl.h b/src/group_impl.h index 81a24b9d5b..f5169650a1 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -898,6 +898,7 @@ static void secp256k1_ge_from_storage(secp256k1_ge *r, const secp256k1_ge_storag static SECP256K1_INLINE void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k1_gej *a, int flag) { SECP256K1_GEJ_VERIFY(r); SECP256K1_GEJ_VERIFY(a); + VERIFY_CHECK(flag == 0 || flag == 1); secp256k1_fe_cmov(&r->x, &a->x, flag); secp256k1_fe_cmov(&r->y, &a->y, flag); @@ -908,6 +909,7 @@ static SECP256K1_INLINE void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k } static SECP256K1_INLINE void secp256k1_ge_storage_cmov(secp256k1_ge_storage *r, const secp256k1_ge_storage *a, int flag) { + VERIFY_CHECK(flag == 0 || flag == 1); secp256k1_fe_storage_cmov(&r->x, &a->x, flag); secp256k1_fe_storage_cmov(&r->y, &a->y, flag); } diff --git a/src/scalar.h b/src/scalar.h index 70f49b1cf2..40d67191ac 100644 --- a/src/scalar.h +++ b/src/scalar.h @@ -48,7 +48,7 @@ static void secp256k1_scalar_get_b32(unsigned char *bin, const secp256k1_scalar* /** Add two scalars together (modulo the group order). Returns whether it overflowed. */ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, const secp256k1_scalar *b); -/** Conditionally add a power of two to a scalar. The result is not allowed to overflow. */ +/** Conditionally add a power of two to a scalar. The result is not allowed to overflow. Flag must be 0 or 1. */ static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag); /** Multiply two scalars (modulo the group order). */ @@ -78,7 +78,7 @@ static int secp256k1_scalar_is_even(const secp256k1_scalar *a); /** Check whether a scalar is higher than the group order divided by 2. */ static int secp256k1_scalar_is_high(const secp256k1_scalar *a); -/** Conditionally negate a number, in constant time. +/** Conditionally negate a number, in constant time. Flag must be 0 or 1. * Returns -1 if the number was negated, 1 otherwise */ static int secp256k1_scalar_cond_negate(secp256k1_scalar *a, int flag); @@ -95,7 +95,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT /** Multiply a and b (without taking the modulus!), divide by 2**shift, and round to the nearest integer. Shift must be at least 256. */ static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, const secp256k1_scalar *a, const secp256k1_scalar *b, unsigned int shift); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag); /** Check invariants on a scalar (no-op unless VERIFY is enabled). */ diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h index 807b9b70ab..a5bf18feb9 100644 --- a/src/scalar_4x64_impl.h +++ b/src/scalar_4x64_impl.h @@ -120,6 +120,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) { secp256k1_uint128 t; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); VERIFY_CHECK(bit < 256); @@ -259,6 +260,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { uint64_t mask = -vflag; uint64_t nonzero = (secp256k1_scalar_is_zero(r) != 0) - 1; secp256k1_uint128 t; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); secp256k1_u128_from_u64(&t, r->d[0] ^ mask); @@ -911,6 +913,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) { uint64_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(a); SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d)); diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h index 2610496052..aa87b1d3d1 100644 --- a/src/scalar_8x32_impl.h +++ b/src/scalar_8x32_impl.h @@ -147,6 +147,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) { uint64_t t; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); VERIFY_CHECK(bit < 256); @@ -314,6 +315,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { uint32_t mask = -vflag; uint32_t nonzero = 0xFFFFFFFFUL * (secp256k1_scalar_is_zero(r) == 0); uint64_t t = (uint64_t)(r->d[0] ^ mask) + ((SECP256K1_N_0 + 1) & mask); + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); r->d[0] = t & nonzero; t >>= 32; @@ -709,6 +711,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(a); SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d)); diff --git a/src/scalar_low_impl.h b/src/scalar_low_impl.h index 84e1a380a3..628bfd33e8 100644 --- a/src/scalar_low_impl.h +++ b/src/scalar_low_impl.h @@ -56,6 +56,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) { SECP256K1_SCALAR_VERIFY(r); + VERIFY_CHECK(flag == 0 || flag == 1); if (flag && bit < 32) *r += ((uint32_t)1 << bit); @@ -121,6 +122,7 @@ static int secp256k1_scalar_is_high(const secp256k1_scalar *a) { static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { SECP256K1_SCALAR_VERIFY(r); + VERIFY_CHECK(flag == 0 || flag == 1); if (flag) secp256k1_scalar_negate(r, r); @@ -157,6 +159,7 @@ SECP256K1_INLINE static int secp256k1_scalar_eq(const secp256k1_scalar *a, const static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(a); SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r)); diff --git a/src/util.h b/src/util.h index 94e0837dab..46ab381077 100644 --- a/src/util.h +++ b/src/util.h @@ -212,6 +212,7 @@ static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) { take only be 0 or 1, which leads to variable time code. */ volatile int vflag = flag; unsigned char mask = -(unsigned char) vflag; + VERIFY_CHECK(flag == 0 || flag == 1); while (len) { *p &= ~mask; p++; @@ -294,7 +295,8 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size return ret; } -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized and non-negative.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized and non-negative. Flag must be 0 or 1. */ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) { unsigned int mask0, mask1, r_masked, a_masked; /* Access flag with a volatile-qualified lvalue. @@ -302,6 +304,7 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) take only be 0 or 1, which leads to variable time code. */ volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); /* Casting a negative int to unsigned and back to int is implementation defined behavior */ VERIFY_CHECK(*r >= 0 && *a >= 0);