From: Vladimír Čunát Date: Fri, 12 Jan 2018 15:15:08 +0000 (+0100) Subject: validator: fix NSEC* -> NODATA X-Git-Tag: v1.5.2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f90d27de49c9d3be0424d5d5457fb18df7d5c3f3;p=thirdparty%2Fknot-resolver.git validator: fix NSEC* -> NODATA --- diff --git a/lib/dnssec/nsec.c b/lib/dnssec/nsec.c index ea64678ea..52bfb9430 100644 --- a/lib/dnssec/nsec.c +++ b/lib/dnssec/nsec.c @@ -216,40 +216,74 @@ static int coverign_rrsig_labels(const knot_rrset_t *nsec, const knot_pktsection return ret; } -/** - * Perform check of RR type existence denial according to RFC4035 5.4, bullet 1. - * @param flags Flags to be set according to check outcome. - * @param nsec NSEC RR. - * @param type Type to be checked. - * @return 0 or error code. - */ -static int no_data_response_check_rrtype(int *flags, const knot_rrset_t *nsec, - uint16_t type) -{ - assert(flags && nsec); - uint8_t *bm = NULL; - uint16_t bm_size; - knot_nsec_bitmap(&nsec->rrs, &bm, &bm_size); +int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t type) +{ + const int NO_PROOF = abs(ENOENT); if (!bm) { return kr_error(EINVAL); } + if (kr_nsec_bitmap_contains_type(bm, bm_size, type)) { + return NO_PROOF; + } - if (!kr_nsec_bitmap_contains_type(bm, bm_size, type)) { - /* The type is not listed in the NSEC bitmap. */ + if (type != KNOT_RRTYPE_CNAME + && kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_CNAME)) { + return NO_PROOF; + } + /* Special behavior around zone cuts. */ + switch (type) { + case KNOT_RRTYPE_DS: /* Security feature: in case of DS also check for SOA * non-existence to be more certain that we don't hold * a child-side NSEC by some mistake (e.g. when forwarding). * See RFC4035 5.2, next-to-last paragraph. */ - if (type != KNOT_RRTYPE_DS - || !kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)) { - *flags |= FLG_NOEXIST_RRTYPE; + if (kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)) { + return NO_PROOF; + } + break; + case KNOT_RRTYPE_CNAME: + /* Exception from the `default` rule. It's perhaps disputable, + * but existence of CNAME at zone apex is not allowed, so we + * consider a parent-side record to be enough to prove non-existence. */ + break; + default: + /* Parent-side delegation record isn't authoritative for non-DS; + * see RFC6840 4.1. */ + if (kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_NS) + && !kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_SOA)) { + return NO_PROOF; } + /* LATER(opt): perhaps short-circuit test if we repeat it here. */ } 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) +{ + assert(flags && nsec); + + uint8_t *bm = NULL; + uint16_t bm_size = 0; + knot_nsec_bitmap(&nsec->rrs, &bm, &bm_size); + int ret = kr_nsec_bitmap_nodata_check(bm, bm_size, type); + 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. @@ -473,11 +507,15 @@ int kr_nsec_ref_to_unsigned(const knot_pkt_t *pkt) int kr_nsec_matches_name_and_type(const knot_rrset_t *nsec, const knot_dname_t *name, uint16_t type) { - if (!nsec || !name) { - return (EINVAL); + /* It's not secure enough to just check a single bit for (some) other types, + * but we don't (currently) only use this API for NS. See RFC 6840 sec. 4. + */ + if (type != KNOT_RRTYPE_NS || !nsec || !name) { + assert(!EINVAL); + return kr_error(EINVAL); } if (!knot_dname_is_equal(nsec->owner, name)) { - return (ENOENT); + return kr_error(ENOENT); } uint8_t *bm = NULL; uint16_t bm_size = 0; @@ -485,8 +523,9 @@ int kr_nsec_matches_name_and_type(const knot_rrset_t *nsec, if (!bm) { return kr_error(EINVAL); } - if (!kr_nsec_bitmap_contains_type(bm, bm_size, type)) { - return (ENOENT); + if (kr_nsec_bitmap_contains_type(bm, bm_size, type)) { + return kr_ok(); + } else { + return kr_error(ENOENT); } - return kr_ok(); } diff --git a/lib/dnssec/nsec.h b/lib/dnssec/nsec.h index 0c4f23d0f..c86a9a980 100644 --- a/lib/dnssec/nsec.h +++ b/lib/dnssec/nsec.h @@ -27,6 +27,16 @@ */ bool kr_nsec_bitmap_contains_type(const uint8_t *bm, uint16_t bm_size, uint16_t type); +/** + * Check an NSEC or NSEC3 bitmap for NODATA for a type. + * @param bm Bitmap. + * @param bm_size Bitmap size. + * @param type RR type to check. + * @note This includes special checks for zone cuts, e.g. from RFC 6840 sec. 4. + * @return 0, abs(ENOENT) (no proof), kr_error(EINVAL) + */ +int kr_nsec_bitmap_nodata_check(const uint8_t *bm, uint16_t bm_size, uint16_t type); + /** * Name error response check (RFC4035 3.1.3.2; RFC4035 5.4, bullet 2). * @note No RRSIGs are validated. @@ -87,7 +97,7 @@ int kr_nsec_ref_to_unsigned(const knot_pkt_t *pkt); * Checks whether supplied NSEC RR matches the supplied name and type. * @param nsec NSEC RR. * @param name Name to be checked. - * @param type Type to be checked. + * @param type Type to be checked. Only use with NS! TODO (+copy&paste NSEC3) * @return 0 or error code. */ int kr_nsec_matches_name_and_type(const knot_rrset_t *nsec, diff --git a/lib/dnssec/nsec3.c b/lib/dnssec/nsec3.c index 14029297b..88f6f119b 100644 --- a/lib/dnssec/nsec3.c +++ b/lib/dnssec/nsec3.c @@ -299,11 +299,11 @@ static bool has_optout(const knot_rrset_t *nsec3) * @param flags Flags to be set according to check outcome. * @param nsec3 NSEC3 RR. * @param name Name to be checked. - * @return 0 or error code. + * @return 0 if matching, >0 if not (abs(ENOENT)), or error code (<0). */ -static int matches_name(int *flags, const knot_rrset_t *nsec3, const knot_dname_t *name) +static int matches_name(const knot_rrset_t *nsec3, const knot_dname_t *name) { - assert(flags && nsec3 && name); + assert(nsec3 && name); dnssec_binary_t owner_hash = {0, }; uint8_t hash_data[MAX_HASH_BYTES] = {0, }; @@ -328,8 +328,9 @@ static int matches_name(int *flags, const knot_rrset_t *nsec3, const knot_dname_ if ((owner_hash.size == name_hash.size) && (memcmp(owner_hash.data, name_hash.data, owner_hash.size) == 0)) { - *flags |= FLG_NAME_MATCHED; ret = kr_ok(); + } else { + ret = abs(ENOENT); } fail: @@ -512,81 +513,36 @@ int kr_nsec3_name_error_response_check(const knot_pkt_t *pkt, knot_section_t sec } /** - * Checks whether supplied NSEC3 RR matches the supplied name and type. - * @param flags Flags to be set according to check outcome. - * @param nsec3 NSEC3 RR. - * @param name Name to be checked. - * @param type Type to be checked. - * @return 0 or error code. - */ -static int matches_name_and_type(int *flags, const knot_rrset_t *nsec3, - const knot_dname_t *name, uint16_t type) -{ - assert(flags && nsec3 && name); - - int ret = matches_name(flags, nsec3, name); - if (ret != 0) { - return ret; - } - - if (!(*flags & FLG_NAME_MATCHED)) { - return kr_ok(); - } - - uint8_t *bm = NULL; - uint16_t bm_size; - knot_nsec3_bitmap(&nsec3->rrs, 0, &bm, &bm_size); - if (!bm) { - return kr_error(EINVAL); - } - - if (!kr_nsec_bitmap_contains_type(bm, bm_size, type)) { - *flags |= FLG_TYPE_BIT_MISSING; - if (type == KNOT_RRTYPE_CNAME) { - *flags |= FLG_CNAME_BIT_MISSING; - } - } - - if ((type != KNOT_RRTYPE_CNAME) && - !kr_nsec_bitmap_contains_type(bm, bm_size, KNOT_RRTYPE_CNAME)) { - *flags |= FLG_CNAME_BIT_MISSING; - } - - return kr_ok(); -} - -/** - * No data response check, no DS (RFC5155 7.2.3). + * Search the packet section for a matching NSEC3 with nodata-proving bitmap. * @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. + * @note This does NOT check the opt-out case if type is DS; + * see RFC 5155 8.6. */ -static int no_data_response_no_ds(const knot_pkt_t *pkt, knot_section_t section_id, - const knot_dname_t *sname, uint16_t stype) +static int nodata_find(const knot_pkt_t *pkt, knot_section_t section_id, + const knot_dname_t *name, const uint16_t type) { const knot_pktsection_t *sec = knot_pkt_section(pkt, section_id); - if (!sec || !sname) { + if (!sec || !name) { return kr_error(EINVAL); } - int flags; for (unsigned i = 0; i < sec->count; ++i) { - const knot_rrset_t *rrset = knot_pkt_rr(sec, i); - if (rrset->type != KNOT_RRTYPE_NSEC3) { + const knot_rrset_t *nsec3 = knot_pkt_rr(sec, i); + /* Records causing any errors are simply skipped. */ + if (nsec3->type != KNOT_RRTYPE_NSEC3 + || matches_name(nsec3, name) != kr_ok()) { continue; + /* LATER(optim.): we repeatedly recompute the hash of `name` */ } - flags = 0; - int ret = matches_name_and_type(&flags, rrset, sname, stype); - if (ret != 0) { - return ret; - } - - if ((flags & FLG_NAME_MATCHED) && - (flags & FLG_TYPE_BIT_MISSING) && - (flags & FLG_CNAME_BIT_MISSING)) { + uint8_t *bm = NULL; + uint16_t bm_size; + knot_nsec3_bitmap(&nsec3->rrs, 0, &bm, &bm_size); + if (kr_nsec_bitmap_nodata_check(bm, bm_size, type) == kr_ok()) { return kr_ok(); } } @@ -610,38 +566,13 @@ static int matches_closest_encloser_wildcard(const knot_pkt_t *pkt, knot_section return kr_error(EINVAL); } - uint8_t wildcard[KNOT_DNAME_MAXLEN]; + uint8_t wildcard[KNOT_DNAME_MAXLEN]; /**< the source of synthesis */ int ret = prepend_asterisk(wildcard, sizeof(wildcard), encloser); if (ret < 0) { return ret; } assert(ret >= 3); - - int flags; - for (unsigned i = 0; i < sec->count; ++i) { - const knot_rrset_t *rrset = knot_pkt_rr(sec, i); - if (rrset->type != KNOT_RRTYPE_NSEC3) { - continue; - } - flags = 0; - - int ret = matches_name_and_type(&flags, rrset, wildcard, stype); - if (ret != 0) { - return ret; - } - - /* TODO -- The loop resembles no_data_response_no_ds() exept - * the following condition. - */ - if ((flags & FLG_NAME_MATCHED) && - (flags & FLG_TYPE_BIT_MISSING) && - (flags & FLG_CNAME_BIT_MISSING)) { - /* rfc5155 8.7 */ - return kr_ok(); - } - } - - return kr_error(ENOENT); + return nodata_find(pkt, section_id, wildcard, stype); } int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t section_id, @@ -682,7 +613,7 @@ int kr_nsec3_no_data(const knot_pkt_t *pkt, knot_section_t section_id, const knot_dname_t *sname, uint16_t stype) { /* DS record may be also matched by an existing NSEC3 RR. */ - int ret = no_data_response_no_ds(pkt, section_id, sname, stype); + int ret = nodata_find(pkt, section_id, sname, stype); if (ret == 0) { /* Satisfies RFC5155 8.5 and 8.6, both first paragraph. */ return ret; @@ -760,17 +691,9 @@ int kr_nsec3_ref_to_unsigned(const knot_pkt_t *pkt) continue; } nsec3_found = true; - /* nsec3 found, check if owner name matches - * the delegation name - */ - ret = matches_name(&flags, nsec3, ns->owner); - if (ret != 0) { - return kr_error(EINVAL); - } - if (!(flags & FLG_NAME_MATCHED)) { - /* nsec3 owner name does not match - * the delegation name - */ + /* nsec3 found, check if owner name matches the delegation name. + * Just skip in case of *any* errors. */ + if (matches_name(nsec3, ns->owner) != kr_ok()) { continue; } knot_nsec3_bitmap(&nsec3->rrs, 0, &bm, &bm_size); @@ -821,11 +744,26 @@ int kr_nsec3_ref_to_unsigned(const knot_pkt_t *pkt) int kr_nsec3_matches_name_and_type(const knot_rrset_t *nsec3, const knot_dname_t *name, uint16_t type) { - int flags = 0; - int ret = matches_name_and_type(&flags, nsec3, name, type); - if (ret != kr_ok()) { - return ret; + /* It's not secure enough to just check a single bit for (some) other types, + * but we don't (currently) only use this API for NS. See RFC 6840 sec. 4. + */ + if (type != KNOT_RRTYPE_NS) { + assert(!EINVAL); + return kr_error(EINVAL); + } + int ret = matches_name(nsec3, name); + if (ret) { + return kr_error(ret); + } + uint8_t *bm = NULL; + uint16_t bm_size = 0; + knot_nsec3_bitmap(&nsec3->rrs, 0, &bm, &bm_size); + if (!bm) { + return kr_error(EINVAL); + } + if (kr_nsec_bitmap_contains_type(bm, bm_size, type)) { + return kr_ok(); + } else { + return kr_error(ENOENT); } - return ((flags & (FLG_NAME_MATCHED | FLG_TYPE_BIT_MISSING)) != FLG_NAME_MATCHED) ? - kr_error(ENOENT) : kr_ok(); } diff --git a/lib/dnssec/nsec3.h b/lib/dnssec/nsec3.h index b07af7e53..33a396a46 100644 --- a/lib/dnssec/nsec3.h +++ b/lib/dnssec/nsec3.h @@ -72,10 +72,10 @@ int kr_nsec3_no_data(const knot_pkt_t *pkt, knot_section_t section_id, int kr_nsec3_ref_to_unsigned(const knot_pkt_t *pkt); /** - * Checks whether supplied NSEC3 RR matches the supplied name and type. + * Checks whether supplied NSEC3 RR matches the supplied name and NS type. * @param nsec3 NSEC3 RR. * @param name Name to be checked. - * @param type Type to be checked. + * @param type Type to be checked. Only use with NS! TODO * @return 0 or error code. */ int kr_nsec3_matches_name_and_type(const knot_rrset_t *nsec3,