Merge #1066: Abstract out and merge all the magnitude/normalized logic

7fc642f Simplify secp256k1_fe_{impl_,}verify (Pieter Wuille)
4e176ad Abstract out verify logic for fe_is_square_var (Pieter Wuille)
4371f98 Abstract out verify logic for fe_add_int (Pieter Wuille)
89e324c Abstract out verify logic for fe_half (Pieter Wuille)
283cd80 Abstract out verify logic for fe_get_bounds (Pieter Wuille)
d5aa2f0 Abstract out verify logic for fe_inv{,_var} (Pieter Wuille)
3167646 Abstract out verify logic for fe_from_storage (Pieter Wuille)
76d31e5 Abstract out verify logic for fe_to_storage (Pieter Wuille)
1e6894b Abstract out verify logic for fe_cmov (Pieter Wuille)
be82bd8 Improve comments/checks for fe_sqrt (Pieter Wuille)
6ab3508 Abstract out verify logic for fe_sqr (Pieter Wuille)
4c25f6e Abstract out verify logic for fe_mul (Pieter Wuille)
e179e65 Abstract out verify logic for fe_add (Pieter Wuille)
7e7ad7f Abstract out verify logic for fe_mul_int (Pieter Wuille)
65d82a3 Abstract out verify logic for fe_negate (Pieter Wuille)
1446708 Abstract out verify logic for fe_get_b32 (Pieter Wuille)
f7a7666 Abstract out verify logic for fe_set_b32 (Pieter Wuille)
ce4d209 Abstract out verify logic for fe_cmp_var (Pieter Wuille)
7d7d43c Improve comments/check for fe_equal{,_var} (Pieter Wuille)
c5e788d Abstract out verify logic for fe_is_odd (Pieter Wuille)
d3f3fe8 Abstract out verify logic for fe_is_zero (Pieter Wuille)
c701d9a Abstract out verify logic for fe_clear (Pieter Wuille)
19a2bfe Abstract out verify logic for fe_set_int (Pieter Wuille)
864f9db Abstract out verify logic for fe_normalizes_to_zero{,_var} (Pieter Wuille)
6c31371 Abstract out verify logic for fe_normalize_var (Pieter Wuille)
e28b51f Abstract out verify logic for fe_normalize_weak (Pieter Wuille)
b6b6f9c Abstract out verify logic for fe_normalize (Pieter Wuille)
7fa5195 Bugfix: correct SECP256K1_FE_CONST mag/norm fields (Pieter Wuille)
b29566c Merge magnitude/normalized fields, move/improve comments (Pieter Wuille)

Pull request description:

  Right now, all the logic for propagating/computing the magnitude/normalized fields in `secp256k1_fe` (when `VERIFY` is defined) and the code for checking it, is duplicated across the two field implementations. I believe that is undesirable, as these properties should purely be a function of the performed fe_ functions, and not of the choice of field implementation. This becomes even uglier with #967, which would copy all that, and even needs an additional dimension that would then need to be added to the two other fields. It's also related to #1001, which I think will become easier if it doesn't need to be done/reasoned about separately for every field.

  This PR moves all logic around these fields (collectively called field verification) to implementations in field_impl.h, which dispatch to renamed functions in field_*_impl.h for the actual implementation.

  Fixes #1060.

ACKs for top commit:
  jonasnick:
    ACK 7fc642f
  real-or-random:
    ACK 7fc642f

Tree-SHA512: 0f94e13fedc47e47859261a182c4077308f8910495691f7e4d7877d9298385172c70e98b4a1e270b6bde4d0062b932607106306bdb35a519cdeab9695a5c71e4

src/field.h

@@ -7,19 +7,36 @@
 #ifndef SECP256K1_FIELD_H
 #define SECP256K1_FIELD_H
 
-/** Field element module.
- *
- *  Field elements can be represented in several ways, but code accessing
- *  it (and implementations) need to take certain properties into account:
- *  - Each field element can be normalized or not.
- *  - Each field element has a magnitude, which represents how far away
- *    its representation is away from normalization. Normalized elements
- *    always have a magnitude of 0 or 1, but a magnitude of 1 doesn't
- *    imply normality.
- */
-
 #include "util.h"
 
+/* This file defines the generic interface for working with secp256k1_fe
+ * objects, which represent field elements (integers modulo 2^256 - 2^32 - 977).
+ *
+ * The actual definition of the secp256k1_fe type depends on the chosen field
+ * implementation; see the field_5x52.h and field_10x26.h files for details.
+ *
+ * All secp256k1_fe objects have implicit properties that determine what
+ * operations are permitted on it. These are purely a function of what
+ * secp256k1_fe_ operations are applied on it, generally (implicitly) fixed at
+ * compile time, and do not depend on the chosen field implementation. Despite
+ * that, what these properties actually entail for the field representation
+ * values depends on the chosen field implementation. These properties are:
+ * - magnitude: an integer in [0,32]
+ * - normalized: 0 or 1; normalized=1 implies magnitude <= 1.
+ *
+ * In VERIFY mode, they are materialized explicitly as fields in the struct,
+ * allowing run-time verification of these properties. In that case, the field
+ * implementation also provides a secp256k1_fe_verify routine to verify that
+ * these fields match the run-time value and perform internal consistency
+ * checks. */
+#ifdef VERIFY
+#  define SECP256K1_FE_VERIFY_FIELDS \
+    int magnitude; \
+    int normalized;
+#else
+#  define SECP256K1_FE_VERIFY_FIELDS
+#endif
+
 #if defined(SECP256K1_WIDEMUL_INT128)
 #include "field_5x52.h"
 #elif defined(SECP256K1_WIDEMUL_INT64)
@@ -28,119 +45,287 @@
 #error "Please select wide multiplication implementation"
 #endif
 
+#ifdef VERIFY
+/* Magnitude and normalized value for constants. */
+#define SECP256K1_FE_VERIFY_CONST(d7, d6, d5, d4, d3, d2, d1, d0) \
+    /* Magnitude is 0 for constant 0; 1 otherwise. */ \
+    , (((d7) | (d6) | (d5) | (d4) | (d3) | (d2) | (d1) | (d0)) != 0) \
+    /* Normalized is 1 unless sum(d_i<<(32*i) for i=0..7) exceeds field modulus. */ \
+    , (!(((d7) & (d6) & (d5) & (d4) & (d3) & (d2)) == 0xfffffffful && ((d1) == 0xfffffffful || ((d1) == 0xfffffffe && (d0 >= 0xfffffc2f)))))
+#else
+#define SECP256K1_FE_VERIFY_CONST(d7, d6, d5, d4, d3, d2, d1, d0)
+#endif
+
+/** This expands to an initializer for a secp256k1_fe valued sum((i*32) * d_i, i=0..7) mod p.
+ *
+ * It has magnitude 1, unless d_i are all 0, in which case the magnitude is 0.
+ * It is normalized, unless sum(2^(i*32) * d_i, i=0..7) >= p.
+ *
+ * SECP256K1_FE_CONST_INNER is provided by the implementation.
+ */
+#define SECP256K1_FE_CONST(d7, d6, d5, d4, d3, d2, d1, d0) {SECP256K1_FE_CONST_INNER((d7), (d6), (d5), (d4), (d3), (d2), (d1), (d0)) SECP256K1_FE_VERIFY_CONST((d7), (d6), (d5), (d4), (d3), (d2), (d1), (d0)) }
+
 static const secp256k1_fe secp256k1_fe_one = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0, 1);
 static const secp256k1_fe secp256k1_const_beta = SECP256K1_FE_CONST(
     0x7ae96a2bul, 0x657c0710ul, 0x6e64479eul, 0xac3434e9ul,
     0x9cf04975ul, 0x12f58995ul, 0xc1396c28ul, 0x719501eeul
 );
 
-/** Normalize a field element. This brings the field element to a canonical representation, reduces
- *  its magnitude to 1, and reduces it modulo field size `p`.
+#ifndef VERIFY
+/* In non-VERIFY mode, we #define the fe operations to be identical to their
+ * internal field implementation, to avoid the potential overhead of a
+ * function call (even though presumably inlinable). */
+#  define secp256k1_fe_normalize secp256k1_fe_impl_normalize
+#  define secp256k1_fe_normalize_weak secp256k1_fe_impl_normalize_weak
+#  define secp256k1_fe_normalize_var secp256k1_fe_impl_normalize_var
+#  define secp256k1_fe_normalizes_to_zero secp256k1_fe_impl_normalizes_to_zero
+#  define secp256k1_fe_normalizes_to_zero_var secp256k1_fe_impl_normalizes_to_zero_var
+#  define secp256k1_fe_set_int secp256k1_fe_impl_set_int
+#  define secp256k1_fe_clear secp256k1_fe_impl_clear
+#  define secp256k1_fe_is_zero secp256k1_fe_impl_is_zero
+#  define secp256k1_fe_is_odd secp256k1_fe_impl_is_odd
+#  define secp256k1_fe_cmp_var secp256k1_fe_impl_cmp_var
+#  define secp256k1_fe_set_b32 secp256k1_fe_impl_set_b32
+#  define secp256k1_fe_get_b32 secp256k1_fe_impl_get_b32
+#  define secp256k1_fe_negate secp256k1_fe_impl_negate
+#  define secp256k1_fe_mul_int secp256k1_fe_impl_mul_int
+#  define secp256k1_fe_add secp256k1_fe_impl_add
+#  define secp256k1_fe_mul secp256k1_fe_impl_mul
+#  define secp256k1_fe_sqr secp256k1_fe_impl_sqr
+#  define secp256k1_fe_cmov secp256k1_fe_impl_cmov
+#  define secp256k1_fe_to_storage secp256k1_fe_impl_to_storage
+#  define secp256k1_fe_from_storage secp256k1_fe_impl_from_storage
+#  define secp256k1_fe_inv secp256k1_fe_impl_inv
+#  define secp256k1_fe_inv_var secp256k1_fe_impl_inv_var
+#  define secp256k1_fe_get_bounds secp256k1_fe_impl_get_bounds
+#  define secp256k1_fe_half secp256k1_fe_impl_half
+#  define secp256k1_fe_add_int secp256k1_fe_impl_add_int
+#  define secp256k1_fe_is_square_var secp256k1_fe_impl_is_square_var
+#endif /* !defined(VERIFY) */
+
+/** Normalize a field element.
+ *
+ * On input, r must be a valid field element.
+ * On output, r represents the same value but has normalized=1 and magnitude=1.
  */
 static void secp256k1_fe_normalize(secp256k1_fe *r);
 
-/** Weakly normalize a field element: reduce its magnitude to 1, but don't fully normalize. */
+/** Give a field element magnitude 1.
+ *
+ * On input, r must be a valid field element.
+ * On output, r represents the same value but has magnitude=1. Normalized is unchanged.
+ */
 static void secp256k1_fe_normalize_weak(secp256k1_fe *r);
 
-/** Normalize a field element, without constant-time guarantee. */
+/** Normalize a field element, without constant-time guarantee.
+ *
+ * Identical in behavior to secp256k1_fe_normalize, but not constant time in r.
+ */
 static void secp256k1_fe_normalize_var(secp256k1_fe *r);
 
-/** Verify whether a field element represents zero i.e. would normalize to a zero value. */
+/** Determine whether r represents field element 0.
+ *
+ * On input, r must be a valid field element.
+ * Returns whether r = 0 (mod p).
+ */
 static int secp256k1_fe_normalizes_to_zero(const secp256k1_fe *r);
 
-/** Verify whether a field element represents zero i.e. would normalize to a zero value,
- *  without constant-time guarantee. */
+/** Determine whether r represents field element 0, without constant-time guarantee.
+ *
+ * Identical in behavior to secp256k1_normalizes_to_zero, but not constant time in r.
+ */
 static int secp256k1_fe_normalizes_to_zero_var(const secp256k1_fe *r);
 
-/** Set a field element equal to a small (not greater than 0x7FFF), non-negative integer.
- *  Resulting field element is normalized; it has magnitude 0 if a == 0, and magnitude 1 otherwise.
+/** Set a field element to an integer in range [0,0x7FFF].
+ *
+ * On input, r does not need to be initialized, a must be in [0,0x7FFF].
+ * On output, r represents value a, is normalized and has magnitude (a!=0).
  */
 static void secp256k1_fe_set_int(secp256k1_fe *r, int a);
 
-/** Sets a field element equal to zero, initializing all fields. */
+/** Set a field element to 0.
+ *
+ * On input, a does not need to be initialized.
+ * On output, a represents 0, is normalized and has magnitude 0.
+ */
 static void secp256k1_fe_clear(secp256k1_fe *a);
 
-/** Verify whether a field element is zero. Requires the input to be normalized. */
+/** Determine whether a represents field element 0.
+ *
+ * On input, a must be a valid normalized field element.
+ * Returns whether a = 0 (mod p).
+ *
+ * This behaves identical to secp256k1_normalizes_to_zero{,_var}, but requires
+ * normalized input (and is much faster).
+ */
 static int secp256k1_fe_is_zero(const secp256k1_fe *a);
 
-/** Check the "oddness" of a field element. Requires the input to be normalized. */
+/** Determine whether a (mod p) is odd.
+ *
+ * On input, a must be a valid normalized field element.
+ * Returns (int(a) mod p) & 1.
+ */
 static int secp256k1_fe_is_odd(const secp256k1_fe *a);
 
-/** Compare two field elements. Requires magnitude-1 inputs. */
+/** Determine whether two field elements are equal.
+ *
+ * On input, a and b must be valid field elements with magnitudes not exceeding
+ * 1 and 31, respectively.
+ * Returns a = b (mod p).
+ */
 static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b);
 
-/** Same as secp256k1_fe_equal, but may be variable time. */
+/** Determine whether two field elements are equal, without constant-time guarantee.
+ *
+ * Identical in behavior to secp256k1_fe_equal, but not constant time in either a or b.
+ */
 static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b);
 
-/** Compare two field elements. Requires both inputs to be normalized */
+/** Compare the values represented by 2 field elements, without constant-time guarantee.
+ *
+ * On input, a and b must be valid normalized field elements.
+ * Returns 1 if a > b, -1 if a < b, and 0 if a = b (comparisons are done as integers
+ * in range 0..p-1).
+ */
 static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b);
 
-/** Set a field element equal to 32-byte big endian value.
- *  Returns 1 if no overflow occurred, and then the output is normalized.
- *  Returns 0 if overflow occurred, and then the output is only weakly normalized. */
+/** Set a field element equal to a provided 32-byte big endian value.
+ *
+ * On input, r does not need to be initalized. a must be a pointer to an initialized 32-byte array.
+ * On output, r = a (mod p). It will have magnitude 1, and if (a < p), it will be normalized.
+ * If not, it will only be weakly normalized. Returns whether (a < p).
+ *
+ * Note that this function is unusual in that the normalization of the output depends on the
+ * run-time value of a.
+ */
 static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a);
 
-/** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */
+/** Convert a field element to 32-byte big endian byte array.
+ * On input, a must be a valid normalized field element, and r a pointer to a 32-byte array.
+ * On output, r = a (mod p).
+ */
 static void secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe *a);
 
-/** Set a field element equal to the additive inverse of another. Takes a maximum magnitude of the input
- *  as an argument. The magnitude of the output is one higher. */
+/** Negate a field element.
+ *
+ * On input, r does not need to be initialized. a must be a valid field element with
+ * magnitude not exceeding m. m must be an integer in [0,31].
+ * Performs {r = -a}.
+ * On output, r will not be normalized, and will have magnitude m+1.
+ */
 static void secp256k1_fe_negate(secp256k1_fe *r, const secp256k1_fe *a, int m);
 
-/** Adds a small integer (up to 0x7FFF) to r. The resulting magnitude increases by one. */
+/** Add a small integer to a field element.
+ *
+ * Performs {r += a}. The magnitude of r increases by 1, and normalized is cleared.
+ * a must be in range [0,0xFFFF].
+ */
 static void secp256k1_fe_add_int(secp256k1_fe *r, int a);
 
-/** Multiplies the passed field element with a small integer constant. Multiplies the magnitude by that
- *  small integer. */
+/** Multiply a field element with a small integer.
+ *
+ * On input, r must be a valid field element. a must be an integer in [0,32].
+ * The magnitude of r times a must not exceed 32.
+ * Performs {r *= a}.
+ * On output, r's magnitude is multiplied by a, and r will not be normalized.
+ */
 static void secp256k1_fe_mul_int(secp256k1_fe *r, int a);
 
-/** Adds a field element to another. The result has the sum of the inputs' magnitudes as magnitude. */
+/** Increment a field element by another.
+ *
+ * On input, r and a must be valid field elements, not necessarily normalized.
+ * The sum of their magnitudes must not exceed 32.
+ * Performs {r += a}.
+ * On output, r will not be normalized, and will have magnitude incremented by a's.
+ */
 static void secp256k1_fe_add(secp256k1_fe *r, const secp256k1_fe *a);
 
-/** Sets a field element to be the product of two others. Requires the inputs' magnitudes to be at most 8.
- *  The output magnitude is 1 (but not guaranteed to be normalized). */
+/** Multiply two field elements.
+ *
+ * On input, a and b must be valid field elements; r does not need to be initialized.
+ * r and a may point to the same object, but neither can be equal to b. The magnitudes
+ * of a and b must not exceed 8.
+ * Performs {r = a * b}
+ * On output, r will have magnitude 1, but won't be normalized.
+ */
 static void secp256k1_fe_mul(secp256k1_fe *r, const secp256k1_fe *a, const secp256k1_fe * SECP256K1_RESTRICT b);
 
-/** Sets a field element to be the square of another. Requires the input's magnitude to be at most 8.
- *  The output magnitude is 1 (but not guaranteed to be normalized). */
+/** Square a field element.
+ *
+ * On input, a must be a valid field element; r does not need to be initialized. The magnitude
+ * of a must not exceed 8.
+ * Performs {r = a**2}
+ * On output, r will have magnitude 1, but won't be normalized.
+ */
 static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a);
 
-/** If a has a square root, it is computed in r and 1 is returned. If a does not
- *  have a square root, the root of its negation is computed and 0 is returned.
- *  The input's magnitude can be at most 8. The output magnitude is 1 (but not
- *  guaranteed to be normalized). The result in r will always be a square
- *  itself. */
-static int secp256k1_fe_sqrt(secp256k1_fe *r, const secp256k1_fe *a);
+/** Compute a square root of a field element.
+ *
+ * On input, a must be a valid field element with magnitude<=8; r need not be initialized.
+ * Performs {r = sqrt(a)} or {r = sqrt(-a)}, whichever exists. The resulting value
+ * represented by r will be a square itself. Variables r and a must not point to the same object.
+ * On output, r will have magnitude 1 but will not be normalized.
+ */
+static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k1_fe * SECP256K1_RESTRICT a);
 
-/** Sets a field element to be the (modular) inverse of another. Requires the input's magnitude to be
- *  at most 8. The output magnitude is 1 (but not guaranteed to be normalized). */
+/** Compute the modular inverse of a field element.
+ *
+ * On input, a must be a valid field element; r need not be initialized.
+ * Performs {r = a**(p-2)} (which maps 0 to 0, and every other element to its
+ * inverse).
+ * On output, r will have magnitude (a.magnitude != 0) and be normalized.
+ */
 static void secp256k1_fe_inv(secp256k1_fe *r, const secp256k1_fe *a);
 
-/** Potentially faster version of secp256k1_fe_inv, without constant-time guarantee. */
+/** Compute the modular inverse of a field element, without constant-time guarantee.
+ *
+ * Behaves identically to secp256k1_fe_inv, but is not constant-time in a.
+ */
 static void secp256k1_fe_inv_var(secp256k1_fe *r, const secp256k1_fe *a);
 
-/** Convert a field element to the storage type. */
+/** Convert a field element to secp256k1_fe_storage.
+ *
+ * On input, a must be a valid normalized field element.
+ * Performs {r = a}.
+ */
 static void secp256k1_fe_to_storage(secp256k1_fe_storage *r, const secp256k1_fe *a);
 
-/** Convert a field element back from the storage type. */
+/** Convert a field element back from secp256k1_fe_storage.
+ *
+ * On input, r need not be initialized.
+ * Performs {r = a}.
+ * On output, r will be normalized and will have magnitude 1.
+ */
 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.*/
 static void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag);
 
-/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time.  Both *r and *a must be initialized.*/
+/** Conditionally move a field element in constant time.
+ *
+ * On input, both r and a must be valid field elements. Flag must be 0 or 1.
+ * Performs {r = flag ? a : r}.
+ * On output, r's magnitude and normalized will equal a's in case of flag=1, unchanged otherwise.
+ */
 static void secp256k1_fe_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag);
 
-/** Halves the value of a field element modulo the field prime. Constant-time.
- *  For an input magnitude 'm', the output magnitude is set to 'floor(m/2) + 1'.
- *  The output is not guaranteed to be normalized, regardless of the input. */
+/** Halve the value of a field element modulo the field prime in constant-time.
+ *
+ * On input, r must be a valid field element.
+ * On output, r will be normalized and have magnitude floor(m/2) + 1 where m is
+ * the magnitude of r on input.
+ */
 static void secp256k1_fe_half(secp256k1_fe *r);
 
-/** Sets each limb of 'r' to its upper bound at magnitude 'm'. The output will also have its
- *  magnitude set to 'm' and is normalized if (and only if) 'm' is zero. */
+/** Sets r to a field element with magnitude m, normalized if (and only if) m==0.
+ *  The value is chosen so that it is likely to trigger edge cases related to
+ *  internal overflows. */
 static void secp256k1_fe_get_bounds(secp256k1_fe *r, int m);
 
-/** Determine whether a is a square (modulo p). */
+/** Determine whether a is a square (modulo p).
+ *
+ * On input, a must be a valid field element.
+ */
 static int secp256k1_fe_is_square_var(const secp256k1_fe *a);
 
 /** Check invariants on a field element (no-op unless VERIFY is enabled). */