From 19f803384082b9e953dab1928e5463cb40f57880 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 29 Nov 2022 16:17:13 +1100 Subject: [PATCH 1/7] Add new view method dns_view_istrusted dns_view_istrusted determines if the given key is treated as being trusted by the view. (cherry picked from commit 7278fff579333d8b334805771ce786a39d573d8c) --- lib/dns/include/dns/view.h | 13 ++++++ lib/dns/view.c | 82 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index a06110b700..c844e1c695 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -1214,6 +1214,19 @@ dns_view_untrust(dns_view_t *view, const dns_name_t *keyname, * \li 'dnskey' is valid. */ +bool +dns_view_istrusted(dns_view_t *view, const dns_name_t *keyname, + const dns_rdata_dnskey_t *dnskey); +/*%< + * Determine if the key defined by 'keyname' and 'dnskey' is + * trusted by 'view'. + * + * Requires: + * \li 'view' is valid. + * \li 'keyname' is valid. + * \li 'dnskey' is valid. + */ + isc_result_t dns_view_setnewzones(dns_view_t *view, bool allow, void *cfgctx, void (*cfg_destroy)(void **), uint64_t mapsize); diff --git a/lib/dns/view.c b/lib/dns/view.c index 5bc787db52..47822a4383 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -2026,6 +2026,88 @@ dns_view_untrust(dns_view_t *view, const dns_name_t *keyname, dns_keytable_detach(&sr); } +bool +dns_view_istrusted(dns_view_t *view, const dns_name_t *keyname, + const dns_rdata_dnskey_t *dnskey) { + isc_result_t result; + dns_keytable_t *sr = NULL; + dns_keynode_t *knode = NULL; + bool answer = false; + dns_rdataset_t dsset; + + REQUIRE(DNS_VIEW_VALID(view)); + REQUIRE(keyname != NULL); + REQUIRE(dnskey != NULL); + + result = dns_view_getsecroots(view, &sr); + if (result != ISC_R_SUCCESS) { + return (false); + } + + dns_rdataset_init(&dsset); + result = dns_keytable_find(sr, keyname, &knode); + if (result == ISC_R_SUCCESS) { + if (dns_keynode_dsset(knode, &dsset)) { + dns_rdata_t rdata = DNS_RDATA_INIT; + unsigned char data[4096], digest[DNS_DS_BUFFERSIZE]; + dns_rdata_dnskey_t tmpkey = *dnskey; + dns_rdata_ds_t ds; + isc_buffer_t b; + dns_rdataclass_t rdclass = tmpkey.common.rdclass; + + /* + * Clear the revoke bit, if set, so that the key + * will match what's in secroots now. + */ + tmpkey.flags &= ~DNS_KEYFLAG_REVOKE; + + isc_buffer_init(&b, data, sizeof(data)); + result = dns_rdata_fromstruct(&rdata, rdclass, + dns_rdatatype_dnskey, + &tmpkey, &b); + if (result != ISC_R_SUCCESS) { + goto finish; + } + + result = dns_ds_fromkeyrdata(keyname, &rdata, + DNS_DSDIGEST_SHA256, + digest, &ds); + if (result != ISC_R_SUCCESS) { + goto finish; + } + + dns_rdata_reset(&rdata); + isc_buffer_init(&b, data, sizeof(data)); + result = dns_rdata_fromstruct( + &rdata, rdclass, dns_rdatatype_ds, &ds, &b); + if (result != ISC_R_SUCCESS) { + goto finish; + } + + result = dns_rdataset_first(&dsset); + while (result == ISC_R_SUCCESS) { + dns_rdata_t this = DNS_RDATA_INIT; + dns_rdataset_current(&dsset, &this); + if (dns_rdata_compare(&rdata, &this) == 0) { + answer = true; + break; + } + result = dns_rdataset_next(&dsset); + } + } + } + +finish: + if (dns_rdataset_isassociated(&dsset)) { + dns_rdataset_disassociate(&dsset); + } + if (knode != NULL) { + dns_keytable_detachkeynode(sr, &knode); + } + dns_keytable_detach(&sr); + return (answer); +} + /* * Create path to a directory and a filename constructed from viewname. * This is a front-end to isc_file_sanitize(), allowing backward From 3cb366b1e0003bbdf1988ebbd50c4c485d4bc776 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 24 Nov 2022 14:18:20 +1100 Subject: [PATCH 2/7] Reduce the number of verifiations required In selfsigned_dnskey only call dns_dnssec_verify if the signature's key id matches a revoked key, the trust is pending and the key matches a trust anchor. Previously named was calling dns_dnssec_verify unconditionally resulted in busy work. (cherry picked from commit e68fecbdaa0e7ad86322bfa5e977eb1944ba821e) --- lib/dns/validator.c | 50 +++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 99c9fd0a26..6cf717f870 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1397,26 +1397,50 @@ selfsigned_dnskey(dns_validator_t *val) { continue; } + /* + * If the REVOKE bit is not set we have a + * theoretically self signed DNSKEY RRset. + * This will be verified later. + */ + if ((key.flags & DNS_KEYFLAG_REVOKE) == 0) { + answer = true; + continue; + } + result = dns_dnssec_keyfromrdata(name, &keyrdata, mctx, &dstkey); if (result != ISC_R_SUCCESS) { continue; } - result = dns_dnssec_verify(name, rdataset, dstkey, true, - val->view->maxbits, mctx, - &sigrdata, NULL); + /* + * If this RRset is pending and it is trusted, + * see if it was self signed by this DNSKEY. + */ + if (DNS_TRUST_PENDING(rdataset->trust) && + dns_view_istrusted(val->view, name, &key)) + { + result = dns_dnssec_verify( + name, rdataset, dstkey, true, + val->view->maxbits, mctx, &sigrdata, + NULL); + if (result == ISC_R_SUCCESS) { + /* + * The key with the REVOKE flag has + * self signed the RRset so it is no + * good. + */ + dns_view_untrust(val->view, name, &key); + } + } else if (rdataset->trust >= dns_trust_secure) { + /* + * We trust this RRset so if the key is + * marked revoked remove it. + */ + dns_view_untrust(val->view, name, &key); + } + dst_key_free(&dstkey); - if (result != ISC_R_SUCCESS) { - continue; - } - - if ((key.flags & DNS_KEYFLAG_REVOKE) == 0) { - answer = true; - continue; - } - - dns_view_untrust(val->view, name, &key); } } From f708172d87a91a71cba8b89f0eb72408a1d6dd82 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 29 Nov 2022 16:07:39 +1100 Subject: [PATCH 3/7] Handle dns_rdata_fromstruct failure dns_keytable_deletekey dns_rdata_fromstruct in dns_keytable_deletekey can potentially fail with ISC_R_NOSPACE. Handle the error condition. (cherry picked from commit b5df9b8591875c00f280d7687ff11430133a36ba) --- lib/dns/keytable.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 7899124c90..e5786f1388 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -459,10 +459,6 @@ dns_keytable_deletekey(dns_keytable_t *keytable, const dns_name_t *keyname, REQUIRE(VALID_KEYTABLE(keytable)); REQUIRE(dnskey != NULL); - isc_buffer_init(&b, data, sizeof(data)); - dns_rdata_fromstruct(&rdata, dnskey->common.rdclass, - dns_rdatatype_dnskey, dnskey, &b); - RWLOCK(&keytable->rwlock, isc_rwlocktype_write); result = dns_rbt_findnode(keytable->table, keyname, NULL, &node, NULL, DNS_RBTFIND_NOOPTIONS, NULL, NULL); @@ -489,6 +485,13 @@ dns_keytable_deletekey(dns_keytable_t *keytable, const dns_name_t *keyname, } RWUNLOCK(&knode->rwlock, isc_rwlocktype_read); + isc_buffer_init(&b, data, sizeof(data)); + result = dns_rdata_fromstruct(&rdata, dnskey->common.rdclass, + dns_rdatatype_dnskey, dnskey, &b); + if (result != ISC_R_SUCCESS) { + goto finish; + } + result = dns_ds_fromkeyrdata(keyname, &rdata, DNS_DSDIGEST_SHA256, digest, &ds); if (result != ISC_R_SUCCESS) { From 489cba33bb5d052eed8dd51035f9ffd7f21fcd23 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 29 Nov 2022 16:11:51 +1100 Subject: [PATCH 4/7] dns_view_untrust modifies dnskey->flags when it shouldn't Copy the structure and declare dnskey as const. (cherry picked from commit 21d828241b47c1fbdc6083c20b21dd95f48acc87) --- lib/dns/include/dns/view.h | 2 +- lib/dns/view.c | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index c844e1c695..d6e7f10e18 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -1197,7 +1197,7 @@ dns_view_ntacovers(dns_view_t *view, isc_stdtime_t now, const dns_name_t *name, void dns_view_untrust(dns_view_t *view, const dns_name_t *keyname, - dns_rdata_dnskey_t *dnskey); + const dns_rdata_dnskey_t *dnskey); /*%< * Remove keys that match 'keyname' and 'dnskey' from the views trust * anchors. diff --git a/lib/dns/view.c b/lib/dns/view.c index 47822a4383..a67dd73a3c 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -1992,26 +1992,28 @@ dns_view_issecuredomain(dns_view_t *view, const dns_name_t *name, void dns_view_untrust(dns_view_t *view, const dns_name_t *keyname, - dns_rdata_dnskey_t *dnskey) { + const dns_rdata_dnskey_t *dnskey) { isc_result_t result; dns_keytable_t *sr = NULL; + dns_rdata_dnskey_t tmpkey; REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(keyname != NULL); REQUIRE(dnskey != NULL); - /* - * Clear the revoke bit, if set, so that the key will match what's - * in secroots now. - */ - dnskey->flags &= ~DNS_KEYFLAG_REVOKE; - result = dns_view_getsecroots(view, &sr); if (result != ISC_R_SUCCESS) { return; } - result = dns_keytable_deletekey(sr, keyname, dnskey); + /* + * Clear the revoke bit, if set, so that the key will match what's + * in secroots now. + */ + tmpkey = *dnskey; + tmpkey.flags &= ~DNS_KEYFLAG_REVOKE; + + result = dns_keytable_deletekey(sr, keyname, &tmpkey); if (result == ISC_R_SUCCESS) { /* * If key was found in secroots, then it was a From 2e3588e550e4b8cb4cf5ce50c37fab110bebe6de Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 31 Mar 2023 18:19:30 +1100 Subject: [PATCH 5/7] Add CHANGES for [GL #3981] (cherry picked from commit 5de552d5458920ed8d8bb7bd6a4211e0479e906a) --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index b910d12c0d..eeb382eeaf 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6142. [bug] Reduce the number of dns_dnssec_verify calls made + determining if revoked keys needs to be removed from + the trust anchors. [GL #3981] + 6138. [doc] Fix the DF-flag documentation on the outgoing UDP packets. [GL #3710] From 1fd45928a32cb5f9e0c2934869c533490efdfd62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pa=C4=8Dek?= Date: Mon, 3 Apr 2023 16:57:32 +0200 Subject: [PATCH 6/7] Add release note for [GL #3981] (cherry picked from commit 705a9ced010e5b1324a9b2591021f9f41e5a83db) --- doc/notes/notes-current.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c8366f1f10..fd8b4597fa 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -39,6 +39,9 @@ Bug Fixes removed when the number of files exceeded the limit set by ``versions``. :gl:`#828` :gl:`#3959` +- Performance of DNSSEC validation in zones with many DNSKEY records + has been improved. :gl:`#3981` + Known Issues ~~~~~~~~~~~~ From dc6a888ea3f82fe2b39e4d8f34b08418071e5f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pa=C4=8Dek?= Date: Mon, 3 Apr 2023 18:18:43 +0200 Subject: [PATCH 7/7] Export dns_view_istrusted() on Windows --- lib/dns/win32/libdns.def.in | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 6db74498e3..5c0ba7c451 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1139,6 +1139,7 @@ dns_view_initsecroots dns_view_iscacheshared dns_view_isdelegationonly dns_view_issecuredomain +dns_view_istrusted dns_view_load dns_view_loadnta dns_view_ntacovers