From 5339276ada8127e49af0e11a9009c5dbcec6ec3e Mon Sep 17 00:00:00 2001
From: Jan Kadlec <jan.kadlec@nic.cz>
Date: Fri, 21 Feb 2014 17:33:20 +0100
Subject: [PATCH] Review: Simpler remove_nsec(), unified synth_rrsig() error
checking.
---
src/knot/dnssec/nsec-chain.c | 93 ++++++++++++++++---------------
src/knot/dnssec/nsec-chain.h | 7 +--
src/knot/dnssec/nsec3-chain.c | 19 ++-----
src/knot/dnssec/zone-sign.c | 24 ++++----
src/knot/nameserver/nsec_proofs.c | 2 +
src/knot/zone/semantic-check.c | 7 ++-
src/libknot/dnssec/rrset-sign.c | 6 +-
7 files changed, 79 insertions(+), 79 deletions(-)
diff --git a/src/knot/dnssec/nsec-chain.c b/src/knot/dnssec/nsec-chain.c
index f048946e3..57f859a4e 100644
--- a/src/knot/dnssec/nsec-chain.c
+++ b/src/knot/dnssec/nsec-chain.c
@@ -72,8 +72,6 @@ static int update_nsec(const knot_node_t *from, const knot_node_t *to,
assert(from && to && out_ch);
const knot_rrset_t *nsec_rrset = knot_node_rrset(from,
KNOT_RRTYPE_NSEC);
- const knot_rrset_t *rrsigs = knot_node_rrset(from,
- KNOT_RRTYPE_RRSIG);
// Create new NSEC
knot_rrset_t *new_nsec;
if (only_nsec_in_node(from)) {
@@ -93,8 +91,7 @@ static int update_nsec(const knot_node_t *from, const knot_node_t *to,
dbg_dnssec_detail("Creating new NSEC for %s\n",
knot_dname_to_str(new_nsec->owner));
// Drop old
- int ret = knot_nsec_changeset_remove(nsec_rrset, rrsigs,
- out_ch);
+ int ret = knot_nsec_changeset_remove(from, out_ch);
if (ret != KNOT_EOK) {
knot_rrset_deep_free(&new_nsec, 1, NULL);
return ret;
@@ -121,8 +118,7 @@ static int update_nsec(const knot_node_t *from, const knot_node_t *to,
}
} else {
// Drop old, no longer needed
- int ret = knot_nsec_changeset_remove(nsec_rrset, rrsigs,
- out_ch);
+ int ret = knot_nsec_changeset_remove(from, out_ch);
if (ret != KNOT_EOK) {
knot_rrset_deep_free(&new_nsec, 1, NULL);
return ret;
@@ -208,7 +204,6 @@ static int connect_nsec_nodes(knot_node_t *a, knot_node_t *b,
}
knot_rrset_t *old_next_nsec = knot_node_get_rrset(b, KNOT_RRTYPE_NSEC);
- const knot_rrset_t *old_next_rrsigs = knot_node_get_rrset(b, KNOT_RRTYPE_RRSIG);
int ret = 0;
/*!
@@ -217,8 +212,7 @@ static int connect_nsec_nodes(knot_node_t *a, knot_node_t *b,
*/
if (old_next_nsec != NULL
&& knot_nsec_only_nsec_and_rrsigs_in_node(b)) {
- ret = knot_nsec_changeset_remove(old_next_nsec, old_next_rrsigs,
- data->changeset);
+ ret = knot_nsec_changeset_remove(b, data->changeset);
if (ret != KNOT_EOK) {
return ret;
}
@@ -234,7 +228,6 @@ static int connect_nsec_nodes(knot_node_t *a, knot_node_t *b,
}
knot_rrset_t *old_nsec = knot_node_get_rrset(a, KNOT_RRTYPE_NSEC);
- const knot_rrset_t *old_rrsigs = knot_node_get_rrset(a, KNOT_RRTYPE_RRSIG);
if (old_nsec != NULL) {
if (knot_rrset_equal(new_nsec, old_nsec,
KNOT_RRSET_COMPARE_WHOLE)) {
@@ -248,7 +241,7 @@ static int connect_nsec_nodes(knot_node_t *a, knot_node_t *b,
// current NSEC is invalid, replace it and drop RRSIG
// mark the node, so later we know this NSEC needs new RRSIGs
knot_node_set_replaced_nsec(a);
- ret = knot_nsec_changeset_remove(old_nsec, old_rrsigs, data->changeset);
+ ret = knot_nsec_changeset_remove(a, data->changeset);
if (ret != KNOT_EOK) {
knot_rrset_deep_free(&new_nsec, 1, NULL);
return ret;
@@ -279,10 +272,8 @@ static int handle_deleted_node(const knot_node_t *node,
assert(knot_node_is_non_auth(node));
return NSEC_NODE_SKIP;
}
- const knot_rrset_t *old_nsec = knot_node_rrset(node, KNOT_RRTYPE_NSEC);
- assert(old_nsec);
- const knot_rrset_t *old_rrsigs = knot_node_rrset(node, KNOT_RRTYPE_RRSIG);
- int ret = knot_nsec_changeset_remove(old_nsec, old_rrsigs, fix_data->out_ch);
+
+ int ret = knot_nsec_changeset_remove(node, fix_data->out_ch);
if (ret != KNOT_EOK) {
return ret;
}
@@ -292,6 +283,9 @@ static int handle_deleted_node(const knot_node_t *node,
* previous node.
*/
if (fix_data->next_dname == NULL) {
+ const knot_rrset_t *old_nsec =
+ knot_node_rrset(node, KNOT_RRTYPE_NSEC);
+ assert(old_nsec);
fix_data->next_dname =
(knot_dname_t *)knot_rdata_nsec_next(old_nsec);
assert(fix_data->next_dname);
@@ -619,51 +613,58 @@ int knot_nsec_chain_iterate_fix(hattrie_t *nodes, chain_iterate_fix_cb callback,
/*!
* \brief Add entry for removed NSEC to the changeset.
*/
-int knot_nsec_changeset_remove(const knot_rrset_t *oldrr,
- const knot_rrset_t *rrsigs,
+int knot_nsec_changeset_remove(const knot_node_t *n,
knot_changeset_t *changeset)
{
- if (oldrr == NULL) {
- return KNOT_EOK;
- }
if (changeset == NULL) {
return KNOT_EINVAL;
}
- int result;
+ int result = KNOT_EOK;
+
+ const knot_rrset_t *nsec = knot_node_rrset(n, KNOT_RRTYPE_NSEC);
+ if (nsec == NULL) {
+ nsec = knot_node_rrset(n, KNOT_RRTYPE_NSEC3);
+ }
+ const knot_rrset_t *rrsigs = knot_node_rrset(n, KNOT_RRTYPE_RRSIG);
// extract copy of NSEC
knot_rrset_t *old_nsec = NULL;
- result = knot_rrset_deep_copy(oldrr, &old_nsec, NULL);
- if (result != KNOT_EOK) {
- return result;
- }
+ if (nsec) {
+ result = knot_rrset_deep_copy(nsec, &old_nsec, NULL);
+ if (result != KNOT_EOK) {
+ return result;
+ }
- // update changeset
+ // update changeset
- result = knot_changeset_add_rrset(changeset, old_nsec,
- KNOT_CHANGESET_REMOVE);
- if (result != KNOT_EOK) {
- knot_rrset_deep_free(&old_nsec, 1, NULL);
- return result;
+ result = knot_changeset_add_rrset(changeset, old_nsec,
+ KNOT_CHANGESET_REMOVE);
+ if (result != KNOT_EOK) {
+ knot_rrset_deep_free(&old_nsec, 1, NULL);
+ return result;
+ }
}
- // extract copy of RRSIG
- knot_rrset_t *synth_rrsigs = NULL;
- result = knot_rrset_synth_rrsig(rrsigs, oldrr->type, &synth_rrsigs, NULL);
- if (result != KNOT_EOK && result != KNOT_ENOENT) {
- return result;
- }
- if (result == KNOT_ENOENT) {
- return KNOT_EOK;
- }
+ if (rrsigs) {
+ // Sythesize RRSets' RRSIG
+ knot_rrset_t *synth_rrsigs = NULL;
+ result = knot_rrset_synth_rrsig(rrsigs, KNOT_RRTYPE_NSEC,
+ &synth_rrsigs, NULL);
+ if (result != KNOT_EOK) {
+ if (result != KNOT_ENOENT) {
+ return result;
+ }
+ return KNOT_EOK;
+ }
- // store RRSIG
- result = knot_changeset_add_rrset(changeset, synth_rrsigs,
- KNOT_CHANGESET_REMOVE);
- if (result != KNOT_EOK) {
- knot_rrset_deep_free(&synth_rrsigs, 1, NULL);
- return result;
+ // store RRSIG
+ result = knot_changeset_add_rrset(changeset, synth_rrsigs,
+ KNOT_CHANGESET_REMOVE);
+ if (result != KNOT_EOK) {
+ knot_rrset_deep_free(&synth_rrsigs, 1, NULL);
+ return result;
+ }
}
return KNOT_EOK;
diff --git a/src/knot/dnssec/nsec-chain.h b/src/knot/dnssec/nsec-chain.h
index 4aa826171..39f0ac730 100644
--- a/src/knot/dnssec/nsec-chain.h
+++ b/src/knot/dnssec/nsec-chain.h
@@ -136,15 +136,14 @@ int knot_nsec_chain_iterate_fix(hattrie_t *nodes,
chain_fix_data_t *data);
/*!
- * \brief Add entry for removed NSEC to the changeset.
+ * \brief Add entry for removed NSEC(3) and its RRSIG to the changeset.
*
- * \param oldrr Old NSEC RR set to be removed (including RRSIG).
+ * \param n Node to extract NSEC(3) from.
* \param changeset Changeset to add the old RR into.
*
* \return Error code, KNOT_EOK if successful.
*/
-int knot_nsec_changeset_remove(const knot_rrset_t *oldrr,
- const knot_rrset_t *rrsigs,
+int knot_nsec_changeset_remove(const knot_node_t *n,
knot_changeset_t *changeset);
/*!
diff --git a/src/knot/dnssec/nsec3-chain.c b/src/knot/dnssec/nsec3-chain.c
index 7454dcb85..4786b8060 100644
--- a/src/knot/dnssec/nsec3-chain.c
+++ b/src/knot/dnssec/nsec3-chain.c
@@ -348,10 +348,7 @@ static int update_nsec3(const knot_dname_t *from, const knot_dname_t *to,
// Drop old
int ret = KNOT_EOK;
if (old_nsec3) {
-
- const knot_rrset_t *rrsigs = knot_node_rrset(from_node,
- KNOT_RRTYPE_RRSIG);
- ret = knot_nsec_changeset_remove(old_nsec3, rrsigs, out_ch);
+ ret = knot_nsec_changeset_remove(from_node, out_ch);
if (ret != KNOT_EOK) {
knot_rrset_deep_free(&gen_nsec3, 1, NULL);
return ret;
@@ -741,11 +738,7 @@ static int create_nsec3_nodes(const knot_zone_contents_t *zone, uint32_t ttl,
* Remove possible NSEC from the node. (Do not allow both NSEC
* and NSEC3 in the zone at once.)
*/
- result = knot_nsec_changeset_remove(knot_node_rrset(node,
- KNOT_RRTYPE_NSEC),
- knot_node_rrset(node,
- KNOT_RRTYPE_RRSIG),
- chgset);
+ result = knot_nsec_changeset_remove(node, chgset);
if (result != KNOT_EOK) {
break;
}
@@ -1063,10 +1056,7 @@ static int handle_deleted_node(const knot_node_t *node,
assert(knot_node_is_non_auth(node));
return NSEC_NODE_SKIP;
}
- const knot_rrset_t *old_nsec3 = knot_node_rrset(node, KNOT_RRTYPE_NSEC3);
- assert(old_nsec3);
- const knot_rrset_t *old_rrsigs = knot_node_rrset(node, KNOT_RRTYPE_RRSIG);
- int ret = knot_nsec_changeset_remove(old_nsec3, old_rrsigs, fix_data->out_ch);
+ int ret = knot_nsec_changeset_remove(node, fix_data->out_ch);
if (ret != KNOT_EOK) {
return ret;
}
@@ -1076,6 +1066,9 @@ static int handle_deleted_node(const knot_node_t *node,
* previous node.
*/
if (fix_data->next_dname == NULL) {
+ const knot_rrset_t *old_nsec3 =
+ knot_node_rrset(node, KNOT_RRTYPE_NSEC3);
+ assert(old_nsec3);
fix_data->next_dname =
next_dname_from_nsec3_rrset(old_nsec3,
fix_data->zone->apex->owner);
diff --git a/src/knot/dnssec/zone-sign.c b/src/knot/dnssec/zone-sign.c
index 3abac02c8..e2d7d56e9 100644
--- a/src/knot/dnssec/zone-sign.c
+++ b/src/knot/dnssec/zone-sign.c
@@ -220,12 +220,11 @@ static int remove_expired_rrsigs(const knot_rrset_t *covered,
knot_rrset_t *synth_rrsig = NULL;
result = knot_rrset_synth_rrsig(rrsigs, covered->type,
&synth_rrsig, NULL);
- if (result == KNOT_ENOENT) {
- // Nothing to remove
- return KNOT_EOK;
- }
if (result != KNOT_EOK) {
- return result;
+ if (result != KNOT_ENOENT) {
+ return result;
+ }
+ return KNOT_EOK;
}
size_t rrsig_rdata_count = knot_rrset_rr_count(synth_rrsig);
@@ -359,12 +358,11 @@ static int remove_rrset_rrsigs(const knot_rrset_t *rrset,
knot_rrset_t *synth_rrsig = NULL;
int ret = knot_rrset_synth_rrsig(rrsigs, rrset->type, &synth_rrsig, NULL);
- if (ret == KNOT_ENOENT) {
- // Nothing to remove
- return KNOT_EOK;
- }
if (ret != KNOT_EOK) {
- return ret;
+ if (ret != KNOT_ENOENT) {
+ return ret;
+ }
+ return KNOT_EOK;
}
ret = knot_changeset_add_rrset(changeset, synth_rrsig, KNOT_CHANGESET_REMOVE);
@@ -1008,8 +1006,10 @@ static int update_dnskeys(const knot_zone_contents_t *zone,
knot_rrset_t *dnskey_rrsig = NULL;
result = knot_rrset_synth_rrsig(rrsigs, KNOT_RRTYPE_DNSKEY,
&dnskey_rrsig, NULL);
- if (result != KNOT_EOK && result != KNOT_ENOENT) {
- return result;
+ if (result != KNOT_EOK) {
+ if (result != KNOT_ENOENT) {
+ return result;
+ }
}
bool modified = (knot_changeset_size(changeset) != changes_before);
diff --git a/src/knot/nameserver/nsec_proofs.c b/src/knot/nameserver/nsec_proofs.c
index a89f7deed..8c2b5daf5 100644
--- a/src/knot/nameserver/nsec_proofs.c
+++ b/src/knot/nameserver/nsec_proofs.c
@@ -802,6 +802,8 @@ int nsec_append_rrsigs(knot_pkt_t *pkt, struct query_data *qdata, bool optional)
if (ret != KNOT_ENOENT) {
return ret;
}
+ // Nothing to insert
+ continue;
}
ret = knot_pkt_put(pkt, compr_hint, synth_sig, flags);
if (ret != KNOT_EOK) {
diff --git a/src/knot/zone/semantic-check.c b/src/knot/zone/semantic-check.c
index 79242cf61..90dfe36c2 100644
--- a/src/knot/zone/semantic-check.c
+++ b/src/knot/zone/semantic-check.c
@@ -442,12 +442,13 @@ static int check_rrsig_in_rrset(err_handler_t *handler,
ret = knot_rrset_synth_rrsig(knot_node_rrset(node, KNOT_RRTYPE_RRSIG),
rrset->type,
&rrsigs, NULL);
- if (ret != KNOT_EOK && ret != KNOT_ENOENT) {
- return ret;
+ if (ret != KNOT_EOK) {
+ if (ret != KNOT_ENOENT) {
+ return ret;
+ }
}
if (rrsigs == NULL) {
- assert(ret == KNOT_ENOENT);
err_handler_handle_error(handler, node,
ZC_ERR_RRSIG_NO_RRSIG,
info_str);
diff --git a/src/libknot/dnssec/rrset-sign.c b/src/libknot/dnssec/rrset-sign.c
index 5e9184a37..1810af7c3 100644
--- a/src/libknot/dnssec/rrset-sign.c
+++ b/src/libknot/dnssec/rrset-sign.c
@@ -364,7 +364,11 @@ int knot_is_valid_signature(const knot_rrset_t *covered,
int result = knot_rrset_synth_rrsig(rrsigs, covered->type,
&synth_rrsigs, NULL);
if (result != KNOT_EOK) {
- return result;
+ if (result != KNOT_ENOENT) {
+ return result;
+ }
+ // No signature exists
+ return KNOT_EINVAL;
}
// identify fields in the signature being validated
--
GitLab