Merge bitcoin-core/secp256k1#1299: Infinity handling: ecmult_const(in…

…finity) works, and group verification

bbc8344 Avoid secp256k1_ge_set_gej_zinv with uninitialized z (Pieter Wuille)
0a2e0b2 Make secp256k1_{fe,ge,gej}_verify work as no-op if non-VERIFY (Pieter Wuille)
f202667 Add invariant checking to group elements (Pieter Wuille)
a18821d Always initialize output coordinates in secp256k1_ge_set_gej (Pieter Wuille)
3086cb9 Expose secp256k1_fe_verify to other modules (Pieter Wuille)
a0e696f Make secp256k1_ecmult_const handle infinity (Gregory Maxwell)

Pull request description:

  Rebase of #791.

  * Clean up infinity handling, make x/y/z always initialized for infinity.
  * Make secp256k1_ecmult_const handle infinity.
    * Infinity isn't currently needed here, but correctly handling it is a little more safe against future changes.
    * Update docs for it to make it clear that it is not constant time in Q. It never was constant time in Q (and would be a little complicated to make constant time in Q: needs a constant time addition function that tracks RZR). It isn't typical for ECDH to be constant time in terms of the pubkey. If it was later made constant time in Q infinity support would be easy to preserve, e.g. by running it on a dummy value and cmoving infinity into the output.
  * Add group verification (`secp256k1_ge_verify` and `secp256k1_gej_verify`, mimicking `secp256k1_fe_verify`).
  * Make the `secp256k1_{fe,ge,gej}_verify` functions also defined (as no-ops) in non-VERIFY mode.

ACKs for top commit:
  jonasnick:
    ACK bbc8344
  real-or-random:
    ACK bbc8344

Tree-SHA512: 82cb51faa2c207603aa10359a311ea618fcb5a81ba175bf15515bf84043223db6428434875854cdfce9ae95f9cfd68c74e4e415f26bd574f1791b5dec1615d19

src/ecmult_const.h

@@ -11,8 +11,7 @@
 #include "group.h"
 
 /**
- * Multiply: R = q*A (in constant-time)
- * A must not be infinity.
+ * Multiply: R = q*A (in constant-time for q)
  */
 static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, const secp256k1_scalar *q);
 

src/ecmult_const_impl.h

@@ -144,6 +144,11 @@ static void secp256k1_ecmult_const(secp256k1_gej *r, const secp256k1_ge *a, cons
 
     int i;
 
+    if (secp256k1_ge_is_infinity(a)) {
+        secp256k1_gej_set_infinity(r);
+        return;
+    }
+
     /* build wnaf representation for q. */
     /* split q into q_1 and q_lam (where q = q_1 + q_lam*lambda, and q_1 and q_lam are ~128 bit) */
     secp256k1_scalar_split_lambda(&q_1, &q_lam, scalar);

src/field.h

@@ -143,4 +143,7 @@ static void secp256k1_fe_get_bounds(secp256k1_fe *r, int m);
 /** Determine whether a is a square (modulo p). */
 static int secp256k1_fe_is_square_var(const secp256k1_fe *a);
 
+/** Check invariants on a field element (no-op unless VERIFY is enabled). */
+static void secp256k1_fe_verify(const secp256k1_fe *a);
+
 #endif /* SECP256K1_FIELD_H */

src/field_10x26_impl.h

@@ -21,8 +21,8 @@
  *  - 2*M*(2^26-1) is the max (inclusive) of the remaining limbs
  */
 
-#ifdef VERIFY
 static void secp256k1_fe_verify(const secp256k1_fe *a) {
+#ifdef VERIFY
     const uint32_t *d = a->n;
     int m = a->normalized ? 1 : 2 * a->magnitude, r = 1;
     r &= (d[0] <= 0x3FFFFFFUL * m);
@@ -47,8 +47,9 @@ static void secp256k1_fe_verify(const secp256k1_fe *a) {
         }
     }
     VERIFY_CHECK(r == 1);
-}
 #endif
+    (void)a;
+}
 
 static void secp256k1_fe_get_bounds(secp256k1_fe *r, int m) {
     VERIFY_CHECK(m >= 0);
@@ -458,9 +459,7 @@ SECP256K1_INLINE static void secp256k1_fe_mul_int(secp256k1_fe *r, int a) {
 }
 
 SECP256K1_INLINE static void secp256k1_fe_add(secp256k1_fe *r, const secp256k1_fe *a) {
-#ifdef VERIFY
     secp256k1_fe_verify(a);
-#endif
     r->n[0] += a->n[0];
     r->n[1] += a->n[1];
     r->n[2] += a->n[2];
@@ -479,11 +478,9 @@ SECP256K1_INLINE static void secp256k1_fe_add(secp256k1_fe *r, const secp256k1_f
 }
 
 SECP256K1_INLINE static void secp256k1_fe_add_int(secp256k1_fe *r, int a) {
-#ifdef VERIFY
     secp256k1_fe_verify(r);
     VERIFY_CHECK(a >= 0);
     VERIFY_CHECK(a <= 0x7FFF);
-#endif
     r->n[0] += a;
 #ifdef VERIFY
     r->magnitude += 1;

src/field_5x52_impl.h

@@ -33,8 +33,8 @@
  *  0 or 1, and its value is already reduced modulo the order of the field.
  */
 
-#ifdef VERIFY
 static void secp256k1_fe_verify(const secp256k1_fe *a) {
+#ifdef VERIFY
     const uint64_t *d = a->n;
     int m = a->normalized ? 1 : 2 * a->magnitude, r = 1;
    /* secp256k1 'p' value defined in "Standards for Efficient Cryptography" (SEC2) 2.7.1. */
@@ -52,8 +52,9 @@ static void secp256k1_fe_verify(const secp256k1_fe *a) {
         }
     }
     VERIFY_CHECK(r == 1);
-}
 #endif
+    (void)a;
+}
 
 static void secp256k1_fe_get_bounds(secp256k1_fe *r, int m) {
     VERIFY_CHECK(m >= 0);
@@ -422,11 +423,9 @@ SECP256K1_INLINE static void secp256k1_fe_mul_int(secp256k1_fe *r, int a) {
 }
 
 SECP256K1_INLINE static void secp256k1_fe_add_int(secp256k1_fe *r, int a) {
-#ifdef VERIFY
     secp256k1_fe_verify(r);
     VERIFY_CHECK(a >= 0);
     VERIFY_CHECK(a <= 0x7FFF);
-#endif
     r->n[0] += a;
 #ifdef VERIFY
     r->magnitude += 1;
@@ -436,9 +435,7 @@ SECP256K1_INLINE static void secp256k1_fe_add_int(secp256k1_fe *r, int a) {
 }
 
 SECP256K1_INLINE static void secp256k1_fe_add(secp256k1_fe *r, const secp256k1_fe *a) {
-#ifdef VERIFY
     secp256k1_fe_verify(a);
-#endif
     r->n[0] += a->n[0];
     r->n[1] += a->n[1];
     r->n[2] += a->n[2];

src/group.h

@@ -164,4 +164,10 @@ static void secp256k1_gej_rescale(secp256k1_gej *r, const secp256k1_fe *b);
  */
 static int secp256k1_ge_is_in_correct_subgroup(const secp256k1_ge* ge);
 
+/** Check invariants on an affine group element (no-op unless VERIFY is enabled). */
+static void secp256k1_ge_verify(const secp256k1_ge *a);
+
+/** Check invariants on a Jacobian group element (no-op unless VERIFY is enabled). */
+static void secp256k1_gej_verify(const secp256k1_gej *a);
+
 #endif /* SECP256K1_GROUP_H */