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.
This commit is contained in:
Evan Hunt 2025-03-14 16:41:47 -07:00
parent 341b962665
commit 2e6107008d
5 changed files with 79 additions and 93 deletions

View file

@ -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;
}
}

View file

@ -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);
/*%<

View file

@ -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 (sig.algorithm != ks.algorithm ||
(ks.protocol != DNS_KEYPROTO_DNSSEC &&
ks.protocol != DNS_KEYPROTO_ANY))
{
continue;
}
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;
}
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);
continue;
}
result = dns_dnssec_verifymessage(&msgb, msg, key);
dst_key_free(&key);
if (result == ISC_R_SUCCESS) {

View file

@ -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;
}

View file

@ -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;
}