From 3619cad1413d32715cf4fe67f955b10330b770d4 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. --- lib/dns/include/dns/view.h | 39 ++++++++++++++++++ lib/dns/view.c | 81 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 721ac5e60a..5d78999941 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -144,6 +144,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; @@ -1282,4 +1284,41 @@ dns_view_flushonshutdown(dns_view_t *view, bool flush); * Requires: *\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 1669cd18a8..4200eeb8c5 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -127,6 +127,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) { @@ -215,6 +217,7 @@ cleanup_zt: } cleanup_mutex: + isc_rwlock_destroy(&view->sfd_lock); isc_mutex_destroy(&view->lock); if (view->nta_file != NULL) { @@ -381,6 +384,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; @@ -464,6 +470,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); @@ -2334,3 +2341,77 @@ dns_view_flushonshutdown(dns_view_t *view, bool flush) { view->flush = flush; } + +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 228dadb0268e5c6982975214c41594df05db2157 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. --- lib/ns/query.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/ns/query.c b/lib/ns/query.c index c7624290b0..879a9d68eb 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -9855,13 +9856,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; @@ -9869,11 +9873,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. @@ -10000,6 +10022,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 f716bd68d4c17fc38766e5b4ba3cfbc5b837bfde 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. --- lib/dns/zone.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 67e9e526b9..e9495436e6 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1635,9 +1635,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 a559d6fdd13cc617ea2baae2df2e23eda3e06abb 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. --- bin/named/server.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/named/server.c b/bin/named/server.c index 81f724abf4..0051ba5d30 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6321,6 +6321,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 a5b57ed2934cd467ba9d8472dc318624b3864bd0 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. --- 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 0051ba5d30..dcb7a708f4 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -862,6 +862,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: @@ -875,8 +882,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; @@ -949,8 +955,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", @@ -966,7 +972,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); @@ -994,7 +1000,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 a5bbaebe97..586be89103 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -1296,7 +1296,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 901e709fb2..56dceb2dc1 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 e9495436e6..9d8ad65009 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -4290,6 +4290,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. */ @@ -4314,7 +4331,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); @@ -4359,7 +4377,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 bc7b60d12e..f96f199390 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 8af5d0ad68ca939d98dd57706b534c58741e94a8 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". --- 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 33454fb0e9726cb95b7254fbce2275af11b97445 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 --- .../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 682c6eb5332fd2174df4875574c0ad483fe32379 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] --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 02341a06ff..8d8a465a01 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +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] + 5913. [placeholder] 5912. [cleanup] The "glue-cache" option has been removed. The glue cache From 07d5c23cac7eec679c13e7937b826669ad8d178d 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] --- 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 0a73b3da61..1a48dae0a5 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -53,3 +53,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`