diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h index 8f539c4bc6b9f..7f3992786101c 100644 --- a/src/scalar_4x64_impl.h +++ b/src/scalar_4x64_impl.h @@ -192,9 +192,9 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { tl = t; \ } \ c0 += tl; /* overflow is handled on the next line */ \ - th += (c0 < tl) ? 1 : 0; /* at most 0xFFFFFFFFFFFFFFFF */ \ + th += (c0 < tl); /* at most 0xFFFFFFFFFFFFFFFF */ \ c1 += th; /* overflow is handled on the next line */ \ - c2 += (c1 < th) ? 1 : 0; /* never overflows by contract (verified in the next line) */ \ + c2 += (c1 < th); /* never overflows by contract (verified in the next line) */ \ VERIFY_CHECK((c1 >= th) || (c2 != 0)); \ } @@ -207,7 +207,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { tl = t; \ } \ c0 += tl; /* overflow is handled on the next line */ \ - th += (c0 < tl) ? 1 : 0; /* at most 0xFFFFFFFFFFFFFFFF */ \ + th += (c0 < tl); /* at most 0xFFFFFFFFFFFFFFFF */ \ c1 += th; /* never overflows by contract (verified in the next line) */ \ VERIFY_CHECK(c1 >= th); \ } @@ -221,16 +221,16 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { tl = t; \ } \ th2 = th + th; /* at most 0xFFFFFFFFFFFFFFFE (in case th was 0x7FFFFFFFFFFFFFFF) */ \ - c2 += (th2 < th) ? 1 : 0; /* never overflows by contract (verified the next line) */ \ + c2 += (th2 < th); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((th2 >= th) || (c2 != 0)); \ tl2 = tl + tl; /* at most 0xFFFFFFFFFFFFFFFE (in case the lowest 63 bits of tl were 0x7FFFFFFFFFFFFFFF) */ \ - th2 += (tl2 < tl) ? 1 : 0; /* at most 0xFFFFFFFFFFFFFFFF */ \ + th2 += (tl2 < tl); /* at most 0xFFFFFFFFFFFFFFFF */ \ c0 += tl2; /* overflow is handled on the next line */ \ - th2 += (c0 < tl2) ? 1 : 0; /* second overflow is handled on the next line */ \ + th2 += (c0 < tl2); /* second overflow is handled on the next line */ \ c2 += (c0 < tl2) & (th2 == 0); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((c0 >= tl2) || (th2 != 0) || (c2 != 0)); \ c1 += th2; /* overflow is handled on the next line */ \ - c2 += (c1 < th2) ? 1 : 0; /* never overflows by contract (verified the next line) */ \ + c2 += (c1 < th2); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((c1 >= th2) || (c2 != 0)); \ } @@ -238,15 +238,15 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { #define sumadd(a) { \ unsigned int over; \ c0 += (a); /* overflow is handled on the next line */ \ - over = (c0 < (a)) ? 1 : 0; \ + over = (c0 < (a)); \ c1 += over; /* overflow is handled on the next line */ \ - c2 += (c1 < over) ? 1 : 0; /* never overflows by contract */ \ + c2 += (c1 < over); /* never overflows by contract */ \ } /** Add a to the number defined by (c0,c1). c1 must never overflow, c2 must be zero. */ #define sumadd_fast(a) { \ c0 += (a); /* overflow is handled on the next line */ \ - c1 += (c0 < (a)) ? 1 : 0; /* never overflows by contract (verified the next line) */ \ + c1 += (c0 < (a)); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((c1 != 0) | (c0 >= (a))); \ VERIFY_CHECK(c2 == 0); \ } diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h index 3c372f34fe933..f8c7fa7efaba9 100644 --- a/src/scalar_8x32_impl.h +++ b/src/scalar_8x32_impl.h @@ -271,9 +271,9 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { tl = t; \ } \ c0 += tl; /* overflow is handled on the next line */ \ - th += (c0 < tl) ? 1 : 0; /* at most 0xFFFFFFFF */ \ + th += (c0 < tl); /* at most 0xFFFFFFFF */ \ c1 += th; /* overflow is handled on the next line */ \ - c2 += (c1 < th) ? 1 : 0; /* never overflows by contract (verified in the next line) */ \ + c2 += (c1 < th); /* never overflows by contract (verified in the next line) */ \ VERIFY_CHECK((c1 >= th) || (c2 != 0)); \ } @@ -286,7 +286,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { tl = t; \ } \ c0 += tl; /* overflow is handled on the next line */ \ - th += (c0 < tl) ? 1 : 0; /* at most 0xFFFFFFFF */ \ + th += (c0 < tl); /* at most 0xFFFFFFFF */ \ c1 += th; /* never overflows by contract (verified in the next line) */ \ VERIFY_CHECK(c1 >= th); \ } @@ -300,16 +300,16 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { tl = t; \ } \ th2 = th + th; /* at most 0xFFFFFFFE (in case th was 0x7FFFFFFF) */ \ - c2 += (th2 < th) ? 1 : 0; /* never overflows by contract (verified the next line) */ \ + c2 += (th2 < th); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((th2 >= th) || (c2 != 0)); \ tl2 = tl + tl; /* at most 0xFFFFFFFE (in case the lowest 63 bits of tl were 0x7FFFFFFF) */ \ - th2 += (tl2 < tl) ? 1 : 0; /* at most 0xFFFFFFFF */ \ + th2 += (tl2 < tl); /* at most 0xFFFFFFFF */ \ c0 += tl2; /* overflow is handled on the next line */ \ - th2 += (c0 < tl2) ? 1 : 0; /* second overflow is handled on the next line */ \ + th2 += (c0 < tl2); /* second overflow is handled on the next line */ \ c2 += (c0 < tl2) & (th2 == 0); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((c0 >= tl2) || (th2 != 0) || (c2 != 0)); \ c1 += th2; /* overflow is handled on the next line */ \ - c2 += (c1 < th2) ? 1 : 0; /* never overflows by contract (verified the next line) */ \ + c2 += (c1 < th2); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((c1 >= th2) || (c2 != 0)); \ } @@ -317,15 +317,15 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { #define sumadd(a) { \ unsigned int over; \ c0 += (a); /* overflow is handled on the next line */ \ - over = (c0 < (a)) ? 1 : 0; \ + over = (c0 < (a)); \ c1 += over; /* overflow is handled on the next line */ \ - c2 += (c1 < over) ? 1 : 0; /* never overflows by contract */ \ + c2 += (c1 < over); /* never overflows by contract */ \ } /** Add a to the number defined by (c0,c1). c1 must never overflow, c2 must be zero. */ #define sumadd_fast(a) { \ c0 += (a); /* overflow is handled on the next line */ \ - c1 += (c0 < (a)) ? 1 : 0; /* never overflows by contract (verified the next line) */ \ + c1 += (c0 < (a)); /* never overflows by contract (verified the next line) */ \ VERIFY_CHECK((c1 != 0) | (c0 >= (a))); \ VERIFY_CHECK(c2 == 0); \ } diff --git a/src/util.h b/src/util.h index 8289e23e0c5db..17f6b1851cf81 100644 --- a/src/util.h +++ b/src/util.h @@ -197,10 +197,15 @@ static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) { /** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized and non-negative.*/ 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. + This prevents clang from figuring out (after inlining) that flag can + take only be 0 or 1, which leads to variable time code. */ + volatile int vflag = flag; + /* Casting a negative int to unsigned and back to int is implementation defined behavior */ VERIFY_CHECK(*r >= 0 && *a >= 0); - mask0 = (unsigned int)flag + ~0u; + mask0 = (unsigned int)vflag + ~0u; mask1 = ~mask0; r_masked = ((unsigned int)*r & mask0); a_masked = ((unsigned int)*a & mask1);