Skip to content

Commit 1f33bb2

Browse files
jonasnickjonasnick
committed
Merge #1205: field: Improve docs +tests of secp256k1_fe_set_b32
162da73 tests: Add debug helper for printing buffers (Tim Ruffing) e9fd3df field: Improve docs and tests of secp256k1_fe_set_b32 (Tim Ruffing) ca92a35 field: Simplify code in secp256k1_fe_set_b32 (Tim Ruffing) d93f62e field: Verify field element even after secp256k1_fe_set_b32 fails (Tim Ruffing) Pull request description: ACKs for top commit: jonasnick: ACK 162da73 Tree-SHA512: b3ed8e45c969d0420275ff154462f3820b72b57832ccba1f6f427e0cfd9cff3e27440c20994f69ea33a576b1903eb7f04a989f0dbd574bbd96ee56c6dd4500f7
2 parents f6bef03 + 162da73 commit 1f33bb2

File tree

4 files changed

+90
-13
lines changed

4 files changed

+90
-13
lines changed

‎src/field.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b);
7575
/** Compare two field elements. Requires both inputs to be normalized */
7676
static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b);
7777

78-
/** Set a field element equal to 32-byte big endian value. If successful, the resulting field element is normalized. */
78+
/** Set a field element equal to 32-byte big endian value.
79+
* Returns 1 if no overflow occurred, and then the output is normalized.
80+
* Returns 0 if overflow occurred, and then the output is only weakly normalized. */
7981
static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a);
8082

8183
/** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */

‎src/field_10x26_impl.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -367,12 +367,8 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) {
367367
ret = !((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));
368368
#ifdef VERIFY
369369
r->magnitude = 1;
370-
if (ret) {
371-
r->normalized = 1;
372-
secp256k1_fe_verify(r);
373-
} else {
374-
r->normalized = 0;
375-
}
370+
r->normalized = ret;
371+
secp256k1_fe_verify(r);
376372
#endif
377373
return ret;
378374
}

‎src/field_5x52_impl.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,12 +342,8 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) {
342342
ret = !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL));
343343
#ifdef VERIFY
344344
r->magnitude = 1;
345-
if (ret) {
346-
r->normalized = 1;
347-
secp256k1_fe_verify(r);
348-
} else {
349-
r->normalized = 0;
350-
}
345+
r->normalized = ret;
346+
secp256k1_fe_verify(r);
351347
#endif
352348
return ret;
353349
}

‎src/tests.c‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,25 @@ static int all_bytes_equal(const void* s, unsigned char value, size_t n) {
4444
return 1;
4545
}
4646

47+
/* Debug helper for printing arrays of unsigned char. */
48+
#define PRINT_BUF(buf, len) do { \
49+
printf("%s[%lu] = ", #buf, (unsigned long)len); \
50+
print_buf_plain(buf, len); \
51+
} while(0);
52+
static void print_buf_plain(const unsigned char *buf, size_t len) {
53+
size_t i;
54+
printf("{");
55+
for (i = 0; i < len; i++) {
56+
if (i % 8 == 0) {
57+
printf("\n ");
58+
} else {
59+
printf(" ");
60+
}
61+
printf("0x%02X,", buf[i]);
62+
}
63+
printf("\n}\n");
64+
}
65+
4766
/* TODO Use CHECK_ILLEGAL(_VOID) everywhere and get rid of the uncounting callback */
4867
/* CHECK that expr_or_stmt calls the illegal callback of ctx exactly once
4968
*
@@ -3027,6 +3046,69 @@ static void run_field_convert(void) {
30273046
CHECK(secp256k1_memcmp_var(&fes2, &fes, sizeof(fes)) == 0);
30283047
}
30293048

3049+
static void run_field_be32_overflow(void) {
3050+
{
3051+
static const unsigned char zero_overflow[32] = {
3052+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3053+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3054+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3055+
0xFF, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFC, 0x2F,
3056+
};
3057+
static const unsigned char zero[32] = { 0x00 };
3058+
unsigned char out[32];
3059+
secp256k1_fe fe;
3060+
CHECK(secp256k1_fe_set_b32(&fe, zero_overflow) == 0);
3061+
CHECK(secp256k1_fe_normalizes_to_zero(&fe) == 1);
3062+
secp256k1_fe_normalize(&fe);
3063+
CHECK(secp256k1_fe_is_zero(&fe) == 1);
3064+
secp256k1_fe_get_b32(out, &fe);
3065+
CHECK(secp256k1_memcmp_var(out, zero, 32) == 0);
3066+
}
3067+
{
3068+
static const unsigned char one_overflow[32] = {
3069+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3070+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3071+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3072+
0xFF, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFC, 0x30,
3073+
};
3074+
static const unsigned char one[32] = {
3075+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
3076+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
3077+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
3078+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
3079+
};
3080+
unsigned char out[32];
3081+
secp256k1_fe fe;
3082+
CHECK(secp256k1_fe_set_b32(&fe, one_overflow) == 0);
3083+
secp256k1_fe_normalize(&fe);
3084+
CHECK(secp256k1_fe_cmp_var(&fe, &secp256k1_fe_one) == 0);
3085+
secp256k1_fe_get_b32(out, &fe);
3086+
CHECK(secp256k1_memcmp_var(out, one, 32) == 0);
3087+
}
3088+
{
3089+
static const unsigned char ff_overflow[32] = {
3090+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3091+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3092+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3093+
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
3094+
};
3095+
static const unsigned char ff[32] = {
3096+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
3097+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
3098+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
3099+
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x03, 0xD0,
3100+
};
3101+
unsigned char out[32];
3102+
secp256k1_fe fe;
3103+
const secp256k1_fe fe_ff = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0x01, 0x000003d0);
3104+
CHECK(secp256k1_fe_set_b32(&fe, ff_overflow) == 0);
3105+
secp256k1_fe_normalize(&fe);
3106+
CHECK(secp256k1_fe_cmp_var(&fe, &fe_ff) == 0);
3107+
secp256k1_fe_get_b32(out, &fe);
3108+
CHECK(secp256k1_memcmp_var(out, ff, 32) == 0);
3109+
}
3110+
}
3111+
30303112
/* Returns true if two field elements have the same representation. */
30313113
static int fe_identical(const secp256k1_fe *a, const secp256k1_fe *b) {
30323114
int ret = 1;
@@ -7693,6 +7775,7 @@ int main(int argc, char **argv) {
76937775
run_field_half();
76947776
run_field_misc();
76957777
run_field_convert();
7778+
run_field_be32_overflow();
76967779
run_fe_mul();
76977780
run_sqr();
76987781
run_sqrt();

0 commit comments

Comments
 (0)