]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
lib/dnssec: replace kr_nsec_existence_denial()
authorVladimír Čunát <vladimir.cunat@nic.cz>
Sat, 23 Apr 2022 15:23:47 +0000 (17:23 +0200)
committerVladimír Čunát <vladimir.cunat@nic.cz>
Wed, 18 May 2022 13:49:23 +0000 (13:49 +0000)
The NSEC validation code has been written very mechanically
according to RFC 4033..4035, but those explain wildcard-related
topics in a way that's hard to understand right.

So here I rewrite it with a different philosophy, so it should be
easier to understand, a bit faster, and less buggy and bug-prone.

lib/dnssec/nsec.c
lib/dnssec/nsec.h
lib/layer/validate.c
tests/integration/deckard

index 0023ae09ff605790aa99aeaebebf53a5b195e726..fca02f2e22946f9f2b0aa56ef603eee826342493 100644 (file)
 #include "lib/defines.h"
 #include "lib/dnssec/nsec.h"
 #include "lib/utils.h"
-
+#include "resolve.h"
 
 int kr_nsec_children_in_zone_check(const uint8_t *bm, uint16_t bm_size)
 {
-       if (!bm)
+       if (kr_fails_assert(bm))
                return kr_error(EINVAL);
        const bool parent_side =
                dnssec_nsec_bitmap_contains(bm, bm_size, KNOT_RRTYPE_DNAME)
@@ -94,26 +94,28 @@ static int dname_cmp(const knot_dname_t *d1, const knot_dname_t *d2)
 
 
 /**
- * Check whether the NSEC RR proves that there is no closer match for <SNAME, SCLASS>.
+ * Check whether this nsec proves that there is no closer match for sname.
+ *
  * @param nsec  NSEC RRSet.
  * @param sname Searched name.
- * @return      0 if proves, >0 if not (abs(ENOENT)), or error code (<0).
+ * @return      0 if proves, >0 if not (abs(ENOENT) or abs(EEXIST)), or error code (<0).
  */
 static int nsec_covers(const knot_rrset_t *nsec, const knot_dname_t *sname)
 {
        if (kr_fails_assert(nsec && sname))
                return kr_error(EINVAL);
-       if (dname_cmp(sname, nsec->owner) <= 0)
-               return abs(ENOENT); /* 'sname' before 'owner', so can't be covered */
+       const int cmp = dname_cmp(sname, nsec->owner);
+       if (cmp  < 0) return abs(ENOENT); /* 'sname' before 'owner', so can't be covered */
+       if (cmp == 0) return abs(EEXIST); /* matched, not covered */
 
-       /* If NSEC 'owner' >= 'next', it means that there is nothing after 'owner' */
-       /* We have to lower-case it with libknot >= 2.7; see also RFC 6840 5.1. */
+       /* We have to lower-case 'next' with libknot >= 2.7; see also RFC 6840 5.1. */
        knot_dname_t next[KNOT_DNAME_MAXLEN];
        int ret = knot_dname_to_wire(next, knot_nsec_next(nsec->rrs.rdata), sizeof(next));
        if (kr_fails_assert(ret >= 0))
                return kr_error(ret);
        knot_dname_to_lower(next);
 
+       /* If NSEC 'owner' >= 'next', it means that there is nothing after 'owner' */
        const bool is_last_nsec = dname_cmp(nsec->owner, next) >= 0;
        const bool in_range = is_last_nsec || dname_cmp(sname, next) < 0;
        if (!in_range)
@@ -223,47 +225,6 @@ int kr_nsec_name_error_response_check(const knot_pkt_t *pkt, knot_section_t sect
        return kr_nsec_existence_denied(flags) ? kr_ok() : kr_error(ENOENT);
 }
 
-/**
- * Returns the labels from the covering RRSIG RRs.
- * @note The number must be the same in all covering RRSIGs.
- * @param nsec NSEC RR.
- * @param sec  Packet section.
- * @param      Number of labels or (negative) error code.
- */
-static int covering_rrsig_labels(const knot_rrset_t *nsec, const knot_pktsection_t *sec)
-{
-       if (kr_fails_assert(nsec && sec))
-               return kr_error(EINVAL);
-
-       int ret = kr_error(ENOENT);
-
-       for (unsigned i = 0; i < sec->count; ++i) {
-               const knot_rrset_t *rrset = knot_pkt_rr(sec, i);
-               if ((rrset->type != KNOT_RRTYPE_RRSIG) ||
-                   (!knot_dname_is_equal(rrset->owner, nsec->owner))) {
-                       continue;
-               }
-
-               knot_rdata_t *rdata_j = rrset->rrs.rdata;
-               for (uint16_t j = 0; j < rrset->rrs.count;
-                               ++j, rdata_j = knot_rdataset_next(rdata_j)) {
-                       if (knot_rrsig_type_covered(rdata_j) != KNOT_RRTYPE_NSEC)
-                               continue;
-
-                       if (ret < 0) {
-                               ret = knot_rrsig_labels(rdata_j);
-                       } else {
-                               if (ret != knot_rrsig_labels(rdata_j)) {
-                                       return kr_error(EINVAL);
-                               }
-                       }
-               }
-       }
-
-       return ret;
-}
-
-
 int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t type, const knot_dname_t *owner)
 {
        const int NO_PROOF = abs(ENOENT);
@@ -306,114 +267,14 @@ int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t ty
        return kr_ok();
 }
 
-/**
- * Attempt to prove NODATA given a matching NSEC.
- * @param flags Flags to be set according to check outcome.
- * @param nsec  NSEC RR.
- * @param type  Type to be checked.
- * @return      0 on success, abs(ENOENT) for no proof, or error code (<0).
- * @note        It's not a *full* proof, of course (wildcards, etc.)
- * @TODO returning result via `flags` is just ugly.
- */
-static int no_data_response_check_rrtype(int *flags, const knot_rrset_t *nsec,
-                                         uint16_t type)
+/// Convenience wrapper for kr_nsec_bitmap_nodata_check()
+static int no_data_response_check_rrtype(const knot_rrset_t *nsec, uint16_t type)
 {
-       if (kr_fails_assert(flags && nsec))
+       if (kr_fails_assert(nsec && nsec->type == KNOT_RRTYPE_NSEC))
                return kr_error(EINVAL);
-
        const uint8_t *bm = knot_nsec_bitmap(nsec->rrs.rdata);
        uint16_t bm_size = knot_nsec_bitmap_len(nsec->rrs.rdata);
-       int ret = kr_nsec_bitmap_nodata_check(bm, bm_size, type, nsec->owner);
-       if (ret == kr_ok())
-               *flags |= FLG_NOEXIST_RRTYPE;
-       return ret <= 0 ? ret : kr_ok();
-}
-
-/**
- * Perform check for RR type wildcard existence denial according to RFC4035 5.4, bullet 1.
- * @param flags Flags to be set according to check outcome.
- * @param nsec  NSEC RR.
- * @param sec   Packet section to work with.
- * @return      0 or error code.
- */
-static int no_data_wildcard_existence_check(int *flags, const knot_rrset_t *nsec,
-                                            const knot_pktsection_t *sec)
-{
-       if (kr_fails_assert(flags && nsec && sec))
-               return kr_error(EINVAL);
-
-       int rrsig_labels = covering_rrsig_labels(nsec, sec);
-       if (rrsig_labels < 0)
-               return rrsig_labels;
-       int nsec_labels = knot_dname_labels(nsec->owner, NULL);
-       if (nsec_labels < 0)
-               return nsec_labels;
-
-       if (rrsig_labels == nsec_labels)
-               *flags |= FLG_NOEXIST_WILDCARD;
-
-       return kr_ok();
-}
-
-/**
- * Perform check for NSEC wildcard existence that covers sname and
- * have no stype bit set.
- * @param pkt   Packet structure to be processed.
- * @param sec   Packet section to work with.
- * @param sname Queried domain name.
- * @param stype Queried type.
- * @return      0 or error code.
- */
-static int wildcard_match_check(const knot_pkt_t *pkt, const knot_pktsection_t *sec,
-                               const knot_dname_t *sname, uint16_t stype)
-{
-       if (!sec || !sname)
-               return kr_error(EINVAL);
-
-       int flags = 0;
-       for (unsigned i = 0; i < sec->count; ++i) {
-               const knot_rrset_t *rrset = knot_pkt_rr(sec, i);
-               if (rrset->type != KNOT_RRTYPE_NSEC)
-                       continue;
-               if (!knot_dname_is_wildcard(rrset->owner))
-                       continue;
-               if (!knot_dname_is_equal(rrset->owner, sname)) {
-                       int wcard_labels = knot_dname_labels(rrset->owner, NULL);
-                       int common_labels = knot_dname_matched_labels(rrset->owner, sname);
-                       int rrsig_labels = covering_rrsig_labels(rrset, sec);
-                       if (wcard_labels < 1 ||
-                           common_labels != wcard_labels - 1 ||
-                           common_labels != rrsig_labels) {
-                               continue;
-                       }
-               }
-               int ret = no_data_response_check_rrtype(&flags, rrset, stype);
-               if (ret != 0)
-                       return ret;
-       }
-       return (flags & FLG_NOEXIST_RRTYPE) ? kr_ok() : kr_error(ENOENT);
-}
-
-int kr_nsec_no_data_response_check(const knot_pkt_t *pkt, knot_section_t section_id,
-                                   const knot_dname_t *sname, uint16_t stype)
-{
-       const knot_pktsection_t *sec = knot_pkt_section(pkt, section_id);
-       if (!sec || !sname)
-               return kr_error(EINVAL);
-
-       int flags = 0;
-       for (unsigned i = 0; i < sec->count; ++i) {
-               const knot_rrset_t *rrset = knot_pkt_rr(sec, i);
-               if (rrset->type != KNOT_RRTYPE_NSEC)
-                       continue;
-               if (knot_dname_is_equal(rrset->owner, sname)) {
-                       int ret = no_data_response_check_rrtype(&flags, rrset, stype);
-                       if (ret != 0)
-                               return ret;
-               }
-       }
-
-       return (flags & FLG_NOEXIST_RRTYPE) ? kr_ok() : kr_error(ENOENT);
+       return kr_nsec_bitmap_nodata_check(bm, bm_size, type, nsec->owner);
 }
 
 int kr_nsec_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t section_id,
@@ -434,42 +295,73 @@ int kr_nsec_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t
        return kr_error(ENOENT);
 }
 
-int kr_nsec_existence_denial(const knot_pkt_t *pkt, knot_section_t section_id,
-                             const knot_dname_t *sname, uint16_t stype)
+int kr_nsec_negative(const ranked_rr_array_t *rrrs, uint32_t qry_uid,
+                       const knot_dname_t *sname, uint16_t stype)
 {
-       const knot_pktsection_t *sec = knot_pkt_section(pkt, section_id);
-       if (!sec || !sname)
+       // We really only consider the (canonically) first NSEC in each RRset.
+       // Using same owner with differing content probably isn't useful for NSECs anyway.
+       // Many other parts of code do the same, too.
+       if (kr_fails_assert(rrrs && sname))
                return kr_error(EINVAL);
 
-       int flags = 0;
-       for (unsigned i = 0; i < sec->count; ++i) {
-               const knot_rrset_t *rrset = knot_pkt_rr(sec, i);
-               if (rrset->type != KNOT_RRTYPE_NSEC)
-                       continue;
-               /* NSEC proves that name exists, but has no data (RFC4035 4.9, 1) */
-               if (knot_dname_is_equal(rrset->owner, sname)) {
-                       no_data_response_check_rrtype(&flags, rrset, stype);
-               } else {
-                       /* NSEC proves that name doesn't exist (RFC4035, 4.9, 2) */
-                       name_error_response_check_rr(&flags, rrset, sname);
+       // Terminology: https://datatracker.ietf.org/doc/html/rfc4592#section-3.3.1
+       int clencl_labels = -1; // the label count of the closest encloser of sname
+       for (int i = rrrs->len - 1; i >= 0; --i) { // NSECs near the end typically
+               const knot_rrset_t *nsec = rrrs->at[i]->rr;
+               bool ok = rrrs->at[i]->qry_uid == qry_uid
+                       && nsec->type == KNOT_RRTYPE_NSEC
+                       && kr_rank_test(rrrs->at[i]->rank, KR_RANK_SECURE);
+               if (!ok) continue;
+               const int covers = nsec_covers(nsec, sname);
+               if (covers == abs(EEXIST)
+                   && no_data_response_check_rrtype(nsec, stype) == 0) {
+                       return PKT_NODATA; // proven NODATA by matching NSEC
                }
-               no_data_wildcard_existence_check(&flags, rrset, sec);
+               if (covers != 0) continue;
+
+               // We have to lower-case 'next' with libknot >= 2.7; see also RFC 6840 5.1.
+               // LATER(optim.): it's duplicate work with the nsec_covers() call.
+               knot_dname_t next[KNOT_DNAME_MAXLEN];
+               int ret = knot_dname_to_wire(next, knot_nsec_next(nsec->rrs.rdata), sizeof(next));
+               if (kr_fails_assert(ret >= 0))
+                       return kr_error(ret);
+               knot_dname_to_lower(next);
+
+               clencl_labels = MAX(knot_dname_matched_labels(nsec->owner, sname),
+                                   knot_dname_matched_labels(sname, next));
+               break; // reduce indentation again
        }
-       if (kr_nsec_existence_denied(flags)) {
-               /* denial of existence proved accordingly to 4035 5.4 -
-                * NSEC proving either rrset non-existence or
-                * qtype non-existence has been found,
-                * and no wildcard expansion occurred.
-                */
-               return kr_ok();
-       } else if (kr_nsec_rrset_noexist(flags)) {
-               /* NSEC proving either rrset non-existence or
-                * qtype non-existence has been found,
-                * but wildcard expansion occurs.
-                * Try to find matching wildcard and check
-                * corresponding types.
-                */
-               return wildcard_match_check(pkt, sec, sname, stype);
+
+       if (clencl_labels < 0)
+               return kr_error(ENOENT);
+       const int sname_labels = knot_dname_labels(sname, NULL);
+       if (sname_labels == clencl_labels)
+               return PKT_NODATA; // proven NODATA; sname is an empty non-terminal
+
+       // Explicitly construct name for the corresponding source of synthesis.
+       knot_dname_t ssynth[KNOT_DNAME_MAXLEN + 2];
+       ssynth[0] = 1;
+       ssynth[1] = '*';
+       const knot_dname_t *clencl = sname;
+       for (int l = sname_labels; l > clencl_labels; --l)
+               clencl = knot_wire_next_label(clencl, NULL);
+       (void)!!knot_dname_store(&ssynth[2], clencl);
+
+       // Try to (dis)prove the source of synthesis by a covering or matching NSEC.
+       for (int i = rrrs->len - 1; i >= 0; --i) { // NSECs near the end typically
+               const knot_rrset_t *nsec = rrrs->at[i]->rr;
+               bool ok = rrrs->at[i]->qry_uid == qry_uid
+                       && nsec->type == KNOT_RRTYPE_NSEC
+                       && kr_rank_test(rrrs->at[i]->rank, KR_RANK_SECURE);
+               if (!ok) continue;
+               const int covers = nsec_covers(nsec, ssynth);
+               if (covers == abs(EEXIST)) {
+                       int ret = no_data_response_check_rrtype(nsec, stype);
+                       if (ret == 0) return PKT_NODATA; // proven NODATA by wildcard NSEC
+                       // TODO: also try expansion?  Or at least a different return code?
+               } else if (covers == 0) {
+                       return PKT_NXDOMAIN | PKT_NODATA;
+               }
        }
        return kr_error(ENOENT);
 }
index 1a2a092282fa8e7190c80155ff4cb40239e707d8..90d683e5b174a5a9fc4f39e6b5d281bad9f84dc0 100644 (file)
@@ -6,6 +6,8 @@
 
 #include <libknot/packet/pkt.h>
 
+#include "lib/layer/iterate.h"
+
 /**
  * Check bitmap that child names are contained in the same zone.
  * @note see RFC6840 4.1.
@@ -37,17 +39,6 @@ int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t ty
 int kr_nsec_name_error_response_check(const knot_pkt_t *pkt, knot_section_t section_id,
                                       const knot_dname_t *sname);
 
-/**
- * No data response check (RFC4035 3.1.3.1; RFC4035 5.4, bullet 1).
- * @param pkt        Packet structure to be processed.
- * @param section_id Packet section to be processed.
- * @param sname      Name to be checked.
- * @param stype      Type to be checked.
- * @return           0 or error code.
- */
-int kr_nsec_no_data_response_check(const knot_pkt_t *pkt, knot_section_t section_id,
-                                   const knot_dname_t *sname, uint16_t stype);
-
 /**
  * Wildcard answer response check (RFC4035 3.1.3.3).
  * @param pkt        Packet structure to be processed.
@@ -59,16 +50,14 @@ int kr_nsec_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t
                                            const knot_dname_t *sname);
 
 /**
- * Authenticated denial of existence according to RFC4035 5.4.
- * @note No RRSIGs are validated.
- * @param pkt        Packet structure to be processed.
- * @param section_id Packet section to be processed.
- * @param sname      Queried domain name.
- * @param stype      Queried type.
- * @return           0 or error code.
+ * Search for a negative proof for sname+stype among validated NSECs.
+ *
+ * @param rrrs       list of RRs to search; typically kr_request::auth_selected
+ * @param qry_uid    only consider NSECs from this packet, for better efficiency
+ * @return           negative error code, or PKT_NXDOMAIN | PKT_NODATA (both for NXDOMAIN)
  */
-int kr_nsec_existence_denial(const knot_pkt_t *pkt, knot_section_t section_id,
-                             const knot_dname_t *sname, uint16_t stype);
+int kr_nsec_negative(const ranked_rr_array_t *rrrs, uint32_t qry_uid,
+                       const knot_dname_t *sname, uint16_t stype);
 
 /**
  * Referral to unsigned subzone check (RFC4035 5.2).
index 7f372d4ba9be3986352f5d01aec3c2d895e28c04..2ad18b62a6e229755a5e48c754e4d9693b6b889d 100644 (file)
@@ -539,7 +539,15 @@ static int update_delegation(struct kr_request *req, struct kr_query *qry, knot_
                                ret = kr_nsec_ref_to_unsigned(answer);
                        } else {
                                /* No-data answer */
-                               ret = kr_nsec_existence_denial(answer, KNOT_AUTHORITY, proved_name, KNOT_RRTYPE_DS);
+                               ret = kr_nsec_negative(&req->auth_selected, qry->uid,
+                                                       proved_name, KNOT_RRTYPE_DS);
+                               if (ret >= 0) {
+                                       if (ret == PKT_NODATA) {
+                                               ret = kr_ok();
+                                       } else {
+                                               ret = kr_error(ENOENT); // suspicious
+                                       }
+                               }
                        }
                } else {
                        if (referral) {
@@ -1202,7 +1210,15 @@ static int validate(kr_layer_t *ctx, knot_pkt_t *pkt)
                         * ? merge the functionality together to share code/resources
                         */
                        if (!has_nsec3) {
-                               ret = kr_nsec_existence_denial(pkt, KNOT_AUTHORITY, knot_pkt_qname(pkt), knot_pkt_qtype(pkt));
+                               ret = kr_nsec_negative(&req->auth_selected, qry->uid,
+                                                       knot_pkt_qname(pkt), knot_pkt_qtype(pkt));
+                               if (ret >= 0) {
+                                       if (ret == PKT_NODATA) {
+                                               ret = kr_ok();
+                                       } else {
+                                               ret = kr_error(ENOENT); // suspicious
+                                       }
+                               }
                        } else {
                                ret = kr_nsec3_no_data(pkt, KNOT_AUTHORITY, knot_pkt_qname(pkt), knot_pkt_qtype(pkt));
                        }
index c4628156e6cf499e8052c321b24793a176ded494..d331b5013e72601a88158d67677c7e0c44e4d1dd 160000 (submodule)
@@ -1 +1 @@
-Subproject commit c4628156e6cf499e8052c321b24793a176ded494
+Subproject commit d331b5013e72601a88158d67677c7e0c44e4d1dd