Merge pull request BlockstreamResearch#44

21f81a8 Correct secp256k1_fe_verify and use it everywhere (Pieter Wuille) 59447da Test demonstrating discrepancy in sqr output (Peter Dettman)
apoelstra · Jul 23, 2014 · 7d1956f · 7d1956f
2 parents 5e53856 + 21f81a8
commit 7d1956f
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 12 deletions.
diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h
@@ -33,6 +33,27 @@
 void static secp256k1_fe_inner_start(void) {}
 void static secp256k1_fe_inner_stop(void) {}
 
+#ifdef VERIFY
+void static secp256k1_fe_verify(const secp256k1_fe_t *a) {
+    const uint64_t *d = a->n;
+    int m = a->normalized ? 1 : 2 * a->magnitude, r = 1;
+    r &= (d[0] <= 0xFFFFFFFFFFFFFULL * m);
+    r &= (d[1] <= 0xFFFFFFFFFFFFFULL * m);
+    r &= (d[2] <= 0xFFFFFFFFFFFFFULL * m);
+    r &= (d[3] <= 0xFFFFFFFFFFFFFULL * m);
+    r &= (d[4] <= 0x0FFFFFFFFFFFFULL * m);
+    if (a->normalized) {
+        r &= (m == 1);
+        if (r && (d[4] == 0x0FFFFFFFFFFFFULL) && ((d[3] & d[2] & d[1]) == 0xFFFFFFFFFFFFFULL)) {
+            r &= (d[0] < 0xFFFFEFFFFFC2FULL);
+        }
+    }
+    assert(r == 1);
+}
+#else
+void static secp256k1_fe_verify(const secp256k1_fe_t *a) {}
+#endif
+
 void static secp256k1_fe_normalize(secp256k1_fe_t *r) {
     uint64_t t0 = r->n[0], t1 = r->n[1], t2 = r->n[2], t3 = r->n[3], t4 = r->n[4];
 
@@ -72,6 +93,7 @@ void static secp256k1_fe_normalize(secp256k1_fe_t *r) {
 #ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 1;
+    secp256k1_fe_verify(r);
 #endif
 }
 
@@ -81,20 +103,23 @@ void static inline secp256k1_fe_set_int(secp256k1_fe_t *r, int a) {
 #ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 1;
+    secp256k1_fe_verify(r);
 #endif
 }
 
 // TODO: not constant time!
 int static inline secp256k1_fe_is_zero(const secp256k1_fe_t *a) {
 #ifdef VERIFY
     assert(a->normalized);
+    secp256k1_fe_verify(a);
 #endif
     return (a->n[0] == 0 && a->n[1] == 0 && a->n[2] == 0 && a->n[3] == 0 && a->n[4] == 0);
 }
 
 int static inline secp256k1_fe_is_odd(const secp256k1_fe_t *a) {
 #ifdef VERIFY
     assert(a->normalized);
+    secp256k1_fe_verify(a);
 #endif
     return a->n[0] & 1;
 }
@@ -104,6 +129,8 @@ int static inline secp256k1_fe_equal(const secp256k1_fe_t *a, const secp256k1_fe
 #ifdef VERIFY
     assert(a->normalized);
     assert(b->normalized);
+    secp256k1_fe_verify(a);
+    secp256k1_fe_verify(b);
 #endif
     return (a->n[0] == b->n[0] && a->n[1] == b->n[1] && a->n[2] == b->n[2] && a->n[3] == b->n[3] && a->n[4] == b->n[4]);
 }
@@ -120,13 +147,15 @@ void static secp256k1_fe_set_b32(secp256k1_fe_t *r, const unsigned char *a) {
 #ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 1;
+    secp256k1_fe_verify(r);
 #endif
 }
 
 /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */
 void static secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe_t *a) {
 #ifdef VERIFY
     assert(a->normalized);
+    secp256k1_fe_verify(a);
 #endif
     for (int i=0; i<32; i++) {
         int c = 0;
@@ -142,57 +171,71 @@ void static secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe_t *a) {
 void static inline secp256k1_fe_negate(secp256k1_fe_t *r, const secp256k1_fe_t *a, int m) {
 #ifdef VERIFY
     assert(a->magnitude <= m);
-    r->magnitude = m + 1;
-    r->normalized = 0;
+    secp256k1_fe_verify(a);
 #endif
     r->n[0] = 0xFFFFEFFFFFC2FULL * (m + 1) - a->n[0];
     r->n[1] = 0xFFFFFFFFFFFFFULL * (m + 1) - a->n[1];
     r->n[2] = 0xFFFFFFFFFFFFFULL * (m + 1) - a->n[2];
     r->n[3] = 0xFFFFFFFFFFFFFULL * (m + 1) - a->n[3];
     r->n[4] = 0x0FFFFFFFFFFFFULL * (m + 1) - a->n[4];
-}
-
-void static inline secp256k1_fe_mul_int(secp256k1_fe_t *r, int a) {
 #ifdef VERIFY
-    r->magnitude *= a;
+    r->magnitude = m + 1;
     r->normalized = 0;
+    secp256k1_fe_verify(r);
 #endif
+}
+
+void static inline secp256k1_fe_mul_int(secp256k1_fe_t *r, int a) {
     r->n[0] *= a;
     r->n[1] *= a;
     r->n[2] *= a;
     r->n[3] *= a;
     r->n[4] *= a;
-}
-
-void static inline secp256k1_fe_add(secp256k1_fe_t *r, const secp256k1_fe_t *a) {
 #ifdef VERIFY
-    r->magnitude += a->magnitude;
+    r->magnitude *= a;
     r->normalized = 0;
+    secp256k1_fe_verify(r);
 #endif
+}
+
+void static inline secp256k1_fe_add(secp256k1_fe_t *r, const secp256k1_fe_t *a) {
     r->n[0] += a->n[0];
     r->n[1] += a->n[1];
     r->n[2] += a->n[2];
     r->n[3] += a->n[3];
     r->n[4] += a->n[4];
+#ifdef VERIFY
+    r->magnitude += a->magnitude;
+    r->normalized = 0;
+    secp256k1_fe_verify(r);
+    secp256k1_fe_verify(a);
+#endif
 }
 
 void static secp256k1_fe_mul(secp256k1_fe_t *r, const secp256k1_fe_t *a, const secp256k1_fe_t *b) {
 #ifdef VERIFY
     assert(a->magnitude <= 8);
     assert(b->magnitude <= 8);
+    secp256k1_fe_verify(a);
+    secp256k1_fe_verify(b);
+#endif
+    secp256k1_fe_mul_inner(a->n, b->n, r->n);
+#ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 0;
+    secp256k1_fe_verify(r);
 #endif
-    secp256k1_fe_mul_inner(a->n, b->n, r->n);
 }
 
 void static secp256k1_fe_sqr(secp256k1_fe_t *r, const secp256k1_fe_t *a) {
 #ifdef VERIFY
     assert(a->magnitude <= 8);
+#endif
+    secp256k1_fe_sqr_inner(a->n, r->n);
+#ifdef VERIFY
     r->magnitude = 1;
     r->normalized = 0;
 #endif
-    secp256k1_fe_sqr_inner(a->n, r->n);
 }
 
 #endif
diff --git a/src/tests.c b/src/tests.c
@@ -312,6 +312,21 @@ void run_field_inv_all_var() {
     }
 }
 
+void run_sqr() {
+    secp256k1_fe_t x, s;
+
+    {
+        secp256k1_fe_set_int(&x, 1);
+        secp256k1_fe_negate(&x, &x, 1);
+
+        for (int i=1; i<=512; ++i) {
+            secp256k1_fe_mul_int(&x, 2);
+            secp256k1_fe_normalize(&x);
+            secp256k1_fe_sqr(&s, &x);
+        }
+    }
+}
+
 void test_sqrt(const secp256k1_fe_t *a, const secp256k1_fe_t *k) {
     secp256k1_fe_t r1, r2;
     int v = secp256k1_fe_sqrt(&r1, a);
@@ -609,6 +624,7 @@ int main(int argc, char **argv) {
     run_field_inv_var();
     run_field_inv_all();
     run_field_inv_all_var();
+    run_sqr();
     run_sqrt();
 
     // ecmult tests