Skip to content

Commit 73f64ff

Browse files
sipasipa
committed
Merge pull request bitcoin#339
9234391 Overhaul flags handling (Pieter Wuille) 1a36898 Make flags more explicit, add runtime checks. (Rusty Russell)
2 parents 1a3e03a + 9234391 commit 73f64ff

File tree

6 files changed

+44
-28
lines changed

6 files changed

+44
-28
lines changed

‎contrib/lax_der_privatekey_parsing.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,22 @@
3434
* privkeylen: Pointer to an int where the length of the private key in
3535
* privkey will be stored.
3636
* In: seckey: pointer to a 32-byte secret key to export.
37-
* flags: SECP256K1_EC_COMPRESSED if the key should be exported in
38-
* compressed format.
37+
* compressed: 1 if the key should be exported in
38+
* compressed format, 0 otherwise
3939
*
4040
* This function is purely meant for compatibility with applications that
4141
* require BER encoded keys. When working with secp256k1-specific code, the
4242
* simple 32-byte private keys are sufficient.
4343
*
4444
* Note that this function does not guarantee correct DER output. It is
45-
* guaranteed to be parsable by secp256k1_ec_privkey_import.
45+
* guaranteed to be parsable by secp256k1_ec_privkey_import_der
4646
*/
4747
static SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_export_der(
4848
const secp256k1_context* ctx,
4949
unsigned char *privkey,
5050
size_t *privkeylen,
5151
const unsigned char *seckey,
52-
unsigned int flags
52+
int compressed
5353
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
5454

5555
/** Import a private key in DER format.
@@ -116,13 +116,13 @@ static int secp256k1_eckey_privkey_parse(secp256k1_scalar *key, const unsigned c
116116
return !overflow;
117117
}
118118

119-
static int secp256k1_eckey_privkey_serialize(const secp256k1_ecmult_gen_context *ctx, unsigned char *privkey, size_t *privkeylen, const secp256k1_scalar *key, unsigned int flags) {
119+
static int secp256k1_eckey_privkey_serialize(const secp256k1_ecmult_gen_context *ctx, unsigned char *privkey, size_t *privkeylen, const secp256k1_scalar *key, int compressed) {
120120
secp256k1_gej rp;
121121
secp256k1_ge r;
122122
size_t pubkeylen = 0;
123123
secp256k1_ecmult_gen(ctx, &rp, key);
124124
secp256k1_ge_set_gej(&r, &rp);
125-
if (flags & SECP256K1_EC_COMPRESSED) {
125+
if (compressed) {
126126
static const unsigned char begin[] = {
127127
0x30,0x81,0xD3,0x02,0x01,0x01,0x04,0x20
128128
};
@@ -176,7 +176,7 @@ static int secp256k1_eckey_privkey_serialize(const secp256k1_ecmult_gen_context
176176
return 1;
177177
}
178178

179-
static int secp256k1_ec_privkey_export_der(const secp256k1_context* ctx, unsigned char *privkey, size_t *privkeylen, const unsigned char *seckey, unsigned int flags) {
179+
static int secp256k1_ec_privkey_export_der(const secp256k1_context* ctx, unsigned char *privkey, size_t *privkeylen, const unsigned char *seckey, int compressed) {
180180
secp256k1_scalar key;
181181
int ret = 0;
182182
VERIFY_CHECK(ctx != NULL);
@@ -186,7 +186,7 @@ static int secp256k1_ec_privkey_export_der(const secp256k1_context* ctx, unsigne
186186
ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx));
187187

188188
secp256k1_scalar_set_b32(&key, seckey, NULL);
189-
ret = secp256k1_eckey_privkey_serialize(&ctx->ecmult_gen_ctx, privkey, privkeylen, &key, flags);
189+
ret = secp256k1_eckey_privkey_serialize(&ctx->ecmult_gen_ctx, privkey, privkeylen, &key, compressed);
190190
secp256k1_scalar_clear(&key);
191191
return ret;
192192
}

‎include/secp256k1.h‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,23 @@ typedef int (*secp256k1_nonce_function)(
147147
# define SECP256K1_ARG_NONNULL(_x)
148148
# endif
149149

150+
/** All flags' lower 8 bits indicate what they're for. Do not use directly. */
151+
#define SECP256K1_FLAGS_TYPE_MASK ((1 << 8) - 1)
152+
#define SECP256K1_FLAGS_TYPE_CONTEXT (1 << 0)
153+
#define SECP256K1_FLAGS_TYPE_COMPRESSION (1 << 1)
154+
/** The higher bits contain the actual data. Do not use directly. */
155+
#define SECP256K1_FLAGS_BIT_CONTEXT_VERIFY (1 << 8)
156+
#define SECP256K1_FLAGS_BIT_CONTEXT_SIGN (1 << 9)
157+
#define SECP256K1_FLAGS_BIT_COMPRESSION (1 << 8)
158+
150159
/** Flags to pass to secp256k1_context_create. */
151-
# define SECP256K1_CONTEXT_VERIFY (1 << 0)
152-
# define SECP256K1_CONTEXT_SIGN (1 << 1)
160+
#define SECP256K1_CONTEXT_VERIFY (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_VERIFY)
161+
#define SECP256K1_CONTEXT_SIGN (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_SIGN)
162+
#define SECP256K1_CONTEXT_NONE (SECP256K1_FLAGS_TYPE_CONTEXT)
153163

154164
/** Flag to pass to secp256k1_ec_pubkey_serialize and secp256k1_ec_privkey_export. */
155-
# define SECP256K1_EC_COMPRESSED (1 << 0)
165+
#define SECP256K1_EC_COMPRESSED (SECP256K1_FLAGS_TYPE_COMPRESSION | SECP256K1_FLAGS_BIT_COMPRESSION)
166+
#define SECP256K1_EC_UNCOMPRESSED (SECP256K1_FLAGS_TYPE_COMPRESSION)
156167

157168
/** Create a secp256k1 context object.
158169
*
@@ -261,7 +272,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_parse(
261272
* In: pubkey: a pointer to a secp256k1_pubkey containing an initialized
262273
* public key.
263274
* flags: SECP256K1_EC_COMPRESSED if serialization should be in
264-
* compressed format.
275+
* compressed format, otherwise SECP256K1_EC_UNCOMPRESSED.
265276
*/
266277
SECP256K1_API int secp256k1_ec_pubkey_serialize(
267278
const secp256k1_context* ctx,

‎src/eckey.h‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@
1515
#include "ecmult_gen.h"
1616

1717
static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char *pub, size_t size);
18-
static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, unsigned int flags);
19-
20-
static int secp256k1_eckey_privkey_parse(secp256k1_scalar *key, const unsigned char *privkey, size_t privkeylen);
21-
static int secp256k1_eckey_privkey_serialize(const secp256k1_ecmult_gen_context *ctx, unsigned char *privkey, size_t *privkeylen, const secp256k1_scalar *key, unsigned int flags);
18+
static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed);
2219

2320
static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp256k1_scalar *tweak);
2421
static int secp256k1_eckey_pubkey_tweak_add(const secp256k1_ecmult_context *ctx, secp256k1_ge *key, const secp256k1_scalar *tweak);

‎src/eckey_impl.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char
3333
}
3434
}
3535

36-
static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, unsigned int flags) {
36+
static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed) {
3737
if (secp256k1_ge_is_infinity(elem)) {
3838
return 0;
3939
}
4040
secp256k1_fe_normalize_var(&elem->x);
4141
secp256k1_fe_normalize_var(&elem->y);
4242
secp256k1_fe_get_b32(&pub[1], &elem->x);
43-
if (flags & SECP256K1_EC_COMPRESSED) {
43+
if (compressed) {
4444
*size = 33;
4545
pub[0] = 0x02 | (secp256k1_fe_is_odd(&elem->y) ? 0x01 : 0x00);
4646
} else {

‎src/secp256k1.c‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,20 @@ secp256k1_context* secp256k1_context_create(unsigned int flags) {
6262
ret->illegal_callback = default_illegal_callback;
6363
ret->error_callback = default_error_callback;
6464

65+
if (EXPECT((flags & SECP256K1_FLAGS_TYPE_MASK) != SECP256K1_FLAGS_TYPE_CONTEXT, 0)) {
66+
secp256k1_callback_call(&ret->illegal_callback,
67+
"Invalid flags");
68+
free(ret);
69+
return NULL;
70+
}
71+
6572
secp256k1_ecmult_context_init(&ret->ecmult_ctx);
6673
secp256k1_ecmult_gen_context_init(&ret->ecmult_gen_ctx);
6774

68-
if (flags & SECP256K1_CONTEXT_SIGN) {
75+
if (flags & SECP256K1_FLAGS_BIT_CONTEXT_SIGN) {
6976
secp256k1_ecmult_gen_context_build(&ret->ecmult_gen_ctx, &ret->error_callback);
7077
}
71-
if (flags & SECP256K1_CONTEXT_VERIFY) {
78+
if (flags & SECP256K1_FLAGS_BIT_CONTEXT_VERIFY) {
7279
secp256k1_ecmult_context_build(&ret->ecmult_ctx, &ret->error_callback);
7380
}
7481

@@ -166,8 +173,9 @@ int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *o
166173
ARG_CHECK(output != NULL);
167174
ARG_CHECK(outputlen != NULL);
168175
ARG_CHECK(pubkey != NULL);
176+
ARG_CHECK((flags & SECP256K1_FLAGS_TYPE_MASK) == SECP256K1_FLAGS_TYPE_COMPRESSION);
169177
return (secp256k1_pubkey_load(ctx, &Q, pubkey) &&
170-
secp256k1_eckey_pubkey_serialize(&Q, output, outputlen, flags));
178+
secp256k1_eckey_pubkey_serialize(&Q, output, outputlen, flags & SECP256K1_FLAGS_BIT_COMPRESSION));
171179
}
172180

173181
static void secp256k1_ecdsa_signature_load(const secp256k1_context* ctx, secp256k1_scalar* r, secp256k1_scalar* s, const secp256k1_ecdsa_signature* sig) {

‎src/tests.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ void run_context_tests(void) {
139139
unsigned char ctmp[32];
140140
int32_t ecount;
141141
int32_t ecount2;
142-
secp256k1_context *none = secp256k1_context_create(0);
142+
secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
143143
secp256k1_context *sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
144144
secp256k1_context *vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
145145
secp256k1_context *both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
@@ -1964,7 +1964,7 @@ void ec_pubkey_parse_pointtest(const unsigned char *input, int xvalid, int yvali
19641964
VG_CHECK(&pubkey, sizeof(pubkey));
19651965
outl = 65;
19661966
VG_UNDEF(pubkeyo, 65);
1967-
CHECK(secp256k1_ec_pubkey_serialize(ctx, pubkeyo, &outl, &pubkey, 0) == 1);
1967+
CHECK(secp256k1_ec_pubkey_serialize(ctx, pubkeyo, &outl, &pubkey, SECP256K1_EC_UNCOMPRESSED) == 1);
19681968
VG_CHECK(pubkeyo, outl);
19691969
CHECK(outl == 65);
19701970
CHECK(pubkeyo[0] == 4);
@@ -2575,12 +2575,12 @@ void test_ecdsa_end_to_end(void) {
25752575
CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, privkey) == 1);
25762576

25772577
/* Verify exporting and importing public key. */
2578-
CHECK(secp256k1_ec_pubkey_serialize(ctx, pubkeyc, &pubkeyclen, &pubkey, secp256k1_rand_bits(1)) == 1);
2578+
CHECK(secp256k1_ec_pubkey_serialize(ctx, pubkeyc, &pubkeyclen, &pubkey, secp256k1_rand_bits(1) == 1 ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED));
25792579
memset(&pubkey, 0, sizeof(pubkey));
25802580
CHECK(secp256k1_ec_pubkey_parse(ctx, &pubkey, pubkeyc, pubkeyclen) == 1);
25812581

25822582
/* Verify private key import and export. */
2583-
CHECK(secp256k1_ec_privkey_export_der(ctx, seckey, &seckeylen, privkey, secp256k1_rand_bits(1) == 1) ? SECP256K1_EC_COMPRESSED : 0);
2583+
CHECK(secp256k1_ec_privkey_export_der(ctx, seckey, &seckeylen, privkey, secp256k1_rand_bits(1) == 1));
25842584
CHECK(secp256k1_ec_privkey_import_der(ctx, privkey2, seckey, seckeylen) == 1);
25852585
CHECK(memcmp(privkey, privkey2, 32) == 0);
25862586

@@ -2698,7 +2698,7 @@ void test_random_pubkeys(void) {
26982698
size_t size = len;
26992699
firstb = in[0];
27002700
/* If the pubkey can be parsed, it should round-trip... */
2701-
CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, (len == 33) ? SECP256K1_EC_COMPRESSED : 0));
2701+
CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, len == 33));
27022702
CHECK(size == len);
27032703
CHECK(memcmp(&in[1], &out[1], len-1) == 0);
27042704
/* ... except for the type of hybrid inputs. */
@@ -3401,7 +3401,7 @@ void test_ecdsa_edge_cases(void) {
34013401
size_t outlen = 300;
34023402
CHECK(!secp256k1_ec_privkey_export_der(ctx, privkey, &outlen, seckey, 0));
34033403
outlen = 300;
3404-
CHECK(!secp256k1_ec_privkey_export_der(ctx, privkey, &outlen, seckey, SECP256K1_EC_COMPRESSED));
3404+
CHECK(!secp256k1_ec_privkey_export_der(ctx, privkey, &outlen, seckey, 1));
34053405
}
34063406
}
34073407

@@ -3416,7 +3416,7 @@ EC_KEY *get_openssl_key(const secp256k1_scalar *key) {
34163416
const unsigned char* pbegin = privkey;
34173417
int compr = secp256k1_rand_bits(1);
34183418
EC_KEY *ec_key = EC_KEY_new_by_curve_name(NID_secp256k1);
3419-
CHECK(secp256k1_eckey_privkey_serialize(&ctx->ecmult_gen_ctx, privkey, &privkeylen, key, compr ? SECP256K1_EC_COMPRESSED : 0));
3419+
CHECK(secp256k1_eckey_privkey_serialize(&ctx->ecmult_gen_ctx, privkey, &privkeylen, key, compr));
34203420
CHECK(d2i_ECPrivateKey(&ec_key, &pbegin, privkeylen));
34213421
CHECK(EC_KEY_check_key(ec_key));
34223422
return ec_key;

0 commit comments

Comments
 (0)