Split fe_set_b32 into reducing and normalizing variants

src/bench_internal.c

@@ -65,10 +65,10 @@ static void bench_setup(void* arg) {
 
     secp256k1_scalar_set_b32(&data->scalar[0], init[0], NULL);
     secp256k1_scalar_set_b32(&data->scalar[1], init[1], NULL);
-    secp256k1_fe_set_b32(&data->fe[0], init[0]);
-    secp256k1_fe_set_b32(&data->fe[1], init[1]);
-    secp256k1_fe_set_b32(&data->fe[2], init[2]);
-    secp256k1_fe_set_b32(&data->fe[3], init[3]);
+    secp256k1_fe_set_b32_limit(&data->fe[0], init[0]);
+    secp256k1_fe_set_b32_limit(&data->fe[1], init[1]);
+    secp256k1_fe_set_b32_limit(&data->fe[2], init[2]);
+    secp256k1_fe_set_b32_limit(&data->fe[3], init[3]);
     CHECK(secp256k1_ge_set_xo_var(&data->ge[0], &data->fe[0], 0));
     CHECK(secp256k1_ge_set_xo_var(&data->ge[1], &data->fe[1], 1));
     secp256k1_gej_set_ge(&data->gej[0], &data->ge[0]);

src/ecdsa_impl.h

@@ -239,7 +239,8 @@ static int secp256k1_ecdsa_sig_verify(const secp256k1_scalar *sigr, const secp25
 }
 #else
     secp256k1_scalar_get_b32(c, sigr);
-    secp256k1_fe_set_b32(&xr, c);
+    /* we can ignore the fe_set_b32_limit return value, because we know the input is in range */
+    (void)secp256k1_fe_set_b32_limit(&xr, c);
 
     /** We now have the recomputed R point in pr, and its claimed x coordinate (modulo n)
      *  in xr. Naively, we would extract the x coordinate from pr (requiring a inversion modulo p),

src/eckey_impl.h

@@ -17,10 +17,10 @@
 static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char *pub, size_t size) {
     if (size == 33 && (pub[0] == SECP256K1_TAG_PUBKEY_EVEN || pub[0] == SECP256K1_TAG_PUBKEY_ODD)) {
         secp256k1_fe x;
-        return secp256k1_fe_set_b32(&x, pub+1) && secp256k1_ge_set_xo_var(elem, &x, pub[0] == SECP256K1_TAG_PUBKEY_ODD);
+        return secp256k1_fe_set_b32_limit(&x, pub+1) && secp256k1_ge_set_xo_var(elem, &x, pub[0] == SECP256K1_TAG_PUBKEY_ODD);
     } else if (size == 65 && (pub[0] == SECP256K1_TAG_PUBKEY_UNCOMPRESSED || pub[0] == SECP256K1_TAG_PUBKEY_HYBRID_EVEN || pub[0] == SECP256K1_TAG_PUBKEY_HYBRID_ODD)) {
         secp256k1_fe x, y;
-        if (!secp256k1_fe_set_b32(&x, pub+1) || !secp256k1_fe_set_b32(&y, pub+33)) {
+        if (!secp256k1_fe_set_b32_limit(&x, pub+1) || !secp256k1_fe_set_b32_limit(&y, pub+33)) {
             return 0;
         }
         secp256k1_ge_set_xy(elem, &x, &y);

src/ecmult_gen_compute_table_impl.h

@@ -31,7 +31,7 @@ static void secp256k1_ecmult_gen_compute_table(secp256k1_ge_storage* table, cons
         secp256k1_fe nums_x;
         secp256k1_ge nums_ge;
         int r;
-        r = secp256k1_fe_set_b32(&nums_x, nums_b32);
+        r = secp256k1_fe_set_b32_limit(&nums_x, nums_b32);
         (void)r;
         VERIFY_CHECK(r);
         r = secp256k1_ge_set_xo_var(&nums_ge, &nums_x, 0);

src/ecmult_gen_impl.h

@@ -108,7 +108,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
     memset(keydata, 0, sizeof(keydata));
     /* Accept unobservably small non-uniformity. */
     secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
-    overflow = !secp256k1_fe_set_b32(&s, nonce32);
+    overflow = !secp256k1_fe_set_b32_limit(&s, nonce32);
     overflow |= secp256k1_fe_is_zero(&s);
     secp256k1_fe_cmov(&s, &secp256k1_fe_one, overflow);
     /* Randomize the projection to defend against multiplier sidechannels.

src/field.h

@@ -85,7 +85,8 @@ static const secp256k1_fe secp256k1_const_beta = SECP256K1_FE_CONST(
 #  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_set_b32_mod secp256k1_fe_impl_set_b32_mod
+#  define secp256k1_fe_set_b32_limit secp256k1_fe_impl_set_b32_limit
 #  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
@@ -189,16 +190,20 @@ static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b);
  */
 static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b);
 
-/** Set a field element equal to a provided 32-byte big endian value.
+/** Set a field element equal to a provided 32-byte big endian value, reducing it.
  *
  * 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).
+ * On output, r = a (mod p). It will have magnitude 1, and not be normalized.
+ */
+static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a);
+
+/** Set a field element equal to a provided 32-byte big endian value, checking for overflow.
  *
- * Note that this function is unusual in that the normalization of the output depends on the
- * run-time value of a.
+ * On input, r does not need to be initalized. a must be a pointer to an initialized 32-byte array.
+ * On output, r = a if (a < p), it will be normalized with magnitude 1, and 1 is returned.
+ * If a >= p, 0 is returned, and r will be made invalid (and must not be used without overwriting).
  */
-static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a);
+static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a);
 
 /** 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.

src/field_10x26_impl.h

@@ -290,7 +290,7 @@ static int secp256k1_fe_impl_cmp_var(const secp256k1_fe *a, const secp256k1_fe *
     return 0;
 }
 
-static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
+static void secp256k1_fe_impl_set_b32_mod(secp256k1_fe *r, const unsigned char *a) {
     r->n[0] = (uint32_t)a[31] | ((uint32_t)a[30] << 8) | ((uint32_t)a[29] << 16) | ((uint32_t)(a[28] & 0x3) << 24);
     r->n[1] = (uint32_t)((a[28] >> 2) & 0x3f) | ((uint32_t)a[27] << 6) | ((uint32_t)a[26] << 14) | ((uint32_t)(a[25] & 0xf) << 22);
     r->n[2] = (uint32_t)((a[25] >> 4) & 0xf) | ((uint32_t)a[24] << 4) | ((uint32_t)a[23] << 12) | ((uint32_t)(a[22] & 0x3f) << 20);
@@ -301,7 +301,10 @@ static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
     r->n[7] = (uint32_t)((a[9] >> 6) & 0x3) | ((uint32_t)a[8] << 2) | ((uint32_t)a[7] << 10) | ((uint32_t)a[6] << 18);
     r->n[8] = (uint32_t)a[5] | ((uint32_t)a[4] << 8) | ((uint32_t)a[3] << 16) | ((uint32_t)(a[2] & 0x3) << 24);
     r->n[9] = (uint32_t)((a[2] >> 2) & 0x3f) | ((uint32_t)a[1] << 6) | ((uint32_t)a[0] << 14);
+}
 
+static int secp256k1_fe_impl_set_b32_limit(secp256k1_fe *r, const unsigned char *a) {
+    secp256k1_fe_impl_set_b32_mod(r, a);
     return !((r->n[9] == 0x3FFFFFUL) & ((r->n[8] & r->n[7] & r->n[6] & r->n[5] & r->n[4] & r->n[3] & r->n[2]) == 0x3FFFFFFUL) & ((r->n[1] + 0x40UL + ((r->n[0] + 0x3D1UL) >> 26)) > 0x3FFFFFFUL));
 }
 

src/field_5x52_impl.h

@@ -236,7 +236,7 @@ static int secp256k1_fe_impl_cmp_var(const secp256k1_fe *a, const secp256k1_fe *
     return 0;
 }
 
-static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
+static void secp256k1_fe_impl_set_b32_mod(secp256k1_fe *r, const unsigned char *a) {
     r->n[0] = (uint64_t)a[31]
             | ((uint64_t)a[30] << 8)
             | ((uint64_t)a[29] << 16)
@@ -271,6 +271,10 @@ static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
             | ((uint64_t)a[2] << 24)
             | ((uint64_t)a[1] << 32)
             | ((uint64_t)a[0] << 40);
+}
+
+static int secp256k1_fe_impl_set_b32_limit(secp256k1_fe *r, const unsigned char *a) {
+    secp256k1_fe_impl_set_b32_mod(r, a);
     return !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL));
 }
 

src/field_impl.h

@@ -260,13 +260,26 @@ SECP256K1_INLINE static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const se
     return secp256k1_fe_impl_cmp_var(a, b);
 }
 
-static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a);
-SECP256K1_INLINE static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) {
-    int ret = secp256k1_fe_impl_set_b32(r, a);
+static void secp256k1_fe_impl_set_b32_mod(secp256k1_fe *r, const unsigned char *a);
+SECP256K1_INLINE static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a) {
+    secp256k1_fe_impl_set_b32_mod(r, a);
     r->magnitude = 1;
-    r->normalized = ret;
+    r->normalized = 0;
     secp256k1_fe_verify(r);
-    return ret;
+}
+
+static int secp256k1_fe_impl_set_b32_limit(secp256k1_fe *r, const unsigned char *a);
+SECP256K1_INLINE static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a) {
+    if (secp256k1_fe_impl_set_b32_limit(r, a)) {
+        r->magnitude = 1;
+        r->normalized = 1;
+        secp256k1_fe_verify(r);
+        return 1;
+    } else {
+        /* Mark the output field element as invalid. */
+        r->magnitude = -1;
+        return 0;
+    }
 }
 
 static void secp256k1_fe_impl_get_b32(unsigned char *r, const secp256k1_fe *a);

src/modules/extrakeys/main_impl.h

@@ -28,7 +28,7 @@ int secp256k1_xonly_pubkey_parse(const secp256k1_context* ctx, secp256k1_xonly_p
     memset(pubkey, 0, sizeof(*pubkey));
     ARG_CHECK(input32 != NULL);
 
-    if (!secp256k1_fe_set_b32(&x, input32)) {
+    if (!secp256k1_fe_set_b32_limit(&x, input32)) {
         return 0;
     }
     if (!secp256k1_ge_set_xo_var(&pk, &x, 0)) {

src/modules/extrakeys/tests_exhaustive_impl.h

@@ -47,7 +47,7 @@ static void test_exhaustive_extrakeys(const secp256k1_context *ctx, const secp25
         CHECK(secp256k1_memcmp_var(xonly_pubkey_bytes[i - 1], buf, 32) == 0);
 
         /* Compare the xonly_pubkey bytes against the precomputed group. */
-        secp256k1_fe_set_b32(&fe, xonly_pubkey_bytes[i - 1]);
+        secp256k1_fe_set_b32_mod(&fe, xonly_pubkey_bytes[i - 1]);
         CHECK(secp256k1_fe_equal_var(&fe, &group[i].x));
 
         /* Check the parity against the precomputed group. */

src/modules/recovery/main_impl.h

@@ -98,7 +98,7 @@ static int secp256k1_ecdsa_sig_recover(const secp256k1_scalar *sigr, const secp2
     }
 
     secp256k1_scalar_get_b32(brx, sigr);
-    r = secp256k1_fe_set_b32(&fx, brx);
+    r = secp256k1_fe_set_b32_limit(&fx, brx);
     (void)r;
     VERIFY_CHECK(r); /* brx comes from a scalar, so is less than the order; certainly less than p */
     if (recid & 2) {

src/modules/schnorrsig/main_impl.h

@@ -232,7 +232,7 @@ int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned cha
     ARG_CHECK(msg != NULL || msglen == 0);
     ARG_CHECK(pubkey != NULL);
 
-    if (!secp256k1_fe_set_b32(&rx, &sig64[0])) {
+    if (!secp256k1_fe_set_b32_limit(&rx, &sig64[0])) {
         return 0;
     }
 

src/secp256k1.c

@@ -247,8 +247,8 @@ static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge,
     } else {
         /* Otherwise, fall back to 32-byte big endian for X and Y. */
         secp256k1_fe x, y;
-        secp256k1_fe_set_b32(&x, pubkey->data);
-        secp256k1_fe_set_b32(&y, pubkey->data + 32);
+        secp256k1_fe_set_b32_mod(&x, pubkey->data);
+        secp256k1_fe_set_b32_mod(&y, pubkey->data + 32);
         secp256k1_ge_set_xy(ge, &x, &y);
     }
     ARG_CHECK(!secp256k1_fe_is_zero(&ge->x));

src/tests.c

@@ -90,7 +90,7 @@ static void random_field_element_test(secp256k1_fe *fe) {
     do {
         unsigned char b32[32];
         secp256k1_testrand256_test(b32);
-        if (secp256k1_fe_set_b32(fe, b32)) {
+        if (secp256k1_fe_set_b32_limit(fe, b32)) {
             break;
         }
     } while(1);
@@ -2957,7 +2957,7 @@ static void random_fe(secp256k1_fe *x) {
     unsigned char bin[32];
     do {
         secp256k1_testrand256(bin);
-        if (secp256k1_fe_set_b32(x, bin)) {
+        if (secp256k1_fe_set_b32_limit(x, bin)) {
             return;
         }
     } while(1);
@@ -2967,7 +2967,7 @@ static void random_fe_test(secp256k1_fe *x) {
     unsigned char bin[32];
     do {
         secp256k1_testrand256_test(bin);
-        if (secp256k1_fe_set_b32(x, bin)) {
+        if (secp256k1_fe_set_b32_limit(x, bin)) {
             return;
         }
     } while(1);
@@ -3021,7 +3021,7 @@ static void run_field_convert(void) {
     unsigned char b322[32];
     secp256k1_fe_storage fes2;
     /* Check conversions to fe. */
-    CHECK(secp256k1_fe_set_b32(&fe2, b32));
+    CHECK(secp256k1_fe_set_b32_limit(&fe2, b32));
     CHECK(secp256k1_fe_equal_var(&fe, &fe2));
     secp256k1_fe_from_storage(&fe2, &fes);
     CHECK(secp256k1_fe_equal_var(&fe, &fe2));
@@ -3043,7 +3043,8 @@ static void run_field_be32_overflow(void) {
         static const unsigned char zero[32] = { 0x00 };
         unsigned char out[32];
         secp256k1_fe fe;
-        CHECK(secp256k1_fe_set_b32(&fe, zero_overflow) == 0);
+        CHECK(secp256k1_fe_set_b32_limit(&fe, zero_overflow) == 0);
+        secp256k1_fe_set_b32_mod(&fe, zero_overflow);
         CHECK(secp256k1_fe_normalizes_to_zero(&fe) == 1);
         secp256k1_fe_normalize(&fe);
         CHECK(secp256k1_fe_is_zero(&fe) == 1);
@@ -3065,7 +3066,8 @@ static void run_field_be32_overflow(void) {
         };
         unsigned char out[32];
         secp256k1_fe fe;
-        CHECK(secp256k1_fe_set_b32(&fe, one_overflow) == 0);
+        CHECK(secp256k1_fe_set_b32_limit(&fe, one_overflow) == 0);
+        secp256k1_fe_set_b32_mod(&fe, one_overflow);
         secp256k1_fe_normalize(&fe);
         CHECK(secp256k1_fe_cmp_var(&fe, &secp256k1_fe_one) == 0);
         secp256k1_fe_get_b32(out, &fe);
@@ -3087,7 +3089,8 @@ static void run_field_be32_overflow(void) {
         unsigned char out[32];
         secp256k1_fe fe;
         const secp256k1_fe fe_ff = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0x01, 0x000003d0);
-        CHECK(secp256k1_fe_set_b32(&fe, ff_overflow) == 0);
+        CHECK(secp256k1_fe_set_b32_limit(&fe, ff_overflow) == 0);
+        secp256k1_fe_set_b32_mod(&fe, ff_overflow);
         secp256k1_fe_normalize(&fe);
         CHECK(secp256k1_fe_cmp_var(&fe, &fe_ff) == 0);
         secp256k1_fe_get_b32(out, &fe);
@@ -3673,7 +3676,7 @@ static void run_inverse_tests(void)
         b32[31] = i & 0xff;
         b32[30] = (i >> 8) & 0xff;
         secp256k1_scalar_set_b32(&x_scalar, b32, NULL);
-        secp256k1_fe_set_b32(&x_fe, b32);
+        secp256k1_fe_set_b32_mod(&x_fe, b32);
         for (var = 0; var <= 1; ++var) {
             test_inverse_scalar(NULL, &x_scalar, var);
             test_inverse_field(NULL, &x_fe, var);
@@ -3690,7 +3693,7 @@ static void run_inverse_tests(void)
         for (i = 0; i < 64 * COUNT; ++i) {
             (testrand ? secp256k1_testrand256_test : secp256k1_testrand256)(b32);
             secp256k1_scalar_set_b32(&x_scalar, b32, NULL);
-            secp256k1_fe_set_b32(&x_fe, b32);
+            secp256k1_fe_set_b32_mod(&x_fe, b32);
             for (var = 0; var <= 1; ++var) {
                 test_inverse_scalar(NULL, &x_scalar, var);
                 test_inverse_field(NULL, &x_fe, var);

src/tests_exhaustive.c

@@ -60,7 +60,7 @@ static void random_fe(secp256k1_fe *x) {
     unsigned char bin[32];
     do {
         secp256k1_testrand256(bin);
-        if (secp256k1_fe_set_b32(x, bin)) {
+        if (secp256k1_fe_set_b32_limit(x, bin)) {
             return;
         }
     } while(1);