diff --git a/lib/dns/include/dns/keyvalues.h b/lib/dns/include/dns/keyvalues.h index cc5dea7f02..db63d40150 100644 --- a/lib/dns/include/dns/keyvalues.h +++ b/lib/dns/include/dns/keyvalues.h @@ -70,6 +70,7 @@ #define DNS_KEYALG_INDIRECT 252 #define DNS_KEYALG_PRIVATEDNS 253 #define DNS_KEYALG_PRIVATEOID 254 /*%< Key begins with OID giving alg */ +#define DNS_KEYALG_MAX 255 /* Protocol values */ #define DNS_KEYPROTO_RESERVED 0 diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c index fcd8b1ddb4..b51c0b1371 100644 --- a/lib/dns/zoneverify.c +++ b/lib/dns/zoneverify.c @@ -95,6 +95,15 @@ struct nsec3_chain_fixed { */ }; +/* + * Helper function used to calculate length of variable-length + * data section in object pointed to by 'chain'. + */ +static inline size_t +chain_length(struct nsec3_chain_fixed *chain) { + return (chain->salt_length + 2 * chain->next_length); +} + /*% * Log a zone verification error described by 'fmt' and the variable arguments * following it. Either use dns_zone_logv() or print to stderr, depending on @@ -310,18 +319,18 @@ check_no_rrsig(const vctx_t *vctx, const dns_rdataset_t *rdataset, if (sigrdataset.type == dns_rdatatype_rrsig && sigrdataset.covers == rdataset->type) { + dns_name_format(name, namebuf, sizeof(namebuf)); + dns_rdatatype_format(rdataset->type, typebuf, + sizeof(typebuf)); + zoneverify_log_error( + vctx, + "Warning: Found unexpected signatures " + "for %s/%s", + namebuf, typebuf); break; } dns_rdataset_disassociate(&sigrdataset); } - if (result == ISC_R_SUCCESS) { - dns_name_format(name, namebuf, sizeof(namebuf)); - dns_rdatatype_format(rdataset->type, typebuf, sizeof(typebuf)); - zoneverify_log_error(vctx, - "Warning: Found unexpected signatures " - "for %s/%s", - namebuf, typebuf); - } if (dns_rdataset_isassociated(&sigrdataset)) { dns_rdataset_disassociate(&sigrdataset); } @@ -333,8 +342,6 @@ check_no_rrsig(const vctx_t *vctx, const dns_rdataset_t *rdataset, static bool chain_compare(void *arg1, void *arg2) { struct nsec3_chain_fixed *e1 = arg1, *e2 = arg2; - size_t len; - /* * Do each element in turn to get a stable sort. */ @@ -362,8 +369,7 @@ chain_compare(void *arg1, void *arg2) { if (e1->next_length > e2->next_length) { return (false); } - len = e1->salt_length + 2 * e1->next_length; - if (memcmp(e1 + 1, e2 + 1, len) < 0) { + if (memcmp(e1 + 1, e2 + 1, chain_length(e1)) < 0) { return (true); } return (false); @@ -371,9 +377,7 @@ chain_compare(void *arg1, void *arg2) { static bool chain_equal(const struct nsec3_chain_fixed *e1, - const struct nsec3_chain_fixed *e2) { - size_t len; - + const struct nsec3_chain_fixed *e2, size_t data_length) { if (e1->hash != e2->hash) { return (false); } @@ -386,11 +390,8 @@ chain_equal(const struct nsec3_chain_fixed *e1, if (e1->next_length != e2->next_length) { return (false); } - len = e1->salt_length + 2 * e1->next_length; - if (memcmp(e1 + 1, e2 + 1, len) != 0) { - return (false); - } - return (true); + + return (memcmp(e1 + 1, e2 + 1, data_length) == 0); } static isc_result_t @@ -424,6 +425,40 @@ record_nsec3(const vctx_t *vctx, const unsigned char *rawhash, return (result); } +/* + * Check whether any NSEC3 within 'rdataset' matches the parameters in + * 'nsec3param'. + */ +static isc_result_t +find_nsec3_match(const dns_rdata_nsec3param_t *nsec3param, + dns_rdataset_t *rdataset, size_t rhsize, + dns_rdata_nsec3_t *nsec3_match) { + isc_result_t result; + + /* + * Find matching NSEC3 record. + */ + for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; + result = dns_rdataset_next(rdataset)) + { + dns_rdata_t rdata = DNS_RDATA_INIT; + dns_rdataset_current(rdataset, &rdata); + result = dns_rdata_tostruct(&rdata, nsec3_match, NULL); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + if (nsec3_match->hash == nsec3param->hash && + nsec3_match->next_length == rhsize && + nsec3_match->iterations == nsec3param->iterations && + nsec3_match->salt_length == nsec3param->salt_length && + memcmp(nsec3_match->salt, nsec3param->salt, + nsec3param->salt_length) == 0) + { + return (ISC_R_SUCCESS); + } + } + + return (result); +} + static isc_result_t match_nsec3(const vctx_t *vctx, const dns_name_t *name, const dns_rdata_nsec3param_t *nsec3param, dns_rdataset_t *rdataset, @@ -436,26 +471,7 @@ match_nsec3(const vctx_t *vctx, const dns_name_t *name, isc_result_t result; unsigned int len; - /* - * Find matching NSEC3 record. - */ - for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; - result = dns_rdataset_next(rdataset)) - { - dns_rdata_t rdata = DNS_RDATA_INIT; - dns_rdataset_current(rdataset, &rdata); - result = dns_rdata_tostruct(&rdata, &nsec3, NULL); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (nsec3.hash == nsec3param->hash && - nsec3.next_length == rhsize && - nsec3.iterations == nsec3param->iterations && - nsec3.salt_length == nsec3param->salt_length && - memcmp(nsec3.salt, nsec3param->salt, - nsec3param->salt_length) == 0) - { - break; - } - } + result = find_nsec3_match(nsec3param, rdataset, rhsize, &nsec3); if (result != ISC_R_SUCCESS) { dns_name_format(name, namebuf, sizeof(namebuf)); zoneverify_log_error(vctx, "Missing NSEC3 record for %s", @@ -806,14 +822,13 @@ verifynsec3s(const vctx_t *vctx, const dns_name_t *name, static isc_result_t verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, const dns_name_t *name, dns_dbnode_t *node, dst_key_t **dstkeys, size_t nkeys) { - unsigned char set_algorithms[256]; + unsigned char set_algorithms[256] = { 0 }; char namebuf[DNS_NAME_FORMATSIZE]; char algbuf[DNS_SECALG_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; dns_rdataset_t sigrdataset; dns_rdatasetiter_t *rdsiter = NULL; isc_result_t result; - int i; dns_rdataset_init(&sigrdataset); result = dns_db_allrdatasets(vctx->db, node, vctx->ver, 0, &rdsiter); @@ -838,7 +853,7 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, const dns_name_t *name, dns_rdatatype_format(rdataset->type, typebuf, sizeof(typebuf)); zoneverify_log_error(vctx, "No signatures for %s/%s", namebuf, typebuf); - for (i = 0; i < 256; i++) { + for (size_t i = 0; i < ARRAY_SIZE(set_algorithms); i++) { if (vctx->act_algorithms[i] != 0) { vctx->bad_algorithms[i] = 1; } @@ -847,7 +862,6 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, const dns_name_t *name, goto done; } - memset(set_algorithms, 0, sizeof(set_algorithms)); for (result = dns_rdataset_first(&sigrdataset); result == ISC_R_SUCCESS; result = dns_rdataset_next(&sigrdataset)) { @@ -881,10 +895,10 @@ verifyset(vctx_t *vctx, dns_rdataset_t *rdataset, const dns_name_t *name, result = ISC_R_SUCCESS; if (memcmp(set_algorithms, vctx->act_algorithms, - sizeof(set_algorithms))) { + sizeof(set_algorithms)) != 0) { dns_name_format(name, namebuf, sizeof(namebuf)); dns_rdatatype_format(rdataset->type, typebuf, sizeof(typebuf)); - for (i = 0; i < 256; i++) { + for (size_t i = 0; i < ARRAY_SIZE(set_algorithms); i++) { if ((vctx->act_algorithms[i] != 0) && (set_algorithms[i] == 0)) { dns_secalg_format(i, algbuf, sizeof(algbuf)); @@ -911,7 +925,7 @@ verifynode(vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node, bool delegation, dst_key_t **dstkeys, size_t nkeys, dns_rdataset_t *nsecset, dns_rdataset_t *nsec3paramset, const dns_name_t *nextname, isc_result_t *vresult) { - unsigned char types[8192]; + unsigned char types[8192] = { 0 }; unsigned int maxtype = 0; dns_rdataset_t rdataset; dns_rdatasetiter_t *rdsiter = NULL; @@ -919,13 +933,13 @@ verifynode(vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node, REQUIRE(vresult != NULL || (nsecset == NULL && nsec3paramset == NULL)); - memset(types, 0, sizeof(types)); result = dns_db_allrdatasets(vctx->db, node, vctx->ver, 0, &rdsiter); if (result != ISC_R_SUCCESS) { zoneverify_log_error(vctx, "dns_db_allrdatasets(): %s", isc_result_totext(result)); return (result); } + result = dns_rdatasetiter_first(rdsiter); dns_rdataset_init(&rdataset); while (result == ISC_R_SUCCESS) { @@ -1049,19 +1063,6 @@ check_no_nsec(const vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node) { return (nsec_exists ? ISC_R_FAILURE : ISC_R_SUCCESS); } -static bool -newchain(const struct nsec3_chain_fixed *first, - const struct nsec3_chain_fixed *e) { - if (first->hash != e->hash || first->iterations != e->iterations || - first->salt_length != e->salt_length || - first->next_length != e->next_length || - memcmp(first + 1, e + 1, first->salt_length) != 0) - { - return (true); - } - return (false); -} - static void free_element(isc_mem_t *mctx, struct nsec3_chain_fixed *e) { size_t len; @@ -1160,7 +1161,7 @@ verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) { /* * Check that they match. */ - if (chain_equal(e, f)) { + if (chain_equal(e, f, chain_length(e))) { free_element(mctx, f); f = NULL; } else { @@ -1183,7 +1184,9 @@ verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) { isc_heap_delete( vctx->found_chains, 1); } - if (f != NULL && chain_equal(e, f)) { + if (f != NULL && + chain_equal(e, f, chain_length(e))) + { free_element(mctx, f); f = NULL; break; @@ -1199,7 +1202,7 @@ verify_nsec3_chains(const vctx_t *vctx, isc_mem_t *mctx) { if (first == NULL) { prev = first = e; - } else if (newchain(first, e)) { + } else if (!chain_equal(first, e, first->salt_length)) { if (!checklast(mctx, vctx, first, prev)) { result = ISC_R_FAILURE; } @@ -1483,18 +1486,18 @@ check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, dns_dnssec_signs(keyrdata, vctx->origin, &vctx->soaset, &vctx->soasigs, false, vctx->mctx)) { - if (active_keys[dnskey->algorithm] != 255) { + if (active_keys[dnskey->algorithm] != DNS_KEYALG_MAX) { active_keys[dnskey->algorithm]++; } } else { - if (standby_keys[dnskey->algorithm] != 255) { + if (standby_keys[dnskey->algorithm] != DNS_KEYALG_MAX) { standby_keys[dnskey->algorithm]++; } } return; } - if (active_keys[dnskey->algorithm] != 255) { + if (active_keys[dnskey->algorithm] != DNS_KEYALG_MAX) { active_keys[dnskey->algorithm]++; } @@ -1602,9 +1605,9 @@ check_dnskey(vctx_t *vctx) { RUNTIME_CHECK(result == ISC_R_SUCCESS); is_ksk = ((dnskey.flags & DNS_KEYFLAG_KSK) != 0); - if ((dnskey.flags & DNS_KEYOWNER_ZONE) == 0) { - /* Non zone key, skip. */ - } else if ((dnskey.flags & DNS_KEYFLAG_REVOKE) != 0) { + if ((dnskey.flags & DNS_KEYOWNER_ZONE) != 0 && + (dnskey.flags & DNS_KEYFLAG_REVOKE) != 0) + { if ((dnskey.flags & DNS_KEYFLAG_KSK) != 0 && !dns_dnssec_selfsigns(&rdata, vctx->origin, &vctx->keyset, &vctx->keysigs, @@ -1634,11 +1637,13 @@ check_dnskey(vctx_t *vctx) { return (ISC_R_FAILURE); } if ((dnskey.flags & DNS_KEYFLAG_KSK) != 0 && - vctx->revoked_ksk[dnskey.algorithm] != 255) + vctx->revoked_ksk[dnskey.algorithm] != + DNS_KEYALG_MAX) { vctx->revoked_ksk[dnskey.algorithm]++; } else if ((dnskey.flags & DNS_KEYFLAG_KSK) == 0 && - vctx->revoked_zsk[dnskey.algorithm] != 255) + vctx->revoked_zsk[dnskey.algorithm] != + DNS_KEYALG_MAX) { vctx->revoked_zsk[dnskey.algorithm]++; } @@ -1657,11 +1662,10 @@ determine_active_algorithms(vctx_t *vctx, bool ignore_kskflag, bool keyset_kskonly, void (*report)(const char *, ...)) { char algbuf[DNS_SECALG_FORMATSIZE]; - int i; report("Verifying the zone using the following algorithms:"); - for (i = 0; i < 256; i++) { + for (size_t i = 0; i < ARRAY_SIZE(vctx->act_algorithms); i++) { if (ignore_kskflag) { vctx->act_algorithms[i] = (vctx->ksk_algorithms[i] != 0 || @@ -1683,7 +1687,7 @@ determine_active_algorithms(vctx_t *vctx, bool ignore_kskflag, return; } - for (i = 0; i < 256; i++) { + for (size_t i = 0; i < ARRAY_SIZE(vctx->ksk_algorithms); i++) { /* * The counts should both be zero or both be non-zero. Mark * the algorithm as bad if this is not met. @@ -1929,9 +1933,8 @@ static isc_result_t check_bad_algorithms(const vctx_t *vctx, void (*report)(const char *, ...)) { char algbuf[DNS_SECALG_FORMATSIZE]; bool first = true; - int i; - for (i = 0; i < 256; i++) { + for (size_t i = 0; i < ARRAY_SIZE(vctx->bad_algorithms); i++) { if (vctx->bad_algorithms[i] == 0) { continue; } @@ -1955,10 +1958,9 @@ static void print_summary(const vctx_t *vctx, bool keyset_kskonly, void (*report)(const char *, ...)) { char algbuf[DNS_SECALG_FORMATSIZE]; - int i; report("Zone fully signed:"); - for (i = 0; i < 256; i++) { + for (size_t i = 0; i < ARRAY_SIZE(vctx->ksk_algorithms); i++) { if ((vctx->ksk_algorithms[i] == 0) && (vctx->standby_ksk[i] == 0) && (vctx->revoked_ksk[i] == 0) &&