1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
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);
|
