refactor create_keydata

use empty placeholder KEYDATA records for all trust anchors, not just
DS-style trust anchors.

this revealed a pre-existing bug: keyfetch_done() skips keys without
the SEP bit when populating the managed-keys zone. consequently, if a
zone only has a single ZSK which is configured as trust anchor and no
KSKs, then no KEYDATA record is ever written to the managed-keys zone
when keys are refreshed.

that was how the root server in the dnssec system test was configured.
however, previously, the KEYDATA was created when the key was
initialized; this prevented us from noticing the bug until now.

configuring a ZSK as an RFC 5011 trust anchor is not forbidden by the
spec, but it is highly unusual and not well defined.  so for the time
being, I have modified the system test to generate both a KSK and ZSK
for the root zone, enabling the test to pass.

we should consider adding code to detect this condition and allow keys
without the SEP bit to be used as trust anchors if no key with the SEP
bit is available, or at minimum, log a warning.

diff --git a/bin/tests/system/dnssec/ns1/sign.sh b/bin/tests/system/dnssec/ns1/sign.sh
index 385d3100a72b2d569a7b6b359bd4fc18dcb4edd6..fe8a432eebcf0b9dec63747e79fdff0097b6efd9 100644 (file)
--- a/bin/tests/system/dnssec/ns1/sign.sh
+++ b/bin/tests/system/dnssec/ns1/sign.sh
@@ -30,14 +30,15 @@ cp "../ns2/dsset-in-addr.arpa$TP" .
 grep "$DEFAULT_ALGORITHM_NUMBER [12] " "../ns2/dsset-algroll$TP" > "dsset-algroll$TP"
 cp "../ns6/dsset-optout-tld$TP" .
 
-keyname=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone")
+ksk=$("$KEYGEN" -q -fk -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone")
+zsk=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone")
 
-cat "$infile" "$keyname.key" > "$zonefile"
+cat "$infile" "$ksk.key" "$zsk.key" > "$zonefile"
 
 "$SIGNER" -P -g -o "$zone" "$zonefile" > /dev/null 2>&1
 
 # Configure the resolving server with a staitc key.
-keyfile_to_static_keys "$keyname" > trusted.conf
+keyfile_to_static_keys "$ksk" > trusted.conf
 cp trusted.conf ../ns2/trusted.conf
 cp trusted.conf ../ns3/trusted.conf
 cp trusted.conf ../ns4/trusted.conf
@@ -46,11 +47,11 @@ cp trusted.conf ../ns7/trusted.conf
 cp trusted.conf ../ns9/trusted.conf
 
 # ...or with an initializing key.
-keyfile_to_initial_keys "$keyname" > managed.conf
+keyfile_to_initial_keys "$ksk" > managed.conf
 cp managed.conf ../ns4/managed.conf
 
 #
 #  Save keyid for managed key id test.
 #
 
-keyfile_to_key_id "$keyname" > managed.key.id
+keyfile_to_key_id "$ksk" > managed.key.id

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index cad1b8282b46c5d1847df8beedd40cd027c0b01c..f70b460177d15e8839737392499e96baf016eea8 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -3828,121 +3828,59 @@ set_refreshkeytimer(dns_zone_t *zone, dns_rdata_keydata_t *key,
 }
 
 /*
- * Convert key(s) linked from 'keynode' to KEYDATA and add it to the
- * key zone.  If `keynode` doesn't point to a key, then this is a
- * DS-style trust anchor, so create an empty KEYDATA instead; it
- * will be replaced with the correct key on refresh.
+ * If keynode references a key or a DS rdataset, and if the key
+ * zone does not contain a KEYDATA record for the corresponding name,
+ * then create an empty KEYDATA and push it into the zone as a placeholder,
+ * then schedule a key refresh immediately. This new KEYDATA record will be
+ * updated during the refresh.
  *
  * If the key zone is changed, set '*changed' to true.
  */
 static isc_result_t
 create_keydata(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
-              dns_diff_t *diff, dns_keytable_t *keytable,
-              dns_keynode_t **keynodep, dns_name_t *keyname,
-              bool *changed)
+              dns_diff_t *diff, dns_keynode_t *keynode,
+              dns_name_t *keyname, bool *changed)
 {
        const char me[] = "create_keydata";
        isc_result_t result = ISC_R_SUCCESS;
-       isc_buffer_t keyb, dstb;
-       unsigned char key_buf[4096], dst_buf[DST_KEY_MAXSIZE];
-       dns_rdata_keydata_t keydata;
-       dns_rdata_dnskey_t dnskey;
        dns_rdata_t rdata = DNS_RDATA_INIT;
-       dns_keynode_t *keynode = NULL;
+       dns_rdata_keydata_t kd;
+       unsigned char rrdata[4096];
+       isc_buffer_t rrdatabuf;
        isc_stdtime_t now;
-       isc_region_t r;
-       dst_key_t *key = NULL;
 
-       REQUIRE(keynodep != NULL);
-       keynode = *keynodep;
+       REQUIRE(keynode != NULL);
 
        ENTER;
        isc_stdtime_get(&now);
 
-       if (dns_keynode_dsset(keynode) != NULL) {
-               dns_rdata_keydata_t kd = { 0 };
-               unsigned char rrdata[4096];
-               isc_buffer_t rrdatabuf;
-
-               /*
-                * We're creating a placeholder KEYDATA record
-                * to be updated when the key zone is refreshed
-                * later.
-                */
-               kd.common.rdclass = zone->rdclass;
-               kd.common.rdtype = dns_rdatatype_keydata;
-               ISC_LINK_INIT(&kd.common, link);
-
-               isc_buffer_init(&rrdatabuf, rrdata, sizeof(rrdata));
-
-               CHECK(dns_rdata_fromstruct(&rdata, zone->rdclass,
-                                          dns_rdatatype_keydata,
-                                          &kd, &rrdatabuf));
-               /* Add rdata to zone. */
-               CHECK(update_one_rr(db, ver, diff, DNS_DIFFOP_ADD,
-                                   keyname, 0, &rdata));
-               *changed = true;
-
-               /* Refresh new keys from the zone apex as soon as possible. */
-               set_refreshkeytimer(zone, &keydata, now, true);
-
-               result = ISC_R_NOMORE;
-       } else {
-               /* Proceed to the while loop below */
-               result = ISC_R_SUCCESS;
+       /*
+        * If the keynode has neither a key nor a DS RRset,
+        * we shouldn't be here.
+        */
+       if (dns_keynode_key(keynode) == NULL &&
+           dns_keynode_dsset(keynode) == NULL)
+       {
+               return (ISC_R_FAILURE);
        }
 
-       /* Loop in case there's more than one key. */
-       while (result == ISC_R_SUCCESS) {
-               dns_keynode_t *nextnode = NULL;
-
-               key = dns_keynode_key(keynode);
-               if (key == NULL) {
-                       goto skip;
-               }
-
-               isc_buffer_init(&dstb, dst_buf, sizeof(dst_buf));
-               CHECK(dst_key_todns(key, &dstb));
-
-               /* Convert DST key to DNSKEY. */
-               dns_rdata_reset(&rdata);
-               isc_buffer_usedregion(&dstb, &r);
-               dns_rdata_fromregion(&rdata, dst_key_class(key),
-                                    dns_rdatatype_dnskey, &r);
-
-               /* DSTKEY to KEYDATA. */
-               CHECK(dns_rdata_tostruct(&rdata, &dnskey, NULL));
-               CHECK(dns_keydata_fromdnskey(&keydata, &dnskey, now, 0, 0,
-                                            NULL));
-
-               /* KEYDATA to rdata. */
-               dns_rdata_reset(&rdata);
-               isc_buffer_init(&keyb, key_buf, sizeof(key_buf));
-               CHECK(dns_rdata_fromstruct(&rdata,
-                                          zone->rdclass, dns_rdatatype_keydata,
-                                          &keydata, &keyb));
-
-               /* Add rdata to zone. */
-               CHECK(update_one_rr(db, ver, diff, DNS_DIFFOP_ADD,
-                                   keyname, 0, &rdata));
-               *changed = true;
-
-               /* Refresh new keys from the zone apex as soon as possible. */
-               set_refreshkeytimer(zone, &keydata, now, true);
+       memset(&kd, 0, sizeof(kd));
+       kd.common.rdclass = zone->rdclass;
+       kd.common.rdtype = dns_rdatatype_keydata;
+       ISC_LINK_INIT(&kd.common, link);
 
- skip:
-               result = dns_keytable_nextkeynode(keytable, keynode, &nextnode);
-               if (result != ISC_R_NOTFOUND) {
-                       dns_keytable_detachkeynode(keytable, &keynode);
-                       keynode = nextnode;
-               }
-       }
+       isc_buffer_init(&rrdatabuf, rrdata, sizeof(rrdata));
 
-       if (keynode != NULL) {
-               dns_keytable_detachkeynode(keytable, &keynode);
-       }
-       *keynodep = NULL;
+       CHECK(dns_rdata_fromstruct(&rdata, zone->rdclass,
+                                  dns_rdatatype_keydata,
+                                  &kd, &rrdatabuf));
+       /* Add rdata to zone. */
+       CHECK(update_one_rr(db, ver, diff, DNS_DIFFOP_ADD,
+                           keyname, 0, &rdata));
+       *changed = true;
 
+       /* Refresh new keys from the zone apex as soon as possible. */
+       set_refreshkeytimer(zone, &kd, now, true);
        return (ISC_R_SUCCESS);
 
   failure:
@@ -4312,9 +4250,9 @@ addifmissing(dns_keytable_t *keytable, dns_keynode_t *keynode,
        dns_zone_t *zone = ((struct addifmissing_arg *)arg)->zone;
        bool *changed = ((struct addifmissing_arg *)arg)->changed;
        isc_result_t result;
-       dns_keynode_t *dummy = NULL;
        dns_fixedname_t fname;
 
+       UNUSED(keytable);
 
        if (((struct addifmissing_arg *)arg)->result != ISC_R_SUCCESS) {
                return;
@@ -4350,9 +4288,7 @@ addifmissing(dns_keytable_t *keytable, dns_keynode_t *keynode,
        /*
         * Create the keydata.
         */
-       dns_keytable_attachkeynode(keytable, keynode, &dummy);
-       result = create_keydata(zone, db, ver, diff, keytable, &dummy,
-                               keyname, changed);
+       result = create_keydata(zone, db, ver, diff, keynode, keyname, changed);
        if (result != ISC_R_SUCCESS && result != ISC_R_NOMORE) {
                ((struct addifmissing_arg *)arg)->result = result;
        }