optimize key ID check when searching for matching keys

when searching a DNSKEY or KEY rrset for the key that matches
a particular algorithm and ID, it's a waste of time to convert
every key into a dst_key object; it's faster to compute the key
ID by checksumming the region, and then only do the full key
conversion once we know we've found the correct key.

this optimization was already in use in the validator, but it's
been refactored for code clarity, and is now also used in query.c
and message.c.

diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c
index 3f844264c661ee2c5945b727e1766d6bfff1b0b9..ab2d432c18298797a46b4482c48bbc50a849fbd1 100644 (file)
--- a/lib/dns/dst_api.c
+++ b/lib/dns/dst_api.c
@@ -162,8 +162,7 @@ computeid(dst_key_t *key);
 static isc_result_t
 frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags,
           unsigned int protocol, dns_rdataclass_t rdclass,
-          isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata,
-          dst_key_t **keyp);
+          isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp);
 
 static isc_result_t
 algorithm_status(unsigned int alg);
@@ -721,13 +720,6 @@ dst_key_todns(const dst_key_t *key, isc_buffer_t *target) {
 isc_result_t
 dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass,
                isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp) {
-       return dst_key_fromdns_ex(name, rdclass, source, mctx, false, keyp);
-}
-
-isc_result_t
-dst_key_fromdns_ex(const dns_name_t *name, dns_rdataclass_t rdclass,
-                  isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata,
-                  dst_key_t **keyp) {
        uint8_t alg, proto;
        uint32_t flags, extflags;
        dst_key_t *key = NULL;
@@ -756,7 +748,7 @@ dst_key_fromdns_ex(const dns_name_t *name, dns_rdataclass_t rdclass,
        }
 
        result = frombuffer(name, alg, flags, proto, rdclass, source, mctx,
-                           no_rdata, &key);
+                           &key);
        if (result != ISC_R_SUCCESS) {
                return result;
        }
@@ -775,7 +767,7 @@ dst_key_frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags,
        isc_result_t result;
 
        result = frombuffer(name, alg, flags, protocol, rdclass, source, mctx,
-                           false, &key);
+                           &key);
        if (result != ISC_R_SUCCESS) {
                return result;
        }
@@ -2267,8 +2259,7 @@ computeid(dst_key_t *key) {
 static isc_result_t
 frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags,
           unsigned int protocol, dns_rdataclass_t rdclass,
-          isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata,
-          dst_key_t **keyp) {
+          isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp) {
        dst_key_t *key;
        isc_result_t ret;
 
@@ -2290,12 +2281,10 @@ frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags,
                        return DST_R_UNSUPPORTEDALG;
                }
 
-               if (!no_rdata) {
-                       ret = key->func->fromdns(key, source);
-                       if (ret != ISC_R_SUCCESS) {
-                               dst_key_free(&key);
-                               return ret;
-                       }
+               ret = key->func->fromdns(key, source);
+               if (ret != ISC_R_SUCCESS) {
+                       dst_key_free(&key);
+                       return ret;
                }
        }
 

diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h
index 4bcb026b1c187b6019942ef049d3979f48d61e35..f6e1e2e05bb455319129e11b3838e77ad4c8e8cb 100644 (file)
--- a/lib/dns/include/dst/dst.h
+++ b/lib/dns/include/dst/dst.h
@@ -460,10 +460,6 @@ dst_key_tofile(const dst_key_t *key, int type, const char *directory);
  */
 
 isc_result_t
-dst_key_fromdns_ex(const dns_name_t *name, dns_rdataclass_t rdclass,
-                  isc_buffer_t *source, isc_mem_t *mctx, bool no_rdata,
-                  dst_key_t **keyp);
-isc_result_t
 dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass,
                isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp);
 /*%<

diff --git a/lib/dns/message.c b/lib/dns/message.c
index e52dca453317d6ed154be80939a098ab2387fdb6..caeff02f4fcfb683a3a1369afe479b8ec154c6af 100644 (file)
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -3105,7 +3105,7 @@ dns_message_checksig_async(dns_message_t *msg, dns_view_t *view,
 
 isc_result_t
 dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
-       isc_buffer_t b, msgb;
+       isc_buffer_t msgb;
 
        REQUIRE(DNS_MESSAGE_VALID(msg));
 
@@ -3126,7 +3126,7 @@ dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
                        return dns_tsig_verify(&msgb, msg, NULL, NULL);
                }
        } else {
-               dns_rdata_t rdata = DNS_RDATA_INIT;
+               dns_rdata_t sigrdata = DNS_RDATA_INIT;
                dns_rdata_sig_t sig;
                dns_rdataset_t keyset;
                isc_result_t result;
@@ -3134,7 +3134,7 @@ dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
 
                result = dns_rdataset_first(msg->sig0);
                INSIST(result == ISC_R_SUCCESS);
-               dns_rdataset_current(msg->sig0, &rdata);
+               dns_rdataset_current(msg->sig0, &sigrdata);
 
                /*
                 * This can occur when the message is a dynamic update, since
@@ -3143,11 +3143,11 @@ dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
                 * looked for in the additional section, and the dynamic update
                 * meta-records are in the prerequisite and update sections.
                 */
-               if (rdata.length == 0) {
+               if (sigrdata.length == 0) {
                        return ISC_R_UNEXPECTEDEND;
                }
 
-               result = dns_rdata_tostruct(&rdata, &sig, NULL);
+               result = dns_rdata_tostruct(&sigrdata, &sig, NULL);
                if (result != ISC_R_SUCCESS) {
                        return result;
                }
@@ -3191,26 +3191,32 @@ dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
                     message_checks < max_message_checks;
                     key_checks++, result = dns_rdataset_next(&keyset))
                {
+                       dns_rdata_t keyrdata = DNS_RDATA_INIT;
+                       dns_rdata_key_t ks;
                        dst_key_t *key = NULL;
+                       isc_region_t r;
 
-                       dns_rdata_reset(&rdata);
-                       dns_rdataset_current(&keyset, &rdata);
-                       isc_buffer_init(&b, rdata.data, rdata.length);
-                       isc_buffer_add(&b, rdata.length);
+                       dns_rdataset_current(&keyset, &keyrdata);
+                       dns_rdata_tostruct(&keyrdata, &ks, NULL);
 
-                       result = dst_key_fromdns(&sig.signer, rdata.rdclass, &b,
-                                                view->mctx, &key);
-                       if (result != ISC_R_SUCCESS) {
+                       if (sig.algorithm != ks.algorithm ||
+                           (ks.protocol != DNS_KEYPROTO_DNSSEC &&
+                            ks.protocol != DNS_KEYPROTO_ANY))
+                       {
                                continue;
                        }
-                       if (dst_key_alg(key) != sig.algorithm ||
-                           dst_key_id(key) != sig.keyid ||
-                           !(dst_key_proto(key) == DNS_KEYPROTO_DNSSEC ||
-                             dst_key_proto(key) == DNS_KEYPROTO_ANY))
-                       {
-                               dst_key_free(&key);
+
+                       dns_rdata_toregion(&keyrdata, &r);
+                       if (dst_region_computeid(&r) != sig.keyid) {
+                               continue;
+                       }
+
+                       result = dns_dnssec_keyfromrdata(&sig.signer, &keyrdata,
+                                                        view->mctx, &key);
+                       if (result != ISC_R_SUCCESS) {
                                continue;
                        }
+
                        result = dns_dnssec_verifymessage(&msgb, msg, key);
                        dst_key_free(&key);
                        if (result == ISC_R_SUCCESS) {

diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index 770a549ad0add46036f53a0c3e4ecb9335667d98..f103d8368bea7c7c9df1bd7394a71ba954428d7f 100644 (file)
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -1011,59 +1011,46 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
 static isc_result_t
 select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) {
        isc_result_t result;
-       dns_rdata_rrsig_t *siginfo = val->siginfo;
-       isc_buffer_t b;
-       dns_rdata_t rdata = DNS_RDATA_INIT;
-       dst_key_t *oldkey = val->key;
-       bool no_rdata = false;
 
-       if (oldkey == NULL) {
+       if (val->key == NULL) {
                result = dns_rdataset_first(rdataset);
        } else {
-               dst_key_free(&oldkey);
+               dst_key_free(&val->key);
                val->key = NULL;
                result = dns_rdataset_next(rdataset);
        }
-       if (result != ISC_R_SUCCESS) {
-               goto done;
+       if (result == ISC_R_NOMORE) {
+               return ISC_R_NOTFOUND;
        }
 
-       do {
+       for (; result == ISC_R_SUCCESS; result = dns_rdataset_next(rdataset)) {
+               dns_rdata_rrsig_t *siginfo = val->siginfo;
+               dns_rdata_t rdata = DNS_RDATA_INIT;
+               dns_rdata_dnskey_t key;
+               isc_region_t r;
+
                dns_rdataset_current(rdataset, &rdata);
+               dns_rdata_tostruct(&rdata, &key, NULL); /* can't fail */
 
-               isc_buffer_init(&b, rdata.data, rdata.length);
-               isc_buffer_add(&b, rdata.length);
-               INSIST(val->key == NULL);
-               result = dst_key_fromdns_ex(&siginfo->signer, rdata.rdclass, &b,
-                                           val->view->mctx, no_rdata,
-                                           &val->key);
-               if (result == ISC_R_SUCCESS) {
-                       if (siginfo->algorithm ==
-                                   (dns_secalg_t)dst_key_alg(val->key) &&
-                           siginfo->keyid ==
-                                   (dns_keytag_t)dst_key_id(val->key) &&
-                           (dst_key_flags(val->key) & DNS_KEYFLAG_REVOKE) ==
-                                   0 &&
-                           dst_key_iszonekey(val->key))
-                       {
-                               if (no_rdata) {
-                                       /* Retry with full key */
-                                       dns_rdata_reset(&rdata);
-                                       dst_key_free(&val->key);
-                                       no_rdata = false;
-                                       continue;
-                               }
-                               /* This is the key we're looking for. */
-                               goto done;
-                       }
-                       dst_key_free(&val->key);
+               if (key.algorithm != siginfo->algorithm ||
+                   (key.flags & DNS_KEYFLAG_REVOKE) != 0 ||
+                   !dns_dnssec_iszonekey(&key))
+               {
+                       continue;
                }
-               dns_rdata_reset(&rdata);
-               result = dns_rdataset_next(rdataset);
-               no_rdata = true;
-       } while (result == ISC_R_SUCCESS);
 
-done:
+               dns_rdata_toregion(&rdata, &r);
+               if (dst_region_computeid(&r) != siginfo->keyid) {
+                       continue;
+               }
+
+               result = dns_dnssec_keyfromrdata(&siginfo->signer, &rdata,
+                                                val->view->mctx, &val->key);
+               if (result == ISC_R_SUCCESS) {
+                       /* found the key we wanted */
+                       break;
+               }
+       }
        if (result == ISC_R_NOMORE) {
                result = ISC_R_NOTFOUND;
        }

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 44d13382818c7b80d3210c9e3ebe9d69bae9f105..c5e07bfecd7653a23782d093b27c8b34f80f9545 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -2420,25 +2420,33 @@ get_key(ns_client_t *client, dns_db_t *db, dns_rdata_rrsig_t *rrsig,
        for (; result == ISC_R_SUCCESS; result = dns_rdataset_next(keyrdataset))
        {
                dns_rdata_t rdata = DNS_RDATA_INIT;
-               isc_buffer_t b;
+               dns_rdata_dnskey_t key;
+               isc_region_t r;
 
                dns_rdataset_current(keyrdataset, &rdata);
-               isc_buffer_init(&b, rdata.data, rdata.length);
-               isc_buffer_add(&b, rdata.length);
-               result = dst_key_fromdns(&rrsig->signer, rdata.rdclass, &b,
-                                        client->manager->mctx, keyp);
-               if (result != ISC_R_SUCCESS) {
+               dns_rdata_tostruct(&rdata, &key, NULL); /* can't fail */
+
+               if (rrsig->algorithm != key.algorithm ||
+                   !dns_dnssec_iszonekey(&key))
+               {
                        continue;
                }
-               if (rrsig->algorithm == (dns_secalg_t)dst_key_alg(*keyp) &&
-                   rrsig->keyid == (dns_keytag_t)dst_key_id(*keyp) &&
-                   dst_key_iszonekey(*keyp))
-               {
+
+               dns_rdata_toregion(&rdata, &r);
+               if (dst_region_computeid(&r) != rrsig->keyid) {
+                       continue;
+               }
+
+               result = dns_dnssec_keyfromrdata(&rrsig->signer, &rdata,
+                                                client->manager->mctx, keyp);
+               if (result == ISC_R_SUCCESS) {
                        secure = true;
                        break;
                }
+
                dst_key_free(keyp);
        }
+
        return secure;
 }