Remove redundant function 'newchain'

The removed function 'newchain(a, b)' was almost the same as calling
!chain_equal(a, b), varying only in the amount of data compared
in the non-fixed-length data portion of given chain nodes.

A third argument 'data_size' has been introduced into 'chain_equal'
function in order to allow it to know how many bytes to compare in the
variable-length data portion of the chain nodes.

A helper function 'chain_length(e)' has been introduced to allow
easy calculation of the total length of the non-fixed-length data part
of chain nodes.

Check the thread below for more details:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12184
This commit is contained in:
Diego Fronza 2020-06-26 18:53:04 -03:00 committed by Evan Hunt
parent 6a12e37382
commit 37f42d19a1

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
@ -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
@ -1062,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;
@ -1173,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 {
@ -1196,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;
@ -1212,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;
}