From: Libor Peltan Date: Thu, 5 Aug 2021 08:55:57 +0000 (+0200) Subject: bugfix/dnssec: sign NSECs correctly X-Git-Tag: v3.1.1~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5295993573b9c2080c587a11ae2264243c5a0646;p=thirdparty%2Fknot-dns.git bugfix/dnssec: sign NSECs correctly previously, adding RRSIG to the signed node confused iteration over RRSets, leading to duplicit RRSIGs of some NSEC(3)s --- diff --git a/src/knot/dnssec/zone-sign.c b/src/knot/dnssec/zone-sign.c index c23042ce5e..a945abad4e 100644 --- a/src/knot/dnssec/zone-sign.c +++ b/src/knot/dnssec/zone-sign.c @@ -975,6 +975,20 @@ bool knot_zone_sign_soa_expired(const zone_contents_t *zone, return !exist; } +static int sign_in_changeset(zone_node_t *node, uint16_t rrtype, knot_rrset_t *rrsigs, + zone_sign_ctx_t *sign_ctx, int ret_prev, + bool skip_crypto, zone_update_t *up) +{ + if (ret_prev != KNOT_EOK) { + return ret_prev; + } + knot_rrset_t rr = node_rrset(node, rrtype); + if (knot_rrset_empty(&rr)) { + return KNOT_EOK; + } + return add_missing_rrsigs(&rr, rrsigs, sign_ctx, skip_crypto, NULL, up, NULL); +} + int knot_zone_sign_nsecs_in_changeset(const zone_keyset_t *zone_keys, const kdnssec_ctx_t *dnssec_ctx, zone_update_t *update) @@ -996,14 +1010,9 @@ int knot_zone_sign_nsecs_in_changeset(const zone_keyset_t *zone_keys, bool skip_crypto = (n->flags & NODE_FLAGS_RRSIGS_VALID) && !dnssec_ctx->keytag_conflict; knot_rrset_t rrsigs = node_rrset(n, KNOT_RRTYPE_RRSIG); - for (int i = 0; i < n->rrset_count; i++) { - knot_rrset_t rr = node_rrset_at(n, i); - if (rr.type == KNOT_RRTYPE_NSEC || - rr.type == KNOT_RRTYPE_NSEC3 || - rr.type == KNOT_RRTYPE_NSEC3PARAM) { - ret = add_missing_rrsigs(&rr, &rrsigs, sign_ctx, skip_crypto, NULL, update, NULL); - } - } + ret = sign_in_changeset(n, KNOT_RRTYPE_NSEC, &rrsigs, sign_ctx, ret, skip_crypto, update); + ret = sign_in_changeset(n, KNOT_RRTYPE_NSEC3, &rrsigs, sign_ctx, ret, skip_crypto, update); + ret = sign_in_changeset(n, KNOT_RRTYPE_NSEC3PARAM, &rrsigs, sign_ctx, ret, skip_crypto, update); if (ret == KNOT_EOK) { n->flags |= NODE_FLAGS_RRSIGS_VALID; // non-NSEC RRSIGs had been validated in knot_dnssec_sign_update() diff --git a/tests-extra/tests/dnssec/nsec_update/test.py b/tests-extra/tests/dnssec/nsec_update/test.py index 666c4d9d19..0ccb6579a2 100644 --- a/tests-extra/tests/dnssec/nsec_update/test.py +++ b/tests-extra/tests/dnssec/nsec_update/test.py @@ -6,6 +6,40 @@ from dnstest.utils import * from dnstest.test import Test from dnstest.keys import Keymgr import random +import dns + +def check_nsec(server, zone, msg, name="dwidjwoij"): + + q = server.dig(name + "." + zone.name, "AAAA", dnssec=True, udp=False) + found_soas = q.count("SOA", section="authority") + found_nsecs = q.count("NSEC", section="authority") + if found_nsecs == 0: + found_nsecs = q.count("NSEC3", section="authority") + found_rrsigs = q.count("RRSIG", section="authority") + + check_log("Authority %s: %s" % (zone.name, msg)) + check_log("SOAs: %d" % found_soas) + check_log("NSECs: %d" % found_nsecs) + check_log("RRSIGs: %d" % found_rrsigs) + + if found_soas != 1: + set_err("No SOA authority (%d): %s" % (found_soas, msg)) + + if found_nsecs < 1: + set_err("No NSEC(3) authority: %s" % msg) + + if found_nsecs > 3: + set_err("Too many NSEC(3)s authority (%d): %s" % (found_nsecs, msg)) + + if found_rrsigs != found_soas + found_nsecs: + set_err("Unmatching RRSIGs (%d != %d + %d): %s" % (found_rrsigs, found_soas, found_nsecs, msg)) + detail_log("Unmatching RRSIGs [%s] (%d != %d + %d): %s" % (zone.name, found_rrsigs, found_soas, found_nsecs, msg)) + for data in q.resp.authority: + rrset = data.to_rdataset() + if rrset.rdtype == dns.rdatatype.NSEC or rrset.rdtype == dns.rdatatype.NSEC3 or rrset.rdtype == dns.rdatatype.RRSIG: + detail_log(str(data)) + + detail_log(SEP) t = Test() @@ -44,6 +78,8 @@ if master.valgrind: t.start() master.zones_wait(zones) +for z in zones: + check_nsec(master, z, "Initial") master.ctl("zone-flush") slave.ctl("zone-refresh") slave.zones_wait(zones) @@ -67,6 +103,8 @@ t.sleep(1) master.ctl("zone-refresh", wait=True) after_update = master.zones_wait(zones) +for z in zones: + check_nsec(master, z, "After DDNS") # sync slave with current master's state slave.ctl("zone-refresh") @@ -78,6 +116,8 @@ slave.flush(wait=True) # re-sign master and check that the re-sign made nothing master.ctl("zone-sign") after_update15 = master.zones_wait(zones, after_update, equal=False, greater=True) +for z in zones: + check_nsec(master, z, "After re-sign") t.xfr_diff(master, slave, zones, no_rrsig_rdata=True) for zone in zones: @@ -114,6 +154,8 @@ t.sleep(1) master.ctl("zone-refresh", wait=True) after_update2 = master.zones_wait(zones, after_update15, equal=False, greater=True) +for z in zones: + check_nsec(master, z, "After delegation update") # sync slave with current master's state slave.ctl("zone-refresh") @@ -125,6 +167,8 @@ slave.flush(wait=True) # re-sign master and check that the re-sign made nothing master.ctl("zone-sign") after_update25 = master.zones_wait(zones, after_update2, equal=False, greater=True) +for z in zones: + check_nsec(master, z, "After second re-sign") t.xfr_diff(master, slave, zones, no_rrsig_rdata=True) for zone in zones: @@ -148,4 +192,7 @@ for z in zones1: slave.zone_wait(z, after_update25[z.name], equal=False, greater=True) slave.zone_verify(z) +for z in zones: + check_nsec(master, z, "After re-salt") + t.end()