diff options
Diffstat (limited to 'debian/patches/0002-validator-similarly-also-limit-excessive-NSEC3-salt-.patch')
| -rw-r--r-- | debian/patches/0002-validator-similarly-also-limit-excessive-NSEC3-salt-.patch | 143 |
1 files changed, 143 insertions, 0 deletions
diff --git a/debian/patches/0002-validator-similarly-also-limit-excessive-NSEC3-salt-.patch b/debian/patches/0002-validator-similarly-also-limit-excessive-NSEC3-salt-.patch new file mode 100644 index 0000000..2225c90 --- /dev/null +++ b/debian/patches/0002-validator-similarly-also-limit-excessive-NSEC3-salt-.patch @@ -0,0 +1,143 @@ +From: =?utf-8?b?VmxhZGltw61yIMSMdW7DoXQ=?= <vladimir.cunat@nic.cz> +Date: Tue, 2 Jan 2024 11:18:31 +0100 +Subject: validator: similarly also limit excessive NSEC3 salt length + +Limit combination of iterations and salt length, based on estimated +expense of the computation. Note that the result only differs for +salt length > 44 which is rather nonsensical and very rare: +https://chat.dns-oarc.net/community/pl/h58qx9sjkbgt9dajb7x988p78a +--- + lib/cache/api.c | 2 +- + lib/cache/nsec3.c | 2 +- + lib/dnssec/nsec3.c | 4 ++-- + lib/dnssec/nsec3.h | 32 ++++++++++++++++++++++++++++---- + lib/layer/validate.c | 7 ++++--- + 5 files changed, 36 insertions(+), 11 deletions(-) + +diff --git a/lib/cache/api.c b/lib/cache/api.c +index 116d775..bb627ea 100644 +--- a/lib/cache/api.c ++++ b/lib/cache/api.c +@@ -500,7 +500,7 @@ static ssize_t stash_rrset(struct kr_cache *cache, const struct kr_query *qry, + return kr_ok(); + } + if (rr->type == KNOT_RRTYPE_NSEC3 && rr->rrs.count +- && knot_nsec3_iters(rr->rrs.rdata) > KR_NSEC3_MAX_ITERATIONS) { ++ && kr_nsec3_limited_rdata(rr->rrs.rdata)) { + /* This shouldn't happen often, thanks to downgrades during validation. */ + VERBOSE_MSG(qry, "=> skipping NSEC3 with too many iterations\n"); + return kr_ok(); +diff --git a/lib/cache/nsec3.c b/lib/cache/nsec3.c +index 0b70775..9832630 100644 +--- a/lib/cache/nsec3.c ++++ b/lib/cache/nsec3.c +@@ -84,7 +84,7 @@ static knot_db_val_t key_NSEC3_name(struct key *k, const knot_dname_t *name, + .data = (uint8_t *)/*const-cast*/name, + }; + +- if (kr_fails_assert(nsec_p->libknot.iterations <= KR_NSEC3_MAX_ITERATIONS)) { ++ if (kr_fails_assert(!kr_nsec3_limited_params(&nsec_p->libknot))) { + /* This is mainly defensive; it shouldn't happen thanks to downgrades. */ + return VAL_EMPTY; + } +diff --git a/lib/dnssec/nsec3.c b/lib/dnssec/nsec3.c +index 037d5bd..e4d314b 100644 +--- a/lib/dnssec/nsec3.c ++++ b/lib/dnssec/nsec3.c +@@ -71,7 +71,7 @@ static int hash_name(dnssec_binary_t *hash, const dnssec_nsec3_params_t *params, + return kr_error(EINVAL); + if (!name) + return kr_error(EINVAL); +- if (kr_fails_assert(params->iterations <= KR_NSEC3_MAX_ITERATIONS)) { ++ if (kr_fails_assert(!kr_nsec3_limited_params(params))) { + /* This if is mainly defensive; it shouldn't happen. */ + return kr_error(EINVAL); + } +@@ -565,7 +565,7 @@ int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_ + const knot_rrset_t *rrset = knot_pkt_rr(sec, i); + if (rrset->type != KNOT_RRTYPE_NSEC3) + continue; +- if (knot_nsec3_iters(rrset->rrs.rdata) > KR_NSEC3_MAX_ITERATIONS) { ++ if (kr_nsec3_limited_rdata(rrset->rrs.rdata)) { + /* Avoid hashing with too many iterations. + * If we get here, the `sname` wildcard probably ends up bogus, + * but it gets downgraded to KR_RANK_INSECURE when validator +diff --git a/lib/dnssec/nsec3.h b/lib/dnssec/nsec3.h +index 723dc4a..76ef2e9 100644 +--- a/lib/dnssec/nsec3.h ++++ b/lib/dnssec/nsec3.h +@@ -5,15 +5,39 @@ + #pragma once + + #include <libknot/packet/pkt.h> ++#include <libknot/rrtype/nsec3.h> ++#include <libdnssec/nsec.h> ++ ++ ++static inline unsigned int kr_nsec3_price(unsigned int iterations, unsigned int salt_len) ++{ ++ // SHA1 works on 64-byte chunks. ++ // On iterating we hash the salt + 20 bytes of the previous hash. ++ int chunks_per_iter = (20 + salt_len - 1) / 64 + 1; ++ return (iterations + 1) * chunks_per_iter; ++} + + /** High numbers in NSEC3 iterations don't really help security + * +- * ...so we avoid doing all the work. The value is a current compromise; +- * zones shooting over get downgraded to insecure status. ++ * ...so we avoid doing all the work. The limit is a current compromise; ++ * answers using NSEC3 over kr_nsec3_limited* get downgraded to insecure status. + * + https://datatracker.ietf.org/doc/html/rfc9276#name-recommendation-for-validati + */ +-#define KR_NSEC3_MAX_ITERATIONS 50 ++static inline bool kr_nsec3_limited(unsigned int iterations, unsigned int salt_len) ++{ ++ const int MAX_ITERATIONS = 50; // limit with short salt length ++ return kr_nsec3_price(iterations, salt_len) > MAX_ITERATIONS + 1; ++} ++static inline bool kr_nsec3_limited_rdata(const knot_rdata_t *rd) ++{ ++ return kr_nsec3_limited(knot_nsec3_iters(rd), knot_nsec3_salt_len(rd)); ++} ++static inline bool kr_nsec3_limited_params(const dnssec_nsec3_params_t *params) ++{ ++ return kr_nsec3_limited(params->iterations, params->salt.size); ++} ++ + + /** + * Name error response check (RFC5155 7.2.2). +@@ -36,7 +60,7 @@ int kr_nsec3_name_error_response_check(const knot_pkt_t *pkt, knot_section_t sec + * KNOT_ERANGE - NSEC3 RR that covers a wildcard + * has been found, but has opt-out flag set; + * otherwise - error. +- * Records over KR_NSEC3_MAX_ITERATIONS are skipped, so you probably get kr_error(ENOENT). ++ * Too expensive NSEC3 records are skipped, so you probably get kr_error(ENOENT). + */ + int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_t section_id, + const knot_dname_t *sname, int trim_to_next); +diff --git a/lib/layer/validate.c b/lib/layer/validate.c +index 93f1d4f..5fea99d 100644 +--- a/lib/layer/validate.c ++++ b/lib/layer/validate.c +@@ -128,14 +128,15 @@ static bool maybe_downgrade_nsec3(const ranked_rr_array_entry_t *e, struct kr_qu + const knot_rdataset_t *rrs = &e->rr->rrs; + knot_rdata_t *rd = rrs->rdata; + for (int j = 0; j < rrs->count; ++j, rd = knot_rdataset_next(rd)) { +- if (knot_nsec3_iters(rd) > KR_NSEC3_MAX_ITERATIONS) ++ if (kr_nsec3_limited_rdata(rd)) + goto do_downgrade; + } + return false; + + do_downgrade: // we do this deep inside calls because of having signer name available +- VERBOSE_MSG(qry, "<= DNSSEC downgraded due to NSEC3 iterations %d > %d\n", +- (int)knot_nsec3_iters(rd), (int)KR_NSEC3_MAX_ITERATIONS); ++ VERBOSE_MSG(qry, ++ "<= DNSSEC downgraded due to expensive NSEC3: %d iterations, %d salt length\n", ++ (int)knot_nsec3_iters(rd), (int)knot_nsec3_salt_len(rd)); + qry->flags.DNSSEC_WANT = false; + qry->flags.DNSSEC_INSECURE = true; + rank_records(qry, true, KR_RANK_INSECURE, vctx->zone_name); |