sync_secure_db failed to handle some TTL changes

If the DNSKEY, CDNSKEY or CDS RRset had different TTLs then the
filtering of these RRset resulted in dns_diff_apply failing with
"not exact". Identify tuple pairs that are just TTL changes and
allow them through the filter.

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 6f3ea2bc88fdb1c789e20bdaef8ac837bf64ec73..42ca43f2d16f8fe0c39653c9f5f87ba81ae3d150 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -16061,6 +16061,77 @@ failure:
        return (result);
 }
 
+/*
+ * Filter the key material preserving TTL changes.  If kasp in effect honour the
+ * existing ttl.  The lists returned by sync_secure_db/dns_db_diffx should be
+ * DNSSEC RRset order so we can process 'del' and 'add' in parallel rather than
+ * searching for TTL only changes first and processing them, then checking the
+ * 'in use' status on a subsequent pass.
+ */
+
+static void
+filter_keymaterial(dns_zone_t *zone, dns_difftuplelist_t *del,
+                  dns_difftuplelist_t *add, bool kasp, dns_ttl_t ttl) {
+       dns_difftuple_t *deltuple = ISC_LIST_HEAD(*del);
+       dns_difftuple_t *addtuple = ISC_LIST_HEAD(*add);
+       isc_result_t result;
+
+       while (deltuple != NULL || addtuple != NULL) {
+               dns_difftuple_t *delnext = NULL, *addnext = NULL;
+               bool inuse = false;
+               if (deltuple != NULL) {
+                       delnext = ISC_LIST_NEXT(deltuple, link);
+               }
+               if (addtuple != NULL) {
+                       addnext = ISC_LIST_NEXT(addtuple, link);
+               }
+               if (deltuple != NULL && addtuple != NULL) {
+                       int n = dns_rdata_compare(&deltuple->rdata,
+                                                 &addtuple->rdata);
+                       if (n == 0) {
+                               /*
+                                * If the rdata is equal then the only
+                                * difference will be a TTL change.
+                                */
+                               if (kasp) {
+                                       /* TTL is managed by dnssec-policy */
+                                       ISC_LIST_UNLINK(*del, deltuple, link);
+                                       dns_difftuple_free(&deltuple);
+                                       ISC_LIST_UNLINK(*add, addtuple, link);
+                                       dns_difftuple_free(&addtuple);
+                               }
+                               deltuple = delnext;
+                               addtuple = addnext;
+                               continue;
+                       }
+                       if (n < 0) {
+                               goto checkdel;
+                       }
+                       goto checkadd;
+               } else if (deltuple != NULL) {
+               checkdel:
+                       result = dns_zone_dnskey_inuse(zone, &deltuple->rdata,
+                                                      &inuse);
+                       if (result == ISC_R_SUCCESS && inuse) {
+                               ISC_LIST_UNLINK(*del, deltuple, link);
+                               dns_difftuple_free(&deltuple);
+                       }
+                       deltuple = delnext;
+               } else {
+               checkadd:
+                       result = dns_zone_dnskey_inuse(zone, &addtuple->rdata,
+                                                      &inuse);
+                       if (result == ISC_R_SUCCESS && inuse) {
+                               ISC_LIST_UNLINK(*add, addtuple, link);
+                               dns_difftuple_free(&addtuple);
+                       } else if (kasp) {
+                               addtuple->ttl = ttl;
+                       }
+                       addtuple = addnext;
+               }
+       }
+}
+
 static isc_result_t
 sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb,
               dns_dbversion_t *secver, dns_difftuple_t **soatuple,
@@ -16071,6 +16142,16 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb,
        dns_difftuple_t *tuple = NULL, *next;
        dns_difftuple_t *oldtuple = NULL, *newtuple = NULL;
        dns_rdata_soa_t oldsoa, newsoa;
+       dns_difftuplelist_t add = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t del = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t keyadd = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t keydel = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t ckeyadd = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t ckeydel = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t cdsadd = ISC_LIST_INITIALIZER;
+       dns_difftuplelist_t cdsdel = ISC_LIST_INITIALIZER;
+       dns_kasp_t *kasp = NULL;
+       dns_ttl_t keyttl = 0, ckeyttl = 0, cdsttl = 0;
 
        REQUIRE(DNS_ZONE_VALID(seczone));
        REQUIRE(soatuple != NULL && *soatuple == NULL);
@@ -16089,8 +16170,52 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb,
                return (result);
        }
 
+       /*
+        * If kasp is in effect honour the existing DNSKEY, CDNSKEY and CDS
+        * TTLs.
+        */
+       kasp = seczone->kasp;
+       if (kasp != NULL) {
+               dns_rdataset_t rdataset;
+               dns_dbnode_t *node = NULL;
+               dns_ttl_t ttl = dns_kasp_dnskeyttl(kasp);
+
+               dns_rdataset_init(&rdataset);
+
+               result = dns_db_getoriginnode(secdb, &node);
+               RUNTIME_CHECK(result == ISC_R_SUCCESS);
+
+               result = dns_db_findrdataset(
+                       secdb, node, secver, dns_rdatatype_dnskey,
+                       dns_rdatatype_none, 0, &rdataset, NULL);
+               keyttl = (result == ISC_R_SUCCESS) ? rdataset.ttl : ttl;
+               if (dns_rdataset_isassociated(&rdataset)) {
+                       dns_rdataset_disassociate(&rdataset);
+               }
+
+               result = dns_db_findrdataset(
+                       secdb, node, secver, dns_rdatatype_cdnskey,
+                       dns_rdatatype_none, 0, &rdataset, NULL);
+               ckeyttl = (result == ISC_R_SUCCESS) ? rdataset.ttl : ttl;
+               if (dns_rdataset_isassociated(&rdataset)) {
+                       dns_rdataset_disassociate(&rdataset);
+               }
+
+               result = dns_db_findrdataset(
+                       secdb, node, secver, dns_rdatatype_cds,
+                       dns_rdatatype_none, 0, &rdataset, NULL);
+               cdsttl = (result == ISC_R_SUCCESS) ? rdataset.ttl : ttl;
+               if (dns_rdataset_isassociated(&rdataset)) {
+                       dns_rdataset_disassociate(&rdataset);
+               }
+               dns_db_detachnode(secdb, &node);
+       }
+
        for (tuple = ISC_LIST_HEAD(diff->tuples); tuple != NULL; tuple = next) {
+               dns_difftuplelist_t *al = &add, *dl = &del;
+
                next = ISC_LIST_NEXT(tuple, link);
+
                /*
                 * Skip DNSSEC records that BIND maintains with inline-signing.
                 */
@@ -16105,18 +16230,27 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb,
                }
 
                /*
-                * Allow DNSKEY, CDNSKEY, CDS because users should be able to
-                * update the zone with these records from a different provider,
-                * but skip records that are under our control.
+                * Apex DNSKEY, CDNSKEY and CDS need special processing so
+                * split them out.
                 */
-               if (dns_rdatatype_iskeymaterial(tuple->rdata.type)) {
-                       bool inuse = false;
-                       isc_result_t r = dns_zone_dnskey_inuse(
-                               seczone, &tuple->rdata, &inuse);
-                       if (r == ISC_R_SUCCESS && inuse) {
-                               ISC_LIST_UNLINK(diff->tuples, tuple, link);
-                               dns_difftuple_free(&tuple);
-                               continue;
+               if (dns_rdatatype_iskeymaterial(tuple->rdata.type) &&
+                   dns_name_equal(&tuple->name, &seczone->origin))
+               {
+                       switch (tuple->rdata.type) {
+                       case dns_rdatatype_dnskey:
+                               al = &keyadd;
+                               dl = &keydel;
+                               break;
+                       case dns_rdatatype_cdnskey:
+                               al = &ckeyadd;
+                               dl = &ckeydel;
+                               break;
+                       case dns_rdatatype_cds:
+                               al = &cdsadd;
+                               dl = &cdsdel;
+                               break;
+                       default:
+                               UNREACHABLE();
                        }
                }
 
@@ -16130,6 +16264,23 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb,
                                newtuple = tuple;
                        }
                }
+
+               /*
+                * Split into deletions and additions.
+                */
+               ISC_LIST_UNLINK(diff->tuples, tuple, link);
+               switch (tuple->op) {
+               case DNS_DIFFOP_DEL:
+               case DNS_DIFFOP_DELRESIGN:
+                       ISC_LIST_APPEND(*dl, tuple, link);
+                       break;
+               case DNS_DIFFOP_ADD:
+               case DNS_DIFFOP_ADDRESIGN:
+                       ISC_LIST_APPEND(*al, tuple, link);
+                       break;
+               default:
+                       UNREACHABLE();
+               }
        }
 
        if (oldtuple != NULL && newtuple != NULL) {
@@ -16151,13 +16302,32 @@ sync_secure_db(dns_zone_t *seczone, dns_zone_t *raw, dns_db_t *secdb,
                    dns_name_equal(&oldsoa.origin, &newsoa.origin) &&
                    dns_name_equal(&oldsoa.contact, &newsoa.contact))
                {
-                       ISC_LIST_UNLINK(diff->tuples, oldtuple, link);
+                       ISC_LIST_UNLINK(del, oldtuple, link);
                        dns_difftuple_free(&oldtuple);
-                       ISC_LIST_UNLINK(diff->tuples, newtuple, link);
+                       ISC_LIST_UNLINK(add, newtuple, link);
                        dns_difftuple_free(&newtuple);
                }
        }
 
+       /*
+        * Filter out keys we manage but still allow TTL changes.
+        */
+       filter_keymaterial(seczone, &keydel, &keyadd, kasp != NULL, keyttl);
+       filter_keymaterial(seczone, &ckeydel, &ckeyadd, kasp != NULL, ckeyttl);
+       filter_keymaterial(seczone, &cdsdel, &cdsadd, kasp != NULL, cdsttl);
+
+       /*
+        * Rebuild the diff now that we have filtered it
+        */
+       ISC_LIST_APPENDLIST(diff->tuples, del, link);
+       ISC_LIST_APPENDLIST(diff->tuples, keydel, link);
+       ISC_LIST_APPENDLIST(diff->tuples, ckeydel, link);
+       ISC_LIST_APPENDLIST(diff->tuples, cdsdel, link);
+       ISC_LIST_APPENDLIST(diff->tuples, add, link);
+       ISC_LIST_APPENDLIST(diff->tuples, keyadd, link);
+       ISC_LIST_APPENDLIST(diff->tuples, ckeyadd, link);
+       ISC_LIST_APPENDLIST(diff->tuples, cdsadd, link);
+
        if (ISC_LIST_EMPTY(diff->tuples)) {
                return (DNS_R_UNCHANGED);
        }