From b979b6be407a083080e790bdc0a68ebecaa607ea Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 16 Jun 2022 16:31:10 +1000 Subject: [PATCH 1/9] Add a mechanism to record namespaces for synth-from-dnssec When namespace is grafted on, the DNSSEC proofs for non existance need to come from that namespace and not a higher namespace. We add 3 function dns_view_sfd_add, dns_view_sfd_del and dns_view_sfd_find to add, remove and find the namespace that should be used when checking NSEC records. dns_view_sfd_add adds a name to a tree, creating the tree if needed. If the name already existed in the tree the reference count is increased otherwise it is initalised to 1. dns_view_sfd_del removes a reference to a name in the tree, if the count goes to 0 the node is removed. dns_view_sfd_find returns the namespace to be used to entered name. If there isn't an enclosing name in the tree, or the tree does not yet exist, the root name is returned. Access to the tree is controlled by a read/write lock. (cherry picked from commit 3619cad1413d32715cf4fe67f955b10330b770d4) --- lib/dns/include/dns/view.h | 38 ++++++++++++++++++ lib/dns/view.c | 82 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 27d78820b8..26aee1b4e9 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -151,6 +151,8 @@ struct dns_view { dns_rbt_t *denyanswernames; dns_rbt_t *answernames_exclude; dns_rrl_t *rrl; + dns_rbt_t *sfd; + isc_rwlock_t sfd_lock; bool provideixfr; bool requestnsid; bool sendcookie; @@ -1362,4 +1364,40 @@ dns_view_staleanswerenabled(dns_view_t *view); *\li 'view' to be valid. */ +void +dns_view_sfd_add(dns_view_t *view, const dns_name_t *name); +/*%< + * Add 'name' to the synth-from-dnssec namespace tree for the + * view. If the tree does not already exist create it. + * + * Requires: + *\li 'view' to be valid. + *\li 'name' to be valid. + */ + +void +dns_view_sfd_del(dns_view_t *view, const dns_name_t *name); +/*%< + * Delete 'name' to the synth-from-dnssec namespace tree for + * the view when the count of previous adds and deletes becomes + * zero. + * + * Requires: + *\li 'view' to be valid. + *\li 'name' to be valid. + */ + +void +dns_view_sfd_find(dns_view_t *view, const dns_name_t *name, + dns_name_t *foundname); +/*%< + * Find the enclosing name to the synth-from-dnssec namespace tree for 'name' + * in the specified view. + * + * Requires: + *\li 'view' to be valid. + *\li 'name' to be valid. + *\li 'foundname' to be valid with a buffer sufficient to hold the name. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/view.c b/lib/dns/view.c index d6d29a0a36..f413cf2527 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -118,6 +118,8 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, isc_mutex_init(&view->lock); + isc_rwlock_init(&view->sfd_lock, 0, 0); + view->zonetable = NULL; result = dns_zt_create(mctx, rdclass, &view->zonetable); if (result != ISC_R_SUCCESS) { @@ -210,6 +212,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, view->denyanswernames = NULL; view->answernames_exclude = NULL; view->rrl = NULL; + view->sfd = NULL; view->provideixfr = true; view->maxcachettl = 7 * 24 * 3600; view->maxncachettl = 3 * 3600; @@ -336,6 +339,7 @@ cleanup_zt: } cleanup_mutex: + isc_rwlock_destroy(&view->sfd_lock); isc_mutex_destroy(&view->lock); if (view->nta_file != NULL) { @@ -506,6 +510,9 @@ destroy(dns_view_t *view) { if (view->answernames_exclude != NULL) { dns_rbt_destroy(&view->answernames_exclude); } + if (view->sfd != NULL) { + dns_rbt_destroy(&view->sfd); + } if (view->delonly != NULL) { dns_name_t *name; int i; @@ -598,6 +605,7 @@ destroy(dns_view_t *view) { dns_badcache_destroy(&view->failcache); } isc_mutex_destroy(&view->new_zone_lock); + isc_rwlock_destroy(&view->sfd_lock); isc_mutex_destroy(&view->lock); isc_refcount_destroy(&view->references); isc_refcount_destroy(&view->weakrefs); @@ -2581,3 +2589,77 @@ dns_view_staleanswerenabled(dns_view_t *view) { return (result); } + +static void +free_sfd(void *data, void *arg) { + isc_mem_put(arg, data, sizeof(unsigned int)); +} + +void +dns_view_sfd_add(dns_view_t *view, const dns_name_t *name) { + isc_result_t result; + dns_rbtnode_t *node = NULL; + + REQUIRE(DNS_VIEW_VALID(view)); + + RWLOCK(&view->sfd_lock, isc_rwlocktype_write); + if (view->sfd == NULL) { + result = dns_rbt_create(view->mctx, free_sfd, view->mctx, + &view->sfd); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + } + + result = dns_rbt_addnode(view->sfd, name, &node); + RUNTIME_CHECK(result == ISC_R_SUCCESS || result == ISC_R_EXISTS); + if (node->data != NULL) { + unsigned int *count = node->data; + (*count)++; + } else { + unsigned int *count = isc_mem_get(view->mctx, + sizeof(unsigned int)); + *count = 1; + node->data = count; + } + RWUNLOCK(&view->sfd_lock, isc_rwlocktype_write); +} + +void +dns_view_sfd_del(dns_view_t *view, const dns_name_t *name) { + isc_result_t result; + void *data = NULL; + + REQUIRE(DNS_VIEW_VALID(view)); + + RWLOCK(&view->sfd_lock, isc_rwlocktype_write); + INSIST(view->sfd != NULL); + result = dns_rbt_findname(view->sfd, name, 0, NULL, &data); + if (result == ISC_R_SUCCESS) { + unsigned int *count = data; + INSIST(count != NULL); + if (--(*count) == 0U) { + result = dns_rbt_deletename(view->sfd, name, false); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + } + } + RWUNLOCK(&view->sfd_lock, isc_rwlocktype_write); +} + +void +dns_view_sfd_find(dns_view_t *view, const dns_name_t *name, + dns_name_t *foundname) { + REQUIRE(DNS_VIEW_VALID(view)); + + if (view->sfd != NULL) { + isc_result_t result; + void *data = NULL; + + RWLOCK(&view->sfd_lock, isc_rwlocktype_read); + result = dns_rbt_findname(view->sfd, name, 0, foundname, &data); + RWUNLOCK(&view->sfd_lock, isc_rwlocktype_read); + if (result != ISC_R_SUCCESS && result != DNS_R_PARTIALMATCH) { + dns_name_copy(dns_rootname, foundname); + } + } else { + dns_name_copy(dns_rootname, foundname); + } +} From 4d9287dca544986ec2df8daeed0cf7e671893e2f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 16 Jun 2022 16:48:42 +1000 Subject: [PATCH 2/9] Check the synth-form-dnssec namespace when synthesising responses Call dns_view_sfd_find to find the namespace to be used to verify the covering NSEC records returned for the given QNAME. Check that the NSEC owner names are within that namespace. (cherry picked from commit 228dadb0268e5c6982975214c41594df05db2157) --- lib/ns/query.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/ns/query.c b/lib/ns/query.c index eb2b44b70c..18ab138b1c 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -10082,13 +10083,16 @@ query_coveringnsec(query_ctx_t *qctx) { dns_clientinfomethods_t cm; dns_dbnode_t *node = NULL; dns_fixedname_t fixed; + dns_fixedname_t fnamespace; dns_fixedname_t fnowild; dns_fixedname_t fsigner; dns_fixedname_t fwild; dns_name_t *fname = NULL; + dns_name_t *namespace = NULL; dns_name_t *nowild = NULL; dns_name_t *signer = NULL; dns_name_t *wild = NULL; + dns_name_t qname; dns_rdataset_t *soardataset = NULL, *sigsoardataset = NULL; dns_rdataset_t rdataset, sigrdataset; bool done = false; @@ -10096,11 +10100,29 @@ query_coveringnsec(query_ctx_t *qctx) { bool redirected = false; isc_result_t result = ISC_R_SUCCESS; unsigned int dboptions = qctx->client->query.dboptions; + unsigned int labels; CCTRACE(ISC_LOG_DEBUG(3), "query_coveringnsec"); + dns_name_init(&qname, NULL); dns_rdataset_init(&rdataset); dns_rdataset_init(&sigrdataset); + namespace = dns_fixedname_initname(&fnamespace); + + /* + * Check that the NSEC record is from the correct namespace. + * For records that belong to the parent zone (i.e. DS), + * remove a label to find the correct namespace. + */ + dns_name_clone(qctx->client->query.qname, &qname); + labels = dns_name_countlabels(&qname); + if (dns_rdatatype_atparent(qctx->qtype) && labels > 1) { + dns_name_getlabelsequence(&qname, 1, labels - 1, &qname); + } + dns_view_sfd_find(qctx->view, &qname, namespace); + if (!dns_name_issubdomain(qctx->fname, namespace)) { + goto cleanup; + } /* * If we have no signer name, stop immediately. @@ -10230,6 +10252,13 @@ query_coveringnsec(query_ctx_t *qctx) { switch (result) { case DNS_R_COVERINGNSEC: + /* + * Check that the covering NSEC record is from the right + * namespace. + */ + if (!dns_name_issubdomain(nowild, namespace)) { + goto cleanup; + } result = dns_nsec_noexistnodata(qctx->qtype, wild, nowild, &rdataset, &exists, &data, NULL, log_noexistnodata, qctx); From 107c3a452af1df86267b6860fc321a58bfab7f5c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 16 Jun 2022 16:55:06 +1000 Subject: [PATCH 3/9] Add entries to the synth-from-dnssec namespace tree for zones When a zone is attached or detached from the view (zone->view is updated) update the synth-from-dnssec namespace tree. (cherry picked from commit f716bd68d4c17fc38766e5b4ba3cfbc5b837bfde) --- lib/dns/zone.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index b06d37b087..03b3e26d78 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1623,9 +1623,11 @@ dns_zone_setview_helper(dns_zone_t *zone, dns_view_t *view) { INSIST(zone != zone->raw); if (zone->view != NULL) { + dns_view_sfd_del(zone->view, &zone->origin); dns_view_weakdetach(&zone->view); } dns_view_weakattach(view, &zone->view); + dns_view_sfd_add(view, &zone->origin); if (zone->strviewname != NULL) { isc_mem_free(zone->mctx, zone->strviewname); From 90467f4127c54a0a4c2aebd194f74220c46e073b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 16 Jun 2022 17:12:20 +1000 Subject: [PATCH 4/9] Add synth-from-dnssec namespace entries for forward only namespaces Currently forward entries are only removed on view destruction so there is no matching dns_view_sfd_del call. (cherry picked from commit a559d6fdd13cc617ea2baae2df2e23eda3e06abb) --- bin/named/server.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/named/server.c b/bin/named/server.c index 4318c5ca99..73dcdacbb1 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6344,6 +6344,10 @@ configure_forward(const cfg_obj_t *config, dns_view_t *view, goto cleanup; } + if (fwdpolicy == dns_fwdpolicy_only) { + dns_view_sfd_add(view, origin); + } + result = ISC_R_SUCCESS; cleanup: From 30d4e3ee89fb662608fa2e9e69e6052a2b515fdd Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 17 Jun 2022 10:40:47 +1000 Subject: [PATCH 5/9] Add synth-from-dnssec namespaces for keytable entries We do this by adding callbacks for when a node is added or deleted from the keytable. dns_keytable_add and dns_keytable_delete where extended to take a callback. dns_keytable_deletekey does not remove the node so it was not extended. (cherry picked from commit a5b57ed2934cd467ba9d8472dc318624b3864bd0) --- bin/named/server.c | 18 ++++++--- lib/dns/client.c | 2 +- lib/dns/include/dns/keytable.h | 8 +++- lib/dns/keytable.c | 23 ++++++++--- lib/dns/zone.c | 22 +++++++++- tests/dns/keytable_test.c | 74 ++++++++++++++++++++-------------- 6 files changed, 101 insertions(+), 46 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 73dcdacbb1..552434379e 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -876,6 +876,13 @@ cleanup: return (result); } +static void +sfd_add(const dns_name_t *name, void *arg) { + if (arg != NULL) { + dns_view_sfd_add(arg, name); + } +} + /*% * Parse 'key' in the context of view configuration 'vconfig'. If successful, * add the key to 'secroots' if both of the following conditions are true: @@ -889,8 +896,7 @@ cleanup: */ static isc_result_t process_key(const cfg_obj_t *key, dns_keytable_t *secroots, - const dns_name_t *keyname_match, dns_resolver_t *resolver, - bool managed) { + const dns_name_t *keyname_match, dns_view_t *view, bool managed) { dns_fixedname_t fkeyname; dns_name_t *keyname = NULL; const char *namestr = NULL; @@ -963,8 +969,8 @@ process_key(const cfg_obj_t *key, dns_keytable_t *secroots, * its owner name. If it does not, do not load the key and log a * warning, but do not prevent further keys from being processed. */ - if (!dns_resolver_algorithm_supported(resolver, keyname, ds.algorithm)) - { + if (!dns_resolver_algorithm_supported(view->resolver, keyname, + ds.algorithm)) { cfg_obj_log(key, named_g_lctx, ISC_LOG_WARNING, "ignoring %s for '%s': algorithm is disabled", initializing ? "initial-key" : "static-key", @@ -980,7 +986,7 @@ process_key(const cfg_obj_t *key, dns_keytable_t *secroots, * 'managed' and 'initializing' arguments to dns_keytable_add(). */ result = dns_keytable_add(secroots, initializing, initializing, keyname, - &ds); + &ds, sfd_add, view); done: return (result); @@ -1008,7 +1014,7 @@ load_view_keys(const cfg_obj_t *keys, dns_view_t *view, bool managed, for (elt2 = cfg_list_first(keylist); elt2 != NULL; elt2 = cfg_list_next(elt2)) { CHECK(process_key(cfg_listelt_value(elt2), secroots, - keyname, view->resolver, managed)); + keyname, view, managed)); } } diff --git a/lib/dns/client.c b/lib/dns/client.c index 4385aa1db4..bf8d69c559 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -1311,7 +1311,7 @@ dns_client_addtrustedkey(dns_client_t *client, dns_rdataclass_t rdclass, digest, &ds)); } - CHECK(dns_keytable_add(secroots, false, false, name, &ds)); + CHECK(dns_keytable_add(secroots, false, false, name, &ds, NULL, NULL)); cleanup: if (view != NULL) { diff --git a/lib/dns/include/dns/keytable.h b/lib/dns/include/dns/keytable.h index 94db94658c..6095a85e3f 100644 --- a/lib/dns/include/dns/keytable.h +++ b/lib/dns/include/dns/keytable.h @@ -49,6 +49,8 @@ ISC_LANG_BEGINDECLS +typedef void (*dns_keytable_callback_t)(const dns_name_t *name, void *fn_arg); + isc_result_t dns_keytable_create(isc_mem_t *mctx, dns_keytable_t **keytablep); /*%< @@ -106,7 +108,8 @@ dns_keytable_detach(dns_keytable_t **keytablep); isc_result_t dns_keytable_add(dns_keytable_t *keytable, bool managed, bool initial, - dns_name_t *name, dns_rdata_ds_t *ds); + dns_name_t *name, dns_rdata_ds_t *ds, + dns_keytable_callback_t callback, void *callback_arg); /*%< * Add a key to 'keytable'. The keynode associated with 'name' * is updated with the DS specified in 'ds'. @@ -167,7 +170,8 @@ dns_keytable_marksecure(dns_keytable_t *keytable, const dns_name_t *name); */ isc_result_t -dns_keytable_delete(dns_keytable_t *keytable, const dns_name_t *keyname); +dns_keytable_delete(dns_keytable_t *keytable, const dns_name_t *keyname, + dns_keytable_callback_t callback, void *callback_arg); /*%< * Delete all trust anchors from 'keytable' matching name 'keyname' * diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 324b4efc53..9172d51dd0 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -367,7 +367,8 @@ new_keynode(dns_rdata_ds_t *ds, dns_keytable_t *keytable, bool managed, */ static isc_result_t insert(dns_keytable_t *keytable, bool managed, bool initial, - const dns_name_t *keyname, dns_rdata_ds_t *ds) { + const dns_name_t *keyname, dns_rdata_ds_t *ds, + dns_keytable_callback_t callback, void *callback_arg) { dns_rbtnode_t *node = NULL; isc_result_t result; @@ -384,6 +385,9 @@ insert(dns_keytable_t *keytable, bool managed, bool initial, * and attach it to the created node. */ node->data = new_keynode(ds, keytable, managed, initial); + if (callback != NULL) { + (*callback)(keyname, callback_arg); + } } else if (result == ISC_R_EXISTS) { /* * A node already exists for "keyname" in "keytable". @@ -393,6 +397,9 @@ insert(dns_keytable_t *keytable, bool managed, bool initial, if (knode == NULL) { node->data = new_keynode(ds, keytable, managed, initial); + if (callback != NULL) { + (*callback)(keyname, callback_arg); + } } else { add_ds(knode, ds, keytable->mctx); } @@ -407,20 +414,23 @@ insert(dns_keytable_t *keytable, bool managed, bool initial, isc_result_t dns_keytable_add(dns_keytable_t *keytable, bool managed, bool initial, - dns_name_t *name, dns_rdata_ds_t *ds) { + dns_name_t *name, dns_rdata_ds_t *ds, + dns_keytable_callback_t callback, void *callback_arg) { REQUIRE(ds != NULL); REQUIRE(!initial || managed); - return (insert(keytable, managed, initial, name, ds)); + return (insert(keytable, managed, initial, name, ds, callback, + callback_arg)); } isc_result_t dns_keytable_marksecure(dns_keytable_t *keytable, const dns_name_t *name) { - return (insert(keytable, true, false, name, NULL)); + return (insert(keytable, true, false, name, NULL, NULL, NULL)); } isc_result_t -dns_keytable_delete(dns_keytable_t *keytable, const dns_name_t *keyname) { +dns_keytable_delete(dns_keytable_t *keytable, const dns_name_t *keyname, + dns_keytable_callback_t callback, void *callback_arg) { isc_result_t result; dns_rbtnode_t *node = NULL; @@ -434,6 +444,9 @@ dns_keytable_delete(dns_keytable_t *keytable, const dns_name_t *keyname) { if (node->data != NULL) { result = dns_rbt_deletenode(keytable->table, node, false); + if (callback != NULL) { + (*callback)(keyname, callback_arg); + } } else { result = ISC_R_NOTFOUND; } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 03b3e26d78..6796f8b499 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -4282,6 +4282,23 @@ compute_tag(dns_name_t *name, dns_rdata_dnskey_t *dnskey, isc_mem_t *mctx, return (result); } +/* + * Synth-from-dnssec callbacks to add/delete names from namespace tree. + */ +static void +sfd_add(const dns_name_t *name, void *arg) { + if (arg != NULL) { + dns_view_sfd_add(arg, name); + } +} + +static void +sfd_del(const dns_name_t *name, void *arg) { + if (arg != NULL) { + dns_view_sfd_del(arg, name); + } +} + /* * Add key to the security roots. */ @@ -4306,7 +4323,8 @@ trust_key(dns_zone_t *zone, dns_name_t *keyname, dns_rdata_dnskey_t *dnskey, dns_rdatatype_dnskey, dnskey, &buffer); CHECK(dns_ds_fromkeyrdata(keyname, &rdata, DNS_DSDIGEST_SHA256, digest, &ds)); - CHECK(dns_keytable_add(sr, true, initial, keyname, &ds)); + CHECK(dns_keytable_add(sr, true, initial, keyname, &ds, sfd_add, + zone->view)); dns_keytable_detach(&sr); @@ -4351,7 +4369,7 @@ load_secroots(dns_zone_t *zone, dns_name_t *name, dns_rdataset_t *rdataset) { result = dns_view_getsecroots(zone->view, &sr); if (result == ISC_R_SUCCESS) { - dns_keytable_delete(sr, name); + dns_keytable_delete(sr, name, sfd_del, zone->view); dns_keytable_detach(&sr); } diff --git a/tests/dns/keytable_test.c b/tests/dns/keytable_test.c index 0afb6bf68a..615f90eaab 100644 --- a/tests/dns/keytable_test.c +++ b/tests/dns/keytable_test.c @@ -152,15 +152,14 @@ create_dsstruct(dns_name_t *name, uint16_t flags, uint8_t proto, uint8_t alg, /* Common setup: create a keytable and ntatable to test with a few keys */ static void create_tables(void) { - isc_result_t result; unsigned char digest[ISC_MAX_MD_SIZE]; dns_rdata_ds_t ds; dns_fixedname_t fn; dns_name_t *keyname = dns_fixedname_name(&fn); isc_stdtime_t now; - result = dns_test_makeview("view", false, &view); - assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(dns_test_makeview("view", false, &view), + ISC_R_SUCCESS); assert_int_equal(dns_keytable_create(mctx, &keytable), ISC_R_SUCCESS); assert_int_equal( @@ -170,19 +169,21 @@ create_tables(void) { /* Add a normal key */ dns_test_namefromstring("example.com", &fn); create_dsstruct(keyname, 257, 3, 5, keystr1, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); /* Add an initializing managed key */ dns_test_namefromstring("managed.com", &fn); create_dsstruct(keyname, 257, 3, 5, keystr1, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, true, true, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, true, true, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); /* Add a null key */ - assert_int_equal(dns_keytable_marksecure(keytable, str2name("null." - "example")), - ISC_R_SUCCESS); + assert_int_equal( + dns_keytable_marksecure(keytable, str2name("null.example")), + ISC_R_SUCCESS); /* Add a negative trust anchor, duration 1 hour */ isc_stdtime_get(&now); @@ -230,7 +231,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { */ dns_test_namefromstring("example.com", &fn); create_dsstruct(keyname, 257, 3, 5, keystr1, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); dns_keytable_detachkeynode(keytable, &keynode); assert_int_equal( @@ -240,7 +242,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { /* Add another key (different keydata) */ dns_keytable_detachkeynode(keytable, &keynode); create_dsstruct(keyname, 257, 3, 5, keystr2, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); assert_int_equal( dns_keytable_find(keytable, str2name("example.com"), &keynode), @@ -267,7 +270,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { */ dns_test_namefromstring("managed.com", &fn); create_dsstruct(keyname, 257, 3, 5, keystr2, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, true, true, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, true, true, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); assert_int_equal( dns_keytable_find(keytable, str2name("managed.com"), &keynode), @@ -281,7 +285,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { * to a non-initializing key and make sure there are still two key * nodes for managed.com, both containing non-initializing keys. */ - assert_int_equal(dns_keytable_add(keytable, true, false, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, true, false, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); assert_int_equal( dns_keytable_find(keytable, str2name("managed.com"), &keynode), @@ -295,7 +300,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { */ dns_test_namefromstring("two.com", &fn); create_dsstruct(keyname, 257, 3, 5, keystr1, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, true, true, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, true, true, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); assert_int_equal( dns_keytable_find(keytable, str2name("two.com"), &keynode), @@ -310,7 +316,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { * the initialization status should not change. */ create_dsstruct(keyname, 257, 3, 5, keystr2, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, true, false, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, true, false, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); assert_int_equal( dns_keytable_find(keytable, str2name("two.com"), &keynode), @@ -327,7 +334,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { ISC_R_SUCCESS); dns_test_namefromstring("null.example", &fn); create_dsstruct(keyname, 257, 3, 5, keystr2, digest, &ds); - assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds), + assert_int_equal(dns_keytable_add(keytable, false, false, keyname, &ds, + NULL, NULL), ISC_R_SUCCESS); assert_int_equal( dns_keytable_find(keytable, str2name("null.example"), &keynode), @@ -341,9 +349,9 @@ ISC_RUN_TEST_IMPL(dns_keytable_add) { * (Note: this and above checks confirm that if a name has a null key * that's the only key for the name). */ - assert_int_equal(dns_keytable_marksecure(keytable, str2name("null." - "example")), - ISC_R_SUCCESS); + assert_int_equal( + dns_keytable_marksecure(keytable, str2name("null.example")), + ISC_R_SUCCESS); assert_int_equal(dns_keytable_find(keytable, str2name("null.example"), &null_keynode), ISC_R_SUCCESS); @@ -359,23 +367,26 @@ ISC_RUN_TEST_IMPL(dns_keytable_delete) { create_tables(); /* dns_keytable_delete requires exact match */ - assert_int_equal(dns_keytable_delete(keytable, str2name("example.org")), + assert_int_equal(dns_keytable_delete(keytable, str2name("example.org"), + NULL, NULL), ISC_R_NOTFOUND); - assert_int_equal(dns_keytable_delete(keytable, str2name("s.example." - "com")), + assert_int_equal(dns_keytable_delete(keytable, + str2name("s.example.com"), NULL, + NULL), ISC_R_NOTFOUND); - assert_int_equal(dns_keytable_delete(keytable, str2name("example.com")), + assert_int_equal(dns_keytable_delete(keytable, str2name("example.com"), + NULL, NULL), ISC_R_SUCCESS); /* works also for nodes with a null key */ - assert_int_equal(dns_keytable_delete(keytable, str2name("null." - "example")), + assert_int_equal(dns_keytable_delete(keytable, str2name("null.example"), + NULL, NULL), ISC_R_SUCCESS); /* or a negative trust anchor */ - assert_int_equal(dns_ntatable_delete(ntatable, str2name("insecure." - "example")), - ISC_R_SUCCESS); + assert_int_equal( + dns_ntatable_delete(ntatable, str2name("insecure.example")), + ISC_R_SUCCESS); destroy_tables(); } @@ -426,7 +437,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_deletekey) { * after deleting the node, any deletekey or delete attempt should * result in NOTFOUND. */ - assert_int_equal(dns_keytable_delete(keytable, keyname), ISC_R_SUCCESS); + assert_int_equal(dns_keytable_delete(keytable, keyname, NULL, NULL), + ISC_R_SUCCESS); assert_int_equal(dns_keytable_deletekey(keytable, keyname, &dnskey), ISC_R_NOTFOUND); dns_rdata_freestruct(&dnskey); @@ -439,7 +451,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_deletekey) { create_keystruct(257, 3, 5, keystr1, &dnskey); assert_int_equal(dns_keytable_deletekey(keytable, keyname, &dnskey), DNS_R_PARTIALMATCH); - assert_int_equal(dns_keytable_delete(keytable, keyname), ISC_R_SUCCESS); + assert_int_equal(dns_keytable_delete(keytable, keyname, NULL, NULL), + ISC_R_SUCCESS); dns_rdata_freestruct(&dnskey); destroy_tables(); @@ -585,7 +598,8 @@ ISC_RUN_TEST_IMPL(dns_keytable_nta) { dns_test_namefromstring("example", &fn); create_dsstruct(keyname, 257, 3, 5, keystr1, digest, &ds); - result = dns_keytable_add(keytable, false, false, keyname, &ds), + result = dns_keytable_add(keytable, false, false, keyname, &ds, NULL, + NULL), assert_int_equal(result, ISC_R_SUCCESS); isc_stdtime_get(&now); From 00db079f797ec319e55f5f59b6e4d8c2e55f5781 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 17 Jun 2022 14:40:39 +1000 Subject: [PATCH 6/9] Add system test for forward only grafted zone with synth-from-dnssec We are grafting on an unsigned zone "example.internal" where the higher zone (".") is signed and would otherwise cause named to synthesise a NXDOMAIN for example.internal. We prime the cache by performing a lookup for "internal" and then lookup "example.internal". (cherry picked from commit 8af5d0ad68ca939d98dd57706b534c58741e94a8) --- bin/tests/system/synthfromdnssec/ns1/root.db.in | 1 + .../synthfromdnssec/ns2/example.internal.db | 16 ++++++++++++++++ .../system/synthfromdnssec/ns2/named.conf.in | 5 +++++ .../system/synthfromdnssec/ns5/named.conf.in | 7 +++++++ bin/tests/system/synthfromdnssec/tests.sh | 13 +++++++++++++ 5 files changed, 42 insertions(+) create mode 100644 bin/tests/system/synthfromdnssec/ns2/example.internal.db diff --git a/bin/tests/system/synthfromdnssec/ns1/root.db.in b/bin/tests/system/synthfromdnssec/ns1/root.db.in index fa9a21b6c2..bade656f67 100644 --- a/bin/tests/system/synthfromdnssec/ns1/root.db.in +++ b/bin/tests/system/synthfromdnssec/ns1/root.db.in @@ -14,6 +14,7 @@ $TTL 3600 @ NS ns1 ns1 A 10.53.0.1 example NS ns1.example +fun NS ns1.example ns1.example A 10.53.0.1 dnamed NS ns1.dnamed ns1.dnamed A 10.53.0.1 diff --git a/bin/tests/system/synthfromdnssec/ns2/example.internal.db b/bin/tests/system/synthfromdnssec/ns2/example.internal.db new file mode 100644 index 0000000000..938159b899 --- /dev/null +++ b/bin/tests/system/synthfromdnssec/ns2/example.internal.db @@ -0,0 +1,16 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; This Source Code Form is subject to the terms of the Mozilla Public +; License, v. 2.0. If a copy of the MPL was not distributed with this +; file, you can obtain one at https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +$TTL 3600 +@ SOA ns2 hostmaster 1 3600 1200 604800 3600 +@ NS ns2 +@ A 1.2.3.4 +ns2 A 10.53.0.2 diff --git a/bin/tests/system/synthfromdnssec/ns2/named.conf.in b/bin/tests/system/synthfromdnssec/ns2/named.conf.in index b226420e6f..736d428172 100644 --- a/bin/tests/system/synthfromdnssec/ns2/named.conf.in +++ b/bin/tests/system/synthfromdnssec/ns2/named.conf.in @@ -44,4 +44,9 @@ zone "." { file "root.hints"; }; +zone "example.internal" { + type primary; + file "example.internal.db"; +}; + include "../ns1/trusted.conf"; diff --git a/bin/tests/system/synthfromdnssec/ns5/named.conf.in b/bin/tests/system/synthfromdnssec/ns5/named.conf.in index 2f936f4fa7..a98ef39ddf 100644 --- a/bin/tests/system/synthfromdnssec/ns5/named.conf.in +++ b/bin/tests/system/synthfromdnssec/ns5/named.conf.in @@ -25,6 +25,7 @@ options { notify no; dnssec-validation yes; synth-from-dnssec yes; + validate-except { example.internal; }; }; key rndc_key { @@ -45,4 +46,10 @@ zone "." { file "root.hints"; }; +zone "example.internal" { + type forward; + forward only; + forwarders { 10.53.0.2; }; +}; + include "../ns1/trusted.conf"; diff --git a/bin/tests/system/synthfromdnssec/tests.sh b/bin/tests/system/synthfromdnssec/tests.sh index d9cf0927ec..f5383c97e2 100644 --- a/bin/tests/system/synthfromdnssec/tests.sh +++ b/bin/tests/system/synthfromdnssec/tests.sh @@ -870,6 +870,19 @@ n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) +echo_i "check synth-from-dnssec with grafted zone (forward only) ($n)" +ret=0 +#prime cache with NXDOMAIN NSEC covering 'fun' to 'minimal' +dig_with_opts internal @10.53.0.5 > dig.out.ns5-1.test$n || ret=1 +grep "status: NXDOMAIN" dig.out.ns5-1.test$n >/dev/null || ret=1 +grep '^fun\..*NSEC.minimal\. ' dig.out.ns5-1.test$n >/dev/null || ret=1 +#perform lookup in grafted zone +dig_with_opts example.internal @10.53.0.5 > dig.out.ns5-2.test$n || ret=1 +grep "status: NOERROR" dig.out.ns5-2.test$n >/dev/null || ret=1 +grep '^example\.internal\..*A.1.2.3.4$' dig.out.ns5-2.test$n >/dev/null || ret=1 +n=$((n+1)) +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status+ret)) echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From 443fb79a2cf86e1cefb2f79e7f2cb4534a282ee9 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 20 Jun 2022 10:49:24 +1000 Subject: [PATCH 7/9] Test grafting and synth-from-dnssec using primary zone (cherry picked from commit 33454fb0e9726cb95b7254fbce2275af11b97445) --- .../system/synthfromdnssec/ns2/named.conf.in | 5 +++++ .../system/synthfromdnssec/ns5/internal2.db | 17 +++++++++++++++++ .../system/synthfromdnssec/ns5/named.conf.in | 5 +++++ bin/tests/system/synthfromdnssec/tests.sh | 14 ++++++++++++++ 4 files changed, 41 insertions(+) create mode 100644 bin/tests/system/synthfromdnssec/ns5/internal2.db diff --git a/bin/tests/system/synthfromdnssec/ns2/named.conf.in b/bin/tests/system/synthfromdnssec/ns2/named.conf.in index 736d428172..cc303072a2 100644 --- a/bin/tests/system/synthfromdnssec/ns2/named.conf.in +++ b/bin/tests/system/synthfromdnssec/ns2/named.conf.in @@ -49,4 +49,9 @@ zone "example.internal" { file "example.internal.db"; }; +zone "example.internal2" { + type primary; + file "example.internal.db"; +}; + include "../ns1/trusted.conf"; diff --git a/bin/tests/system/synthfromdnssec/ns5/internal2.db b/bin/tests/system/synthfromdnssec/ns5/internal2.db new file mode 100644 index 0000000000..4f07f700d6 --- /dev/null +++ b/bin/tests/system/synthfromdnssec/ns5/internal2.db @@ -0,0 +1,17 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; This Source Code Form is subject to the terms of the Mozilla Public +; License, v. 2.0. If a copy of the MPL was not distributed with this +; file, you can obtain one at https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +$TTL 3600 +@ SOA ns5 hostmaster 1 3600 1200 604800 3600 +@ NS ns5 +ns5 A 10.53.0.5 +example NS ns2.example +ns2.example A 10.53.0.2 diff --git a/bin/tests/system/synthfromdnssec/ns5/named.conf.in b/bin/tests/system/synthfromdnssec/ns5/named.conf.in index a98ef39ddf..885708c767 100644 --- a/bin/tests/system/synthfromdnssec/ns5/named.conf.in +++ b/bin/tests/system/synthfromdnssec/ns5/named.conf.in @@ -52,4 +52,9 @@ zone "example.internal" { forwarders { 10.53.0.2; }; }; +zone "internal2" { + type primary; + file "internal2.db"; +}; + include "../ns1/trusted.conf"; diff --git a/bin/tests/system/synthfromdnssec/tests.sh b/bin/tests/system/synthfromdnssec/tests.sh index f5383c97e2..aee907e2e0 100644 --- a/bin/tests/system/synthfromdnssec/tests.sh +++ b/bin/tests/system/synthfromdnssec/tests.sh @@ -884,5 +884,19 @@ n=$((n+1)) if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) +echo_i "check synth-from-dnssec with grafted zone (primary zone) ($n)" +ret=0 +#prime cache with NXDOMAIN NSEC covering 'fun' to 'minimal' +dig_with_opts internal @10.53.0.5 > dig.out.ns5-1.test$n || ret=1 +grep "status: NXDOMAIN" dig.out.ns5-1.test$n >/dev/null || ret=1 +grep '^fun\..*NSEC.minimal\. ' dig.out.ns5-1.test$n >/dev/null || ret=1 +#perform lookup in grafted zone +dig_with_opts example.internal2 @10.53.0.5 > dig.out.ns5-2.test$n || ret=1 +grep "status: NOERROR" dig.out.ns5-2.test$n >/dev/null || ret=1 +grep '^example\.internal2\..*A.1.2.3.4$' dig.out.ns5-2.test$n >/dev/null || ret=1 +n=$((n+1)) +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From fa4eb975b8d16e7f14a604ea31a22c59894970b9 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 20 Jun 2022 10:56:25 +1000 Subject: [PATCH 8/9] Add CHANGES note for [GL #3402] (cherry picked from commit 682c6eb5332fd2174df4875574c0ad483fe32379) --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index ea395d9559..38fe9f3ab5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,11 @@ 5915. [bug] Detect missing closing brace (}) and computational overflows in $GENERATE directives. [GL #3429] +5914. [bug] When synth-from-dnssec generated a response using + records from a higher zone, it could unexpectedly prove + non-existance of records in a subordinate grafted-on + namespace. [GL #3402] + 5911. [bug] Update HTTP listener settings on reconfiguration. [GL #3415] From 1872105f097879165bd42284c91aceac83037b10 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 20 Jun 2022 11:04:18 +1000 Subject: [PATCH 9/9] Add release note for [GL #3402] (cherry picked from commit 07d5c23cac7eec679c13e7937b826669ad8d178d) --- doc/notes/notes-current.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 4ec50492bc..8634ed47a3 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -52,3 +52,7 @@ Bug Fixes - Fix the assertion failure caused by TCP connection closing between the connect (or accept) and the read from the socket. :gl:`#3400` + +- When grafting on non-delegated namespace, synth-from-dnssec could incorrectly + synthesise non-existance of records within the grafted in namespace using + NSEC records from higher zones. :gl:`#3402`