Merge #147: whitelist: fix SECP256K1_WHITELIST_MAX_N_KEYS constant

27d1c3b whitelist: add test for MAX_N_KEYS (Jonas Nick)
c8ac14d whitelist: fix SECP256K1_WHITELIST_MAX_N_KEYS constant (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    utACK 27d1c3b

Tree-SHA512: 329099b134811462930866f572914075a3210d81fe15a21f48f26e17bc1a4650c31afdcad7a24af8dc4af093b96300386833d68604be05da89c3f7bc0aabf550

Author: jonasnick

include/secp256k1_whitelist.h

@@ -13,7 +13,7 @@
 extern "C" {
 #endif
 
-#define SECP256K1_WHITELIST_MAX_N_KEYS	256
+#define SECP256K1_WHITELIST_MAX_N_KEYS 255
 
 /** Opaque data structure that holds a parsed whitelist proof
  *

src/modules/whitelist/main_impl.h

@@ -144,7 +144,7 @@ int secp256k1_whitelist_signature_parse(const secp256k1_context* ctx, secp256k1_
     }
 
     sig->n_keys = input[0];
-    if (sig->n_keys >= MAX_KEYS || input_len != 1 + 32 * (sig->n_keys + 1)) {
+    if (sig->n_keys > MAX_KEYS || input_len != 1 + 32 * (sig->n_keys + 1)) {
         return 0;
     }
     memcpy(&sig->data[0], &input[1], 32 * (sig->n_keys + 1));

src/modules/whitelist/tests_impl.h

@@ -9,7 +9,39 @@
 
 #include "include/secp256k1_whitelist.h"
 
-void test_whitelist_end_to_end(const size_t n_keys) {
+void test_whitelist_end_to_end_internal(const unsigned char *summed_seckey, const unsigned char *online_seckey, const secp256k1_pubkey *online_pubkeys, const secp256k1_pubkey *offline_pubkeys, const secp256k1_pubkey *sub_pubkey, const size_t signer_i, const size_t n_keys) {
+        unsigned char serialized[32 + 4 + 32 * SECP256K1_WHITELIST_MAX_N_KEYS] = {0};
+        size_t slen = sizeof(serialized);
+        secp256k1_whitelist_signature sig;
+        secp256k1_whitelist_signature sig1;
+
+        CHECK(secp256k1_whitelist_sign(ctx, &sig, online_pubkeys, offline_pubkeys, n_keys, sub_pubkey, online_seckey, summed_seckey, signer_i, NULL, NULL));
+        CHECK(secp256k1_whitelist_verify(ctx, &sig, online_pubkeys, offline_pubkeys, n_keys, sub_pubkey) == 1);
+        /* Check that exchanging keys causes a failure */
+        CHECK(secp256k1_whitelist_verify(ctx, &sig, offline_pubkeys, online_pubkeys, n_keys, sub_pubkey) != 1);
+        /* Serialization round trip */
+        CHECK(secp256k1_whitelist_signature_serialize(ctx, serialized, &slen, &sig) == 1);
+        CHECK(slen == 33 + 32 * n_keys);
+        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen) == 1);
+        /* (Check various bad-length conditions) */
+        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen + 32) == 0);
+        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen + 1) == 0);
+        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen - 1) == 0);
+        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, 0) == 0);
+        CHECK(secp256k1_whitelist_verify(ctx, &sig1, online_pubkeys, offline_pubkeys, n_keys, sub_pubkey) == 1);
+        CHECK(secp256k1_whitelist_verify(ctx, &sig1, offline_pubkeys, online_pubkeys, n_keys, sub_pubkey) != 1);
+
+        /* Test n_keys */
+        CHECK(secp256k1_whitelist_signature_n_keys(&sig) == n_keys);
+        CHECK(secp256k1_whitelist_signature_n_keys(&sig1) == n_keys);
+
+        /* Test bad number of keys in signature */
+        sig.n_keys = n_keys + 1;
+        CHECK(secp256k1_whitelist_verify(ctx, &sig, offline_pubkeys, online_pubkeys, n_keys, sub_pubkey) != 1);
+        sig.n_keys = n_keys;
+}
+
+void test_whitelist_end_to_end(const size_t n_keys, int test_all_keys) {
     unsigned char **online_seckey = (unsigned char **) malloc(n_keys * sizeof(*online_seckey));
     unsigned char **summed_seckey = (unsigned char **) malloc(n_keys * sizeof(*summed_seckey));
     secp256k1_pubkey *online_pubkeys = (secp256k1_pubkey *) malloc(n_keys * sizeof(*online_pubkeys));
@@ -51,36 +83,15 @@ void test_whitelist_end_to_end(const size_t n_keys) {
     }
 
     /* Sign/verify with each one */
-    for (i = 0; i < n_keys; i++) {
-        unsigned char serialized[32 + 4 + 32 * SECP256K1_WHITELIST_MAX_N_KEYS] = {0};
-        size_t slen = sizeof(serialized);
-        secp256k1_whitelist_signature sig;
-        secp256k1_whitelist_signature sig1;
-
-        CHECK(secp256k1_whitelist_sign(ctx, &sig, online_pubkeys, offline_pubkeys, n_keys, &sub_pubkey, online_seckey[i], summed_seckey[i], i, NULL, NULL));
-        CHECK(secp256k1_whitelist_verify(ctx, &sig, online_pubkeys, offline_pubkeys, n_keys, &sub_pubkey) == 1);
-        /* Check that exchanging keys causes a failure */
-        CHECK(secp256k1_whitelist_verify(ctx, &sig, offline_pubkeys, online_pubkeys, n_keys, &sub_pubkey) != 1);
-        /* Serialization round trip */
-        CHECK(secp256k1_whitelist_signature_serialize(ctx, serialized, &slen, &sig) == 1);
-        CHECK(slen == 33 + 32 * n_keys);
-        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen) == 1);
-        /* (Check various bad-length conditions) */
-        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen + 32) == 0);
-        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen + 1) == 0);
-        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, slen - 1) == 0);
-        CHECK(secp256k1_whitelist_signature_parse(ctx, &sig1, serialized, 0) == 0);
-        CHECK(secp256k1_whitelist_verify(ctx, &sig1, online_pubkeys, offline_pubkeys, n_keys, &sub_pubkey) == 1);
-        CHECK(secp256k1_whitelist_verify(ctx, &sig1, offline_pubkeys, online_pubkeys, n_keys, &sub_pubkey) != 1);
-
-        /* Test n_keys */
-        CHECK(secp256k1_whitelist_signature_n_keys(&sig) == n_keys);
-        CHECK(secp256k1_whitelist_signature_n_keys(&sig1) == n_keys);
-
-        /* Test bad number of keys in signature */
-        sig.n_keys = n_keys + 1;
-        CHECK(secp256k1_whitelist_verify(ctx, &sig, offline_pubkeys, online_pubkeys, n_keys, &sub_pubkey) != 1);
-        sig.n_keys = n_keys;
+    if (test_all_keys) {
+        for (i = 0; i < n_keys; i++) {
+            test_whitelist_end_to_end_internal(summed_seckey[i], online_seckey[i], online_pubkeys, offline_pubkeys, &sub_pubkey, i, n_keys);
+        }
+    } else {
+        uint32_t rand_idx = secp256k1_testrand_int(n_keys-1);
+        test_whitelist_end_to_end_internal(summed_seckey[0], online_seckey[0], online_pubkeys, offline_pubkeys, &sub_pubkey, 0, n_keys);
+        test_whitelist_end_to_end_internal(summed_seckey[rand_idx], online_seckey[rand_idx], online_pubkeys, offline_pubkeys, &sub_pubkey, rand_idx, n_keys);
+        test_whitelist_end_to_end_internal(summed_seckey[n_keys-1], online_seckey[n_keys-1], online_pubkeys, offline_pubkeys, &sub_pubkey, n_keys-1, n_keys);
     }
 
     for (i = 0; i < n_keys; i++) {
@@ -142,9 +153,10 @@ void run_whitelist_tests(void) {
     test_whitelist_bad_parse();
     test_whitelist_bad_serialize();
     for (i = 0; i < count; i++) {
-        test_whitelist_end_to_end(1);
-        test_whitelist_end_to_end(10);
-        test_whitelist_end_to_end(50);
+        test_whitelist_end_to_end(1, 1);
+        test_whitelist_end_to_end(10, 1);
+        test_whitelist_end_to_end(50, 1);
+        test_whitelist_end_to_end(SECP256K1_WHITELIST_MAX_N_KEYS, 0);
     }
 }