Grow arrays in signset if key->index is too big

The arrays in signset are sized to allow for one additional
key per RRSIG however the list of known keys is updated in
parallel which means that the index of the returned key can
be too big for the arrays.  Grow the arrays if this happens.

Additionally ensure that the keylist and keycount are locked
when they are being read.

diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
index 5197feda6396b268c3267d22518f0103a24e180b..75f85f31caee77219d818e42d87c5d2aa29746fe 100644 (file)
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -465,6 +465,30 @@ setverifies(dns_name_t *name, dns_rdataset_t *set, dst_key_t *key,
        }
 }
 
+static void
+grow_arrays(unsigned int newarraysize, unsigned int *arraysize,
+           bool **wassignedby, bool **nowsignedby) {
+       bool *nwsb = isc_mem_cget(isc_g_mctx, newarraysize, sizeof(bool));
+       bool *nnsb = isc_mem_cget(isc_g_mctx, newarraysize, sizeof(bool));
+       unsigned int i;
+
+       INSIST(newarraysize > *arraysize);
+
+       for (i = 0; i < *arraysize; i++) {
+               nwsb[i] = (*wassignedby)[i];
+               nnsb[i] = (*nowsignedby)[i];
+       }
+       for (; i < newarraysize; i++) {
+               nwsb[i] = nnsb[i] = false;
+       }
+
+       isc_mem_cput(isc_g_mctx, *wassignedby, *arraysize, sizeof(bool));
+       isc_mem_cput(isc_g_mctx, *nowsignedby, *arraysize, sizeof(bool));
+       *wassignedby = nwsb;
+       *nowsignedby = nnsb;
+       *arraysize = newarraysize;
+}
+
 /*%
  * Signs a set.  Goes through contortions to decide if each RRSIG should
  * be dropped or retained, and then determines if any new SIGs need to
@@ -478,10 +502,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
        isc_result_t result;
        bool nosigs = false;
        bool *wassignedby, *nowsignedby;
-       int arraysize;
+       unsigned int arraysize;
        dns_difftuple_t *tuple;
        dns_ttl_t ttl;
-       int i;
+       unsigned int i;
        char namestr[DNS_NAME_FORMATSIZE];
        char typestr[DNS_RDATATYPE_FORMATSIZE];
        char sigstr[SIG_FORMATSIZE];
@@ -507,7 +531,9 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 
        vbprintf(1, "%s/%s:\n", namestr, typestr);
 
+       RWLOCK(&keylist_lock, isc_rwlocktype_read);
        arraysize = keycount;
+       RWUNLOCK(&keylist_lock, isc_rwlocktype_read);
        if (!nosigs) {
                arraysize += dns_rdataset_count(&sigset);
        }
@@ -533,6 +559,13 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                        future = isc_serial_lt(now, rrsig.timesigned);
 
                        key = keythatsigned(&rrsig);
+                       /*
+                        * Grow arrays if needed.
+                        */
+                       if (key != NULL && key->index >= arraysize) {
+                               grow_arrays(key->index + 1, &arraysize,
+                                           &wassignedby, &nowsignedby);
+                       }
                        offline = (key != NULL) ? key->pubkey : false;
                        sig_format(&rrsig, sigstr, sizeof(sigstr));
                        expired = isc_serial_gt(now, rrsig.timeexpire);
@@ -679,16 +712,29 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
        check_result(result, "dns_rdataset_first/next");
        dns_rdataset_cleanup(&sigset);
 
+       RWLOCK(&keylist_lock, isc_rwlocktype_read);
        ISC_LIST_FOREACH(keylist, key, link) {
+               RWUNLOCK(&keylist_lock, isc_rwlocktype_read);
                if (REVOKE(key->key) && set->type != dns_rdatatype_dnskey) {
+                       RWLOCK(&keylist_lock, isc_rwlocktype_read);
                        continue;
                }
 
+               /*
+                * Grow arrays if needed.
+                */
+               if (key->index >= arraysize) {
+                       grow_arrays(key->index + 1, &arraysize, &wassignedby,
+                                   &nowsignedby);
+               }
+
                if (nowsignedby[key->index]) {
+                       RWLOCK(&keylist_lock, isc_rwlocktype_read);
                        continue;
                }
 
                if (!issigningkey(key)) {
+                       RWLOCK(&keylist_lock, isc_rwlocktype_read);
                        continue;
                }
 
@@ -699,19 +745,27 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                {
                        bool have_ksk = isksk(key);
 
+                       RWLOCK(&keylist_lock, isc_rwlocktype_read);
                        ISC_LIST_FOREACH(keylist, curr, link) {
+                               RWUNLOCK(&keylist_lock, isc_rwlocktype_read);
                                if (dst_key_alg(key->key) !=
                                    dst_key_alg(curr->key))
                                {
+                                       RWLOCK(&keylist_lock,
+                                              isc_rwlocktype_read);
                                        continue;
                                }
                                if (REVOKE(curr->key)) {
+                                       RWLOCK(&keylist_lock,
+                                              isc_rwlocktype_read);
                                        continue;
                                }
                                if (isksk(curr)) {
                                        have_ksk = true;
                                }
+                               RWLOCK(&keylist_lock, isc_rwlocktype_read);
                        }
+                       RWUNLOCK(&keylist_lock, isc_rwlocktype_read);
                        if (isksk(key) || !have_ksk ||
                            (iszsk(key) && !keyset_kskonly))
                        {
@@ -737,14 +791,19 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                                 * - Have key ID equal to the predecessor id.
                                 * - Have a successor that matches 'key' id.
                                 */
+                               RWLOCK(&keylist_lock, isc_rwlocktype_read);
                                ISC_LIST_FOREACH(keylist, curr, link) {
                                        uint32_t suc;
+                                       RWUNLOCK(&keylist_lock,
+                                                isc_rwlocktype_read);
 
                                        if (dst_key_alg(key->key) !=
                                                    dst_key_alg(curr->key) ||
                                            !iszsk(curr) ||
                                            dst_key_id(curr->key) != pre)
                                        {
+                                               RWLOCK(&keylist_lock,
+                                                      isc_rwlocktype_read);
                                                continue;
                                        }
                                        result = dst_key_getnum(
@@ -753,8 +812,16 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                                        if (result != ISC_R_SUCCESS ||
                                            dst_key_id(key->key) != suc)
                                        {
+                                               RWLOCK(&keylist_lock,
+                                                      isc_rwlocktype_read);
                                                continue;
                                        }
+                                       if (curr->index >= arraysize) {
+                                               grow_arrays(curr->index + 1,
+                                                           &arraysize,
+                                                           &wassignedby,
+                                                           &nowsignedby);
+                                       }
 
                                        /*
                                         * curr is the predecessor we were
@@ -764,7 +831,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                                        if (nowsignedby[curr->index]) {
                                                have_pre_sig = true;
                                        }
+                                       RWLOCK(&keylist_lock,
+                                              isc_rwlocktype_read);
                                }
+                               RWUNLOCK(&keylist_lock, isc_rwlocktype_read);
                        }
 
                        /*
@@ -776,7 +846,9 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
                                            "signing with dnskey");
                        }
                }
+               RWLOCK(&keylist_lock, isc_rwlocktype_read);
        }
+       RWUNLOCK(&keylist_lock, isc_rwlocktype_read);
 
        isc_mem_cput(isc_g_mctx, wassignedby, arraysize, sizeof(bool));
        isc_mem_cput(isc_g_mctx, nowsignedby, arraysize, sizeof(bool));