Merge branch '331-further-refactoring-of-functions-in-lib-dns-zoneverify-c' into 'main'

Resolve "Further refactoring of functions in lib/dns/zoneverify.c"

Closes #331

See merge request isc-projects/bind9!3718
This commit is contained in:
Evan Hunt 2021-08-25 22:46:45 +00:00
commit 9a4a1bbe9b
2 changed files with 82 additions and 79 deletions

View file

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

View file

@ -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) &&