From 659b554d7b1e6c75684adbcd181ba5777e476690 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 27 Nov 2014 19:12:13 +0100 Subject: [PATCH] Make constant initializers independent from num --- src/ecdsa_impl.h | 10 +++- src/group.h | 2 - src/group_impl.h | 9 --- src/scalar.h | 3 + src/scalar_impl.h | 20 +++++-- src/tests.c | 142 ++++++++++++++++++++++------------------------ 6 files changed, 92 insertions(+), 94 deletions(-) diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 525516e7bf..81b67e825d 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -29,10 +29,14 @@ static void secp256k1_ecdsa_start(void) { /* Allocate. */ secp256k1_ecdsa_consts_t *ret = (secp256k1_ecdsa_consts_t*)malloc(sizeof(secp256k1_ecdsa_consts_t)); - unsigned char p[32]; - secp256k1_num_get_bin(p, 32, &secp256k1_ge_consts->order); - secp256k1_fe_set_b32(&ret->order_as_fe, p); + static const unsigned char order[] = { + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFE, + 0xBA,0xAE,0xDC,0xE6,0xAF,0x48,0xA0,0x3B, + 0xBF,0xD2,0x5E,0x8C,0xD0,0x36,0x41,0x41 + }; + secp256k1_fe_set_b32(&ret->order_as_fe, order); secp256k1_fe_negate(&ret->p_minus_order, &ret->order_as_fe, 1); secp256k1_fe_normalize(&ret->p_minus_order); diff --git a/src/group.h b/src/group.h index 1600e96a27..b0d42721da 100644 --- a/src/group.h +++ b/src/group.h @@ -27,8 +27,6 @@ typedef struct { /** Global constants related to the group */ typedef struct { - secp256k1_num_t order; /* the order of the curve (= order of its generator) */ - secp256k1_num_t half_order; /* half the order of the curve (= order of its generator) */ secp256k1_ge_t g; /* the generator point */ #ifdef USE_ENDOMORPHISM diff --git a/src/group_impl.h b/src/group_impl.h index 8d535bc4d9..7be5787d15 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -413,12 +413,6 @@ static void secp256k1_gej_mul_lambda(secp256k1_gej_t *r, const secp256k1_gej_t * static void secp256k1_ge_start(void) { - static const unsigned char secp256k1_ge_consts_order[] = { - 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, - 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFE, - 0xBA,0xAE,0xDC,0xE6,0xAF,0x48,0xA0,0x3B, - 0xBF,0xD2,0x5E,0x8C,0xD0,0x36,0x41,0x41 - }; static const unsigned char secp256k1_ge_consts_g_x[] = { 0x79,0xBE,0x66,0x7E,0xF9,0xDC,0xBB,0xAC, 0x55,0xA0,0x62,0x95,0xCE,0x87,0x0B,0x07, @@ -442,9 +436,6 @@ static void secp256k1_ge_start(void) { #endif if (secp256k1_ge_consts == NULL) { secp256k1_ge_consts_t *ret = (secp256k1_ge_consts_t*)malloc(sizeof(secp256k1_ge_consts_t)); - secp256k1_num_set_bin(&ret->order, secp256k1_ge_consts_order, sizeof(secp256k1_ge_consts_order)); - secp256k1_num_copy(&ret->half_order, &ret->order); - secp256k1_num_shift(&ret->half_order, 1); #ifdef USE_ENDOMORPHISM VERIFY_CHECK(secp256k1_fe_set_b32(&ret->beta, secp256k1_ge_consts_beta)); #endif diff --git a/src/scalar.h b/src/scalar.h index 4b5d73fdeb..5b38c60acb 100644 --- a/src/scalar.h +++ b/src/scalar.h @@ -75,6 +75,9 @@ static int secp256k1_scalar_is_high(const secp256k1_scalar_t *a); /** Convert a scalar to a number. */ static void secp256k1_scalar_get_num(secp256k1_num_t *r, const secp256k1_scalar_t *a); +/** Get the order of the group as a number. */ +static void secp256k1_scalar_order_get_num(secp256k1_num_t *r); + /** Compare two scalars. */ static int secp256k1_scalar_eq(const secp256k1_scalar_t *a, const secp256k1_scalar_t *b); diff --git a/src/scalar_impl.h b/src/scalar_impl.h index dcddf3b413..91f9bb11f6 100644 --- a/src/scalar_impl.h +++ b/src/scalar_impl.h @@ -25,6 +25,7 @@ #endif typedef struct { + secp256k1_num_t order; #ifdef USE_ENDOMORPHISM secp256k1_num_t a1b2, b1, a2; #endif @@ -39,6 +40,13 @@ static void secp256k1_scalar_start(void) { /* Allocate. */ secp256k1_scalar_consts_t *ret = (secp256k1_scalar_consts_t*)malloc(sizeof(secp256k1_scalar_consts_t)); + static const unsigned char secp256k1_scalar_consts_order[] = { + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFE, + 0xBA,0xAE,0xDC,0xE6,0xAF,0x48,0xA0,0x3B, + 0xBF,0xD2,0x5E,0x8C,0xD0,0x36,0x41,0x41 + }; + secp256k1_num_set_bin(&ret->order, secp256k1_scalar_consts_order, sizeof(secp256k1_scalar_consts_order)); #ifdef USE_ENDOMORPHISM static const unsigned char secp256k1_scalar_consts_a1b2[] = { 0x30,0x86,0xd2,0x21,0xa7,0xd4,0x6b,0xcd, @@ -78,6 +86,9 @@ static void secp256k1_scalar_get_num(secp256k1_num_t *r, const secp256k1_scalar_ secp256k1_num_set_bin(r, c, 32); } +static void secp256k1_scalar_order_get_num(secp256k1_num_t *r) { + *r = secp256k1_scalar_consts->order; +} static void secp256k1_scalar_inverse(secp256k1_scalar_t *r, const secp256k1_scalar_t *x) { /* First compute x ^ (2^N - 1) for some values of N. */ @@ -238,7 +249,7 @@ static void secp256k1_scalar_inverse_var(secp256k1_scalar_t *r, const secp256k1_ secp256k1_scalar_get_b32(b, x); secp256k1_num_t n; secp256k1_num_set_bin(&n, b, 32); - secp256k1_num_mod_inverse(&n, &n, &secp256k1_ge_consts->order); + secp256k1_num_mod_inverse(&n, &n, &secp256k1_scalar_consts->order); secp256k1_num_get_bin(b, 32, &n); secp256k1_scalar_set_b32(r, b, NULL); #else @@ -256,19 +267,18 @@ static void secp256k1_scalar_split_lambda_var(secp256k1_scalar_t *r1, secp256k1_ secp256k1_num_t rn1, rn2; const secp256k1_scalar_consts_t *c = secp256k1_scalar_consts; - const secp256k1_num_t *order = &secp256k1_ge_consts->order; secp256k1_num_t bnc1, bnc2, bnt1, bnt2, bnn2; - secp256k1_num_copy(&bnn2, order); + secp256k1_num_copy(&bnn2, &c->order); secp256k1_num_shift(&bnn2, 1); secp256k1_num_mul(&bnc1, &na, &c->a1b2); secp256k1_num_add(&bnc1, &bnc1, &bnn2); - secp256k1_num_div(&bnc1, &bnc1, order); + secp256k1_num_div(&bnc1, &bnc1, &c->order); secp256k1_num_mul(&bnc2, &na, &c->b1); secp256k1_num_add(&bnc2, &bnc2, &bnn2); - secp256k1_num_div(&bnc2, &bnc2, order); + secp256k1_num_div(&bnc2, &bnc2, &c->order); secp256k1_num_mul(&bnt1, &bnc1, &c->a1b2); secp256k1_num_mul(&bnt2, &bnc2, &c->a2); diff --git a/src/tests.c b/src/tests.c index 6830f745fd..48f96068e0 100644 --- a/src/tests.c +++ b/src/tests.c @@ -23,23 +23,13 @@ static int count = 64; -/***** NUM TESTS *****/ - -void random_num_negate(secp256k1_num_t *num) { - if (secp256k1_rand32() & 1) - secp256k1_num_negate(num); -} - void random_field_element_test(secp256k1_fe_t *fe) { do { unsigned char b32[32]; secp256k1_rand256_test(b32); - secp256k1_num_t num; - secp256k1_num_set_bin(&num, b32, 32); - if (secp256k1_num_cmp(&num, &secp256k1_fe_consts->p) >= 0) - continue; - VERIFY_CHECK(secp256k1_fe_set_b32(fe, b32)); - break; + if (secp256k1_fe_set_b32(fe, b32)) { + break; + } } while(1); } @@ -75,19 +65,6 @@ void random_group_element_jacobian_test(secp256k1_gej_t *gej, const secp256k1_ge gej->infinity = ge->infinity; } -void random_num_order_test(secp256k1_num_t *num) { - do { - unsigned char b32[32]; - secp256k1_rand256_test(b32); - secp256k1_num_set_bin(num, b32, 32); - if (secp256k1_num_is_zero(num)) - continue; - if (secp256k1_num_cmp(num, &secp256k1_ge_consts->order) >= 0) - continue; - break; - } while(1); -} - void random_scalar_order_test(secp256k1_scalar_t *num) { do { unsigned char b32[32]; @@ -112,17 +89,23 @@ void random_scalar_order(secp256k1_scalar_t *num) { } while(1); } +/***** NUM TESTS *****/ + +void random_num_negate(secp256k1_num_t *num) { + if (secp256k1_rand32() & 1) + secp256k1_num_negate(num); +} + +void random_num_order_test(secp256k1_num_t *num) { + secp256k1_scalar_t sc; + random_scalar_order_test(&sc); + secp256k1_scalar_get_num(num, &sc); +} + void random_num_order(secp256k1_num_t *num) { - do { - unsigned char b32[32]; - secp256k1_rand256(b32); - secp256k1_num_set_bin(num, b32, 32); - if (secp256k1_num_is_zero(num)) - continue; - if (secp256k1_num_cmp(num, &secp256k1_ge_consts->order) >= 0) - continue; - break; - } while(1); + secp256k1_scalar_t sc; + random_scalar_order(&sc); + secp256k1_scalar_get_num(num, &sc); } void test_num_get_set_bin(void) { @@ -208,29 +191,27 @@ void scalar_test(void) { unsigned char c[32]; /* Set 's' to a random scalar, with value 'snum'. */ - secp256k1_rand256_test(c); secp256k1_scalar_t s; - secp256k1_scalar_set_b32(&s, c, NULL); - secp256k1_num_t snum; - secp256k1_num_set_bin(&snum, c, 32); - secp256k1_num_mod(&snum, &secp256k1_ge_consts->order); + random_scalar_order_test(&s); /* Set 's1' to a random scalar, with value 's1num'. */ - secp256k1_rand256_test(c); secp256k1_scalar_t s1; - secp256k1_scalar_set_b32(&s1, c, NULL); - secp256k1_num_t s1num; - secp256k1_num_set_bin(&s1num, c, 32); - secp256k1_num_mod(&s1num, &secp256k1_ge_consts->order); + random_scalar_order_test(&s1); /* Set 's2' to a random scalar, with value 'snum2', and byte array representation 'c'. */ - secp256k1_rand256_test(c); secp256k1_scalar_t s2; - int overflow = 0; - secp256k1_scalar_set_b32(&s2, c, &overflow); - secp256k1_num_t s2num; - secp256k1_num_set_bin(&s2num, c, 32); - secp256k1_num_mod(&s2num, &secp256k1_ge_consts->order); + random_scalar_order_test(&s2); + secp256k1_scalar_get_b32(c, &s2); + + secp256k1_num_t snum, s1num, s2num; + secp256k1_scalar_get_num(&snum, &s); + secp256k1_scalar_get_num(&s1num, &s1); + secp256k1_scalar_get_num(&s2num, &s2); + + secp256k1_num_t order; + secp256k1_scalar_order_get_num(&order); + secp256k1_num_t half_order = order; + secp256k1_num_shift(&half_order, 1); { /* Test that fetching groups of 4 bits from a scalar and recursing n(i)=16*n(i-1)+p(i) reconstructs it. */ @@ -268,22 +249,11 @@ void scalar_test(void) { CHECK(secp256k1_scalar_eq(&n, &s)); } - { - /* Test that get_b32 returns the same as get_bin on the number. */ - unsigned char r1[32]; - secp256k1_scalar_get_b32(r1, &s2); - unsigned char r2[32]; - secp256k1_num_get_bin(r2, 32, &s2num); - CHECK(memcmp(r1, r2, 32) == 0); - /* If no overflow occurred when assigning, it should also be equal to the original byte array. */ - CHECK((memcmp(r1, c, 32) == 0) == (overflow == 0)); - } - { /* Test that adding the scalars together is equal to adding their numbers together modulo the order. */ secp256k1_num_t rnum; secp256k1_num_add(&rnum, &snum, &s2num); - secp256k1_num_mod(&rnum, &secp256k1_ge_consts->order); + secp256k1_num_mod(&rnum, &order); secp256k1_scalar_t r; secp256k1_scalar_add(&r, &s, &s2); secp256k1_num_t r2num; @@ -295,7 +265,7 @@ void scalar_test(void) { /* Test that multipying the scalars is equal to multiplying their numbers modulo the order. */ secp256k1_num_t rnum; secp256k1_num_mul(&rnum, &snum, &s2num); - secp256k1_num_mod(&rnum, &secp256k1_ge_consts->order); + secp256k1_num_mod(&rnum, &order); secp256k1_scalar_t r; secp256k1_scalar_mul(&r, &s, &s2); secp256k1_num_t r2num; @@ -312,14 +282,14 @@ void scalar_test(void) { /* Check that comparison with zero matches comparison with zero on the number. */ CHECK(secp256k1_num_is_zero(&snum) == secp256k1_scalar_is_zero(&s)); /* Check that comparison with the half order is equal to testing for high scalar. */ - CHECK(secp256k1_scalar_is_high(&s) == (secp256k1_num_cmp(&snum, &secp256k1_ge_consts->half_order) > 0)); + CHECK(secp256k1_scalar_is_high(&s) == (secp256k1_num_cmp(&snum, &half_order) > 0)); secp256k1_scalar_t neg; secp256k1_scalar_negate(&neg, &s); secp256k1_num_t negnum; - secp256k1_num_sub(&negnum, &secp256k1_ge_consts->order, &snum); - secp256k1_num_mod(&negnum, &secp256k1_ge_consts->order); + secp256k1_num_sub(&negnum, &order, &snum); + secp256k1_num_mod(&negnum, &order); /* Check that comparison with the half order is equal to testing for high scalar after negation. */ - CHECK(secp256k1_scalar_is_high(&neg) == (secp256k1_num_cmp(&negnum, &secp256k1_ge_consts->half_order) > 0)); + CHECK(secp256k1_scalar_is_high(&neg) == (secp256k1_num_cmp(&negnum, &half_order) > 0)); /* Negating should change the high property, unless the value was already zero. */ CHECK((secp256k1_scalar_is_high(&s) == secp256k1_scalar_is_high(&neg)) == secp256k1_scalar_is_zero(&s)); secp256k1_num_t negnum2; @@ -340,7 +310,7 @@ void scalar_test(void) { secp256k1_scalar_t inv; secp256k1_scalar_inverse(&inv, &s); secp256k1_num_t invnum; - secp256k1_num_mod_inverse(&invnum, &snum, &secp256k1_ge_consts->order); + secp256k1_num_mod_inverse(&invnum, &snum, &order); secp256k1_num_t invnum2; secp256k1_scalar_get_num(&invnum2, &inv); CHECK(secp256k1_num_eq(&invnum, &invnum2)); @@ -431,6 +401,28 @@ void run_scalar_tests(void) { for (int i = 0; i < 128 * count; i++) { scalar_test(); } + + { + // (-1)+1 should be zero. + secp256k1_scalar_t s, o; + secp256k1_scalar_set_int(&s, 1); + secp256k1_scalar_negate(&o, &s); + secp256k1_scalar_add(&o, &o, &s); + CHECK(secp256k1_scalar_is_zero(&o)); + } + + { + // A scalar with value of the curve order should be 0. + secp256k1_num_t order; + secp256k1_scalar_order_get_num(&order); + unsigned char bin[32]; + secp256k1_num_get_bin(bin, 32, &order); + secp256k1_scalar_t zero; + int overflow = 0; + secp256k1_scalar_set_b32(&zero, bin, &overflow); + CHECK(overflow == 1); + CHECK(secp256k1_scalar_is_zero(&zero)); + } } /***** FIELD TESTS *****/ @@ -868,11 +860,11 @@ void test_ecdsa_end_to_end(void) { /* Generate a random key and message. */ { - secp256k1_num_t msg, key; - random_num_order_test(&msg); - random_num_order_test(&key); - secp256k1_num_get_bin(privkey, 32, &key); - secp256k1_num_get_bin(message, 32, &msg); + secp256k1_scalar_t msg, key; + random_scalar_order_test(&msg); + random_scalar_order_test(&key); + secp256k1_scalar_get_b32(privkey, &key); + secp256k1_scalar_get_b32(message, &msg); } /* Construct and verify corresponding public key. */