From 40595f80d978d99324e0ba901b62e1f949b53e03 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sat, 30 Dec 2023 21:01:05 +0000 Subject: [PATCH 01/13] Protection against pathalogical DNSSEC domains. An attacker can create DNSSEC signed domains which need a lot of work to verfify. We limit the number of crypto operations to avoid DoS attacks by CPU exhaustion. --- src/dnssec.c | 95 ++++++++++++++++++++++++++++++++++++--------------- src/forward.c | 31 +++++++++++++---- 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index 29a8e7a..e02dc5d 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -430,6 +430,7 @@ static int explore_rrset(struct dns_header *header, size_t plen, int class, int STAT_SECURE_WILDCARD if it validates and is the result of wildcard expansion. (In this case *wildcard_out points to the "body" of the wildcard within name.) STAT_BOGUS signature is wrong, bad packet. + STAT_ABANDONED validation abandoned do to excess resource usage. STAT_NEED_KEY need DNSKEY to complete validation (name is returned in keyname) STAT_NEED_DS need DS to complete validation (name is returned in keyname) @@ -447,7 +448,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in int algo_in, int keytag_in, unsigned long *ttl_out) { unsigned char *p; - int rdlen, j, name_labels, algo, labels, key_tag; + int rdlen, j, name_labels, algo, labels, key_tag, sig_fail_cnt; struct crec *crecp = NULL; short *rr_desc = rrfilter_desc(type); u32 sig_expiration, sig_inception; @@ -467,7 +468,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in rrsetidx = sort_rrset(header, plen, rr_desc, rrsetidx, rrset, daemon->workspacename, keyname); /* Now try all the sigs to try and find one which validates */ - for (j = 0; j addr.key.algo == algo && crecp->addr.key.keytag == key_tag && - crecp->uid == (unsigned int)class && - verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) - return (labels < name_labels) ? STAT_SECURE_WILDCARD : STAT_SECURE; + crecp->uid == (unsigned int)class) + { + if (verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) + return (labels < name_labels) ? STAT_SECURE_WILDCARD : STAT_SECURE; + + /* An attacker can waste a lot of our CPU by setting up a giant DNSKEY RRSET full of failing + keys, all of which we have to try. Since many failing keys is not likely for + a legitimate domain, set a limit on how many can fail. */ + sig_fail_cnt++; + + if (sig_fail_cnt > 10) /* TODO */ + { + my_syslog(LOG_ERR, "sig_fail_cnt"); + return STAT_ABANDONED; + } + } } } @@ -681,6 +695,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in STAT_OK Done, key(s) in cache. STAT_BOGUS No DNSKEYs found, which can be validated with DS, or self-sign for DNSKEY RRset is not valid, bad packet. + STAT_ABANDONED resource exhaustion. STAT_NEED_DS DS records to validate a key not found, name in keyname STAT_NEED_KEY DNSKEY records to validate a key not found, name in keyname */ @@ -688,7 +703,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch { unsigned char *psave, *p = (unsigned char *)(header+1); struct crec *crecp, *recp1; - int rc, j, qtype, qclass, rdlen, flags, algo, valid, keytag; + int rc, j, qtype, qclass, rdlen, flags, algo, valid, keytag, ds_fail_cnt, key_fail_cnt; unsigned long ttl, sig_ttl; struct blockdata *key; union all_addr a; @@ -713,7 +728,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch } /* NOTE, we need to find ONE DNSKEY which matches the DS */ - for (valid = 0, j = ntohs(header->ancount); j != 0 && !valid; j--) + for (key_fail_cnt = 0, valid = 0, j = ntohs(header->ancount); j != 0 && !valid; j--) { /* Ensure we have type, class TTL and length */ if (!(rc = extract_name(header, plen, &p, name, 0, 10))) @@ -762,7 +777,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch if (!key) continue; - for (recp1 = crecp; recp1; recp1 = cache_find_by_name(recp1, name, now, F_DS)) + for (ds_fail_cnt = 0, recp1 = crecp; recp1; recp1 = cache_find_by_name(recp1, name, now, F_DS)) { void *ctx; unsigned char *digest, *ds_digest; @@ -771,7 +786,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch int wire_len; if (recp1->addr.ds.algo == algo && - recp1->addr.ds.keytag == keytag && + recp1->addr.ds.keytag == keytag && recp1->uid == (unsigned int)class) { failflags &= ~DNSSEC_FAIL_NOKEY; @@ -796,30 +811,54 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch if (!(recp1->flags & F_NEG) && recp1->addr.ds.keylen == (int)hash->digest_size && - (ds_digest = blockdata_retrieve(recp1->addr.ds.keydata, recp1->addr.ds.keylen, NULL)) && - memcmp(ds_digest, digest, recp1->addr.ds.keylen) == 0 && - explore_rrset(header, plen, class, T_DNSKEY, name, keyname, &sigcnt, &rrcnt) && - rrcnt != 0) + (ds_digest = blockdata_retrieve(recp1->addr.ds.keydata, recp1->addr.ds.keylen, NULL))) { - if (sigcnt == 0) - continue; - else - failflags &= ~DNSSEC_FAIL_NOSIG; - - rc = validate_rrset(now, header, plen, class, T_DNSKEY, sigcnt, rrcnt, name, keyname, - NULL, key, rdlen - 4, algo, keytag, &sig_ttl); - - failflags &= rc; - - if (STAT_ISEQUAL(rc, STAT_SECURE)) + if (memcmp(ds_digest, digest, recp1->addr.ds.keylen) != 0) { - valid = 1; - break; + /* limit CPU exhaustion attack from large DS x KEY cross-product. */ + ds_fail_cnt++; + + if (ds_fail_cnt > 5) /* TODO */ + { + my_syslog(LOG_ERR, "ds_fail_cnt"); + return STAT_ABANDONED; + } + } + else if (explore_rrset(header, plen, class, T_DNSKEY, name, keyname, &sigcnt, &rrcnt) && + rrcnt != 0) + { + if (sigcnt == 0) + continue; + else + failflags &= ~DNSSEC_FAIL_NOSIG; + + rc = validate_rrset(now, header, plen, class, T_DNSKEY, sigcnt, rrcnt, name, keyname, + NULL, key, rdlen - 4, algo, keytag, &sig_ttl); + + if (STAT_ISEQUAL(rc, STAT_ABANDONED)) + return STAT_ABANDONED; + + failflags &= rc; + + if (STAT_ISEQUAL(rc, STAT_SECURE)) + { + valid = 1; + break; + } } } } } blockdata_free(key); + + /* limit CPU exhaustion attack from large DS x KEY cross-product. */ + key_fail_cnt++; + + if (key_fail_cnt > 15) /* TODO */ + { + my_syslog(LOG_ERR, "key_fail_cnt"); + return STAT_ABANDONED; + } } if (valid) @@ -916,6 +955,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch STAT_BOGUS no DS in reply or not signed, fails validation, bad packet. STAT_NEED_KEY DNSKEY records to validate a DS not found, name in keyname STAT_NEED_DS DS record needed. + STAT_ABANDONED resource exhaustion. */ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class) @@ -1798,6 +1838,7 @@ static int zone_status(char *name, int class, char *keyname, time_t now) STAT_BOGUS signature is wrong, bad packet, no validation where there should be. STAT_NEED_KEY need DNSKEY to complete validation (name is returned in keyname, class in *class) STAT_NEED_DS need DS to complete validation (name is returned in keyname) + STAT_ABANDONED resource exhaustion. daemon->rr_status points to a char array which corressponds to the RRs in the answer and auth sections. This is set to >1 for each RR which is validated, and 0 for any which aren't. @@ -1984,7 +2025,7 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch rc = validate_rrset(now, header, plen, class1, type1, sigcnt, rrcnt, name, keyname, &wildname, NULL, 0, 0, 0, &sig_ttl); - if (STAT_ISEQUAL(rc, STAT_BOGUS) || STAT_ISEQUAL(rc, STAT_NEED_KEY) || STAT_ISEQUAL(rc, STAT_NEED_DS)) + if (STAT_ISEQUAL(rc, STAT_BOGUS) || STAT_ISEQUAL(rc, STAT_NEED_KEY) || STAT_ISEQUAL(rc, STAT_NEED_DS) || STAT_ISEQUAL(rc, STAT_ABANDONED)) { if (class) *class = class1; /* Class for DS or DNSKEY */ diff --git a/src/forward.c b/src/forward.c index d7f9eb6..f8769bc 100644 --- a/src/forward.c +++ b/src/forward.c @@ -930,11 +930,15 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, status = dnssec_validate_reply(now, header, plen, daemon->namebuff, daemon->keyname, &forward->class, !option_bool(OPT_DNSSEC_IGN_NS) && (forward->sentto->flags & SERV_DO_DNSSEC), NULL, NULL, NULL); -#ifdef HAVE_DUMPFILE - if (STAT_ISEQUAL(status, STAT_BOGUS)) - dump_packet_udp((forward->flags & (FREC_DNSKEY_QUERY | FREC_DS_QUERY)) ? DUMP_SEC_BOGUS : DUMP_BOGUS, - header, (size_t)plen, &forward->sentto->addr, NULL, -daemon->port); -#endif + + if (STAT_ISEQUAL(status, STAT_ABANDONED)) + { + /* Log the actual validation that made us barf. */ + unsigned char *p = (unsigned char *)(header+1); + if (extract_name(header, plen, &p, daemon->namebuff, 0, 4) == 1) + my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), + daemon->namebuff[0] ? daemon->namebuff : "."); + } } /* Can't validate, as we're missing key data. Put this @@ -1065,6 +1069,12 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, status = STAT_ABANDONED; } +#ifdef HAVE_DUMPFILE + if (STAT_ISEQUAL(status, STAT_BOGUS) || STAT_ISEQUAL(status, STAT_ABANDONED)) + dump_packet_udp((forward->flags & (FREC_DNSKEY_QUERY | FREC_DS_QUERY)) ? DUMP_SEC_BOGUS : DUMP_BOGUS, + header, (size_t)plen, &forward->sentto->addr, NULL, -daemon->port); +#endif + /* Validated original answer, all done. */ if (!forward->dependent) return_reply(now, forward, header, plen, status); @@ -1073,7 +1083,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, /* validated subsidiary query/queries, (and cached result) pop that and return to the previous query/queries we were working on. */ struct frec *prev, *nxt = forward->dependent; - + free_frec(forward); while ((prev = nxt)) @@ -2041,6 +2051,15 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si !option_bool(OPT_DNSSEC_IGN_NS) && (server->flags & SERV_DO_DNSSEC), NULL, NULL, NULL); + if (STAT_ISEQUAL(new_status, STAT_ABANDONED)) + { + /* Log the actual validation that made us barf. */ + unsigned char *p = (unsigned char *)(header+1); + if (extract_name(header, n, &p, daemon->namebuff, 0, 4) == 1) + my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), + daemon->namebuff[0] ? daemon->namebuff : "."); + } + if (!STAT_ISEQUAL(new_status, STAT_NEED_DS) && !STAT_ISEQUAL(new_status, STAT_NEED_KEY)) break; From be73efc02064b18b83f5c0149434c1999a8127b3 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 31 Dec 2023 15:11:54 +0000 Subject: [PATCH 02/13] Update header with new EDE values. --- src/dns-protocol.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/dns-protocol.h b/src/dns-protocol.h index 0671adf..2777be9 100644 --- a/src/dns-protocol.h +++ b/src/dns-protocol.h @@ -112,8 +112,11 @@ #define EDE_NO_AUTH 22 /* No Reachable Authority */ #define EDE_NETERR 23 /* Network error */ #define EDE_INVALID_DATA 24 /* Invalid Data */ - - +#define EDE_SIG_E_B_V 25 /* Signature Expired before Valid */ +#define EDE_TOO_EARLY 26 /* To Early */ +#define EDE_UNS_NS3_ITER 27 /* Unsupported NSEC3 Iterations Value */ +#define EDE_UNABLE_POLICY 28 /* Unable to conform to policy */ +#define EDE_SYNTHESIZED 29 /* Synthesized */ struct dns_header { From 51471cafa5a4fa44d6fe490885d9910bd72a5907 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 31 Dec 2023 23:28:11 +0000 Subject: [PATCH 03/13] Update NSEC3 iterations handling to conform with RFC 9276. --- src/dnsmasq.h | 2 + src/dnssec.c | 109 ++++++++++++++++++++++++++------------------------ 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 61885c7..b4b2b7f 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -757,6 +757,8 @@ struct dyndir { #define DNSSEC_FAIL_NONSEC 0x0040 /* No NSEC */ #define DNSSEC_FAIL_NODSSUP 0x0080 /* no supported DS algo. */ #define DNSSEC_FAIL_NOKEY 0x0100 /* no DNSKEY */ +#define DNSSEC_FAIL_NSEC3_ITERS 0x0200 /* too many iterations in NSEC3 */ +#define DNSSEC_FAIL_BADPACKET 0x0400 /* bad packet */ #define STAT_ISEQUAL(a, b) (((a) & 0xffff0000) == (b)) diff --git a/src/dnssec.c b/src/dnssec.c index e02dc5d..ceb6a37 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -1179,6 +1179,7 @@ static int hostname_cmp(const char *a, const char *b) } } +/* returns 0 on success, or DNSSEC_FAIL_* value on failure. */ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsigned char **nsecs, unsigned char **labels, int nsec_count, char *workspace1_in, char *workspace2, char *name, int type, int *nons) { @@ -1203,7 +1204,7 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi GETSHORT(rdlen, p); psave = p; if (!extract_name(header, plen, &p, workspace2, 1, 10)) - return 0; + return DNSSEC_FAIL_BADPACKET; /* If NSEC comes from wildcard expansion, use original wildcard as name for computation. */ @@ -1231,7 +1232,7 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi { /* 4035 para 5.4. Last sentence */ if (type == T_NSEC || type == T_RRSIG) - return 1; + return 0; /* NSEC with the same name as the RR we're testing, check that the type in question doesn't appear in the type map */ @@ -1247,25 +1248,25 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi /* A CNAME answer would also be valid, so if there's a CNAME is should have been returned. */ if ((p[2] & (0x80 >> T_CNAME)) != 0) - return 0; + return DNSSEC_FAIL_NONSEC; /* If the SOA bit is set for a DS record, then we have the DS from the wrong side of the delegation. For the root DS, this is expected. */ if (name_labels != 0 && type == T_DS && (p[2] & (0x80 >> T_SOA)) != 0) - return 0; + return DNSSEC_FAIL_NONSEC; } while (rdlen >= 2) { if (!CHECK_LEN(header, p, plen, rdlen)) - return 0; + return DNSSEC_FAIL_BADPACKET; if (p[0] == type >> 8) { /* Does the NSEC say our type exists? */ if (offset < p[1] && (p[offset+2] & mask) != 0) - return 0; + return DNSSEC_FAIL_NONSEC; break; /* finished checking */ } @@ -1281,17 +1282,17 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi /* Normal case, name falls between NSEC name and next domain name, wrap around case, name falls between NSEC name (rc == -1) and end */ if (hostname_cmp(workspace2, name) >= 0 || hostname_cmp(workspace1, workspace2) >= 0) - return 1; + return 0; } else { /* wrap around case, name falls between start and next domain name */ if (hostname_cmp(workspace1, workspace2) >= 0 && hostname_cmp(workspace2, name) >=0 ) - return 1; + return 0; } } - return 0; + return DNSSEC_FAIL_NONSEC; } /* return digest length, or zero on error */ @@ -1464,6 +1465,7 @@ static int check_nsec3_coverage(struct dns_header *header, size_t plen, int dige return 0; } +/* returns 0 on success, or DNSSEC_FAIL_* value on failure. */ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, unsigned char **nsecs, int nsec_count, char *workspace1, char *workspace2, char *name, int type, char *wildname, int *nons) { @@ -1485,9 +1487,9 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns for (i = 0; i < nsec_count; i++) { if (!(p = skip_name(nsecs[i], header, plen, 15))) - return 0; /* bad packet */ + return DNSSEC_FAIL_BADPACKET; /* bad packet */ - p += 10; /* type, class, TTL, rdlen */ + p += 10; /* type, class, TTL, rdlen */ algo = *p++; if ((hash = hash_find(nsec3_digest_name(algo)))) @@ -1496,22 +1498,19 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns /* No usable NSEC3s */ if (i == nsec_count) - return 0; + return DNSSEC_FAIL_NONSEC; p++; /* flags */ GETSHORT (iterations, p); - /* Upper-bound iterations, to avoid DoS. - Strictly, there are lower bounds for small keys, but - since we don't have key size info here, at least limit - to the largest bound, for 4096-bit keys. RFC 5155 10.3 */ - if (iterations > 2500) - return 0; + /* Upper-bound iterations, to avoid DoS. RFC 9276 refers. */ + if (iterations > 150) + return DNSSEC_FAIL_NSEC3_ITERS; salt_len = *p++; salt = p; if (!CHECK_LEN(header, salt, plen, salt_len)) - return 0; /* bad packet */ + return DNSSEC_FAIL_BADPACKET; /* bad packet */ /* Now prune so we only have NSEC3 records with same iterations, salt and algo */ for (i = 0; i < nsec_count; i++) @@ -1543,7 +1542,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns continue; if (!CHECK_LEN(header, p, plen, salt_len)) - return 0; /* bad packet */ + return DNSSEC_FAIL_BADPACKET; /* bad packet */ if (memcmp(p, salt, salt_len) != 0) continue; @@ -1553,10 +1552,10 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns } if ((digest_len = hash_name(name, &digest, hash, salt, salt_len, iterations)) == 0) - return 0; + return DNSSEC_FAIL_NONSEC; if (check_nsec3_coverage(header, plen, digest_len, digest, type, workspace1, workspace2, nsecs, nsec_count, nons, count_labels(name))) - return 1; + return 0; /* Can't find an NSEC3 which covers the name directly, we need the "closest encloser NSEC3" or an answer inferred from a wildcard record. */ @@ -1572,14 +1571,16 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns break; if ((digest_len = hash_name(closest_encloser, &digest, hash, salt, salt_len, iterations)) == 0) - return 0; + return DNSSEC_FAIL_NONSEC; for (i = 0; i < nsec_count; i++) if ((p = nsecs[i])) { - if (!extract_name(header, plen, &p, workspace1, 1, 0) || - !(base32_len = base32_decode(workspace1, (unsigned char *)workspace2))) - return 0; + if (!extract_name(header, plen, &p, workspace1, 1, 0)) + return DNSSEC_FAIL_BADPACKET; + + if (!(base32_len = base32_decode(workspace1, (unsigned char *)workspace2))) + return DNSSEC_FAIL_NONSEC; if (digest_len == base32_len && memcmp(digest, workspace2, digest_len) == 0) @@ -1594,14 +1595,14 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns while ((closest_encloser = strchr(closest_encloser, '.'))); if (!closest_encloser || !next_closest) - return 0; + return DNSSEC_FAIL_NONSEC; /* Look for NSEC3 that proves the non-existence of the next-closest encloser */ if ((digest_len = hash_name(next_closest, &digest, hash, salt, salt_len, iterations)) == 0) - return 0; + return DNSSEC_FAIL_NONSEC; if (!check_nsec3_coverage(header, plen, digest_len, digest, type, workspace1, workspace2, nsecs, nsec_count, NULL, 1)) - return 0; + return DNSSEC_FAIL_NONSEC; /* Finally, check that there's no seat of wildcard synthesis */ if (!wildname) @@ -1613,15 +1614,16 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns *wildcard = '*'; if ((digest_len = hash_name(wildcard, &digest, hash, salt, salt_len, iterations)) == 0) - return 0; + return DNSSEC_FAIL_NONSEC; if (!check_nsec3_coverage(header, plen, digest_len, digest, type, workspace1, workspace2, nsecs, nsec_count, NULL, 1)) - return 0; + return DNSSEC_FAIL_NONSEC; } - return 1; + return 0; } +/* returns 0 on success, or DNSSEC_FAIL_* value on failure. */ static int prove_non_existence(struct dns_header *header, size_t plen, char *keyname, char *name, int qtype, int qclass, char *wildname, int *nons, int *nsec_ttl) { static unsigned char **nsecset = NULL, **rrsig_labels = NULL; @@ -1634,7 +1636,7 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key /* Move to NS section */ if (!p || !(p = skip_section(p, ntohs(header->ancount), header, plen))) - return 0; + return DNSSEC_FAIL_BADPACKET; auth_start = p; @@ -1643,7 +1645,7 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key unsigned char *pstart = p; if (!extract_name(header, plen, &p, daemon->workspacename, 1, 10)) - return 0; + return DNSSEC_FAIL_BADPACKET; GETSHORT(type, p); GETSHORT(class, p); @@ -1662,12 +1664,12 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key /* No mixed NSECing 'round here, thankyouverymuch */ if (type_found != 0 && type_found != type) - return 0; + return DNSSEC_FAIL_NONSEC; type_found = type; if (!expand_workspace(&nsecset, &nsecset_sz, nsecs_found)) - return 0; + return DNSSEC_FAIL_BADPACKET; if (type == T_NSEC) { @@ -1682,14 +1684,14 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key int res, j, rdlen1, type1, class1; if (!expand_workspace(&rrsig_labels, &rrsig_labels_sz, nsecs_found)) - return 0; + return DNSSEC_FAIL_BADPACKET; rrsig_labels[nsecs_found] = NULL; for (j = ntohs(header->nscount); j != 0; j--) { if (!(res = extract_name(header, plen, &p1, daemon->workspacename, 0, 10))) - return 0; + return DNSSEC_FAIL_BADPACKET; GETSHORT(type1, p1); GETSHORT(class1, p1); @@ -1697,7 +1699,7 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key GETSHORT(rdlen1, p1); if (!CHECK_LEN(header, p1, plen, rdlen1)) - return 0; + return DNSSEC_FAIL_BADPACKET; if (res == 1 && class1 == qclass && type1 == T_RRSIG) { @@ -1705,7 +1707,7 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key unsigned char *psav = p1; if (rdlen1 < 18) - return 0; /* bad packet */ + return DNSSEC_FAIL_BADPACKET; /* bad packet */ GETSHORT(type_covered, p1); @@ -1717,25 +1719,25 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key if (!rrsig_labels[nsecs_found]) rrsig_labels[nsecs_found] = p1; else if (*rrsig_labels[nsecs_found] != *p1) /* algo */ - return 0; + return DNSSEC_FAIL_NONSEC; } p1 = psav; } if (!ADD_RDLEN(header, p1, plen, rdlen1)) - return 0; + return DNSSEC_FAIL_BADPACKET; } /* Must have found at least one sig. */ if (!rrsig_labels[nsecs_found]) - return 0; + return DNSSEC_FAIL_NONSEC; } nsecset[nsecs_found++] = pstart; } if (!ADD_RDLEN(header, p, plen, rdlen)) - return 0; + return DNSSEC_FAIL_BADPACKET; } if (type_found == T_NSEC) @@ -1743,7 +1745,7 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key else if (type_found == T_NSEC3) return prove_non_existence_nsec3(header, plen, nsecset, nsecs_found, daemon->workspacename, keyname, name, qtype, wildname, nons); else - return 0; + return DNSSEC_FAIL_NONSEC; } /* Check signing status of name. @@ -1857,7 +1859,7 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch int type1, class1, rdlen1 = 0, type2, class2, rdlen2, qclass, qtype, targetidx; int i, j, rc = STAT_INSECURE; int secure = STAT_SECURE; - + int rc_nsec; /* extend rr_status if necessary */ if (daemon->rr_status_sz < ntohs(header->ancount) + ntohs(header->nscount)) { @@ -2059,8 +2061,8 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch That's not a problem since if the RRsets later fail we'll return BOGUS then. */ if (STAT_ISEQUAL(rc, STAT_SECURE_WILDCARD) && - !prove_non_existence(header, plen, keyname, name, type1, class1, wildname, NULL, NULL)) - return STAT_BOGUS | DNSSEC_FAIL_NONSEC; + ((rc_nsec = prove_non_existence(header, plen, keyname, name, type1, class1, wildname, NULL, NULL))) != 0) + return STAT_BOGUS | rc_nsec; rc = STAT_SECURE; } @@ -2085,20 +2087,21 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch /* For anything other than a DS record, this situation is OK if either the answer is in an unsigned zone, or there's a NSEC records. */ - if (!prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons, nsec_ttl)) + if ((rc_nsec = prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons, nsec_ttl)) != 0) { /* Empty DS without NSECS */ if (qtype == T_DS) - return STAT_BOGUS | DNSSEC_FAIL_NONSEC; + return STAT_BOGUS | rc_nsec; - if (!STAT_ISEQUAL((rc = zone_status(name, qclass, keyname, now)), STAT_SECURE)) + if ((rc_nsec & (DNSSEC_FAIL_NONSEC | DNSSEC_FAIL_NSEC3_ITERS)) && + !STAT_ISEQUAL((rc = zone_status(name, qclass, keyname, now)), STAT_SECURE)) { if (class) *class = qclass; /* Class for NEED_DS or NEED_KEY */ return rc; } - return STAT_BOGUS | DNSSEC_FAIL_NONSEC; /* signed zone, no NSECs */ + return STAT_BOGUS | rc_nsec; /* signed zone, no NSECs */ } } @@ -2180,6 +2183,8 @@ int errflags_to_ede(int status) return EDE_NO_DNSKEY; else if (status & DNSSEC_FAIL_NODSSUP) return EDE_USUPDS; + else if (status & DNSSEC_FAIL_NSEC3_ITERS) + return EDE_UNS_NS3_ITER; else if (status & DNSSEC_FAIL_NONSEC) return EDE_NO_NSEC; else if (status & DNSSEC_FAIL_INDET) From 59d30390c9e5dcb4bc6e1048cb9efe17c3875ac0 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Mon, 1 Jan 2024 17:17:25 +0000 Subject: [PATCH 04/13] Measure cryptographic work done by DNSSEC. --- src/dnsmasq.h | 10 ++++++---- src/dnssec.c | 44 ++++++++++++++++++++++++++++---------------- src/forward.c | 42 +++++++++++++++++++++++++++++------------- 3 files changed, 63 insertions(+), 33 deletions(-) diff --git a/src/dnsmasq.h b/src/dnsmasq.h index b4b2b7f..ce35fc4 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -796,7 +796,7 @@ struct frec { struct blockdata *stash; /* Saved reply, whilst we validate */ size_t stash_len; #ifdef HAVE_DNSSEC - int class, work_counter; + int class, work_counter, validate_counter; struct frec *dependent; /* Query awaiting internally-generated DNSKEY or DS query */ struct frec *next_dependent; /* list of above. */ struct frec *blocking_query; /* Query which is blocking us. */ @@ -1417,10 +1417,12 @@ int in_zone(struct auth_zone *zone, char *name, char **cut); /* dnssec.c */ #ifdef HAVE_DNSSEC size_t dnssec_generate_query(struct dns_header *header, unsigned char *end, char *name, int class, int type, int edns_pktsz); -int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class); -int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class); +int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, char *name, + char *keyname, int class, int *validate_count); +int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, + char *keyname, int class, int *validate_count); int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int *class, - int check_unsigned, int *neganswer, int *nons, int *nsec_ttl); + int check_unsigned, int *neganswer, int *nons, int *nsec_ttl, int *validate_count); int dnskey_keytag(int alg, int flags, unsigned char *key, int keylen); size_t filter_rrsigs(struct dns_header *header, size_t plen); int setup_timestamp(void); diff --git a/src/dnssec.c b/src/dnssec.c index ceb6a37..cb35175 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -445,7 +445,7 @@ static int explore_rrset(struct dns_header *header, size_t plen, int class, int */ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, int class, int type, int sigidx, int rrsetidx, char *name, char *keyname, char **wildcard_out, struct blockdata *key, int keylen, - int algo_in, int keytag_in, unsigned long *ttl_out) + int algo_in, int keytag_in, unsigned long *ttl_out, int *validate_counter) { unsigned char *p; int rdlen, j, name_labels, algo, labels, key_tag, sig_fail_cnt; @@ -655,8 +655,10 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in if (key) { - if (algo_in == algo && keytag_in == key_tag && - verify(key, keylen, sig, sig_len, digest, hash->digest_size, algo)) + if (algo_in == algo && keytag_in == key_tag) + (*validate_counter)++; + + if (verify(key, keylen, sig, sig_len, digest, hash->digest_size, algo)) return STAT_SECURE; } else @@ -667,7 +669,9 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in crecp->addr.key.keytag == key_tag && crecp->uid == (unsigned int)class) { - if (verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) + (*validate_counter)++; + + if (verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) return (labels < name_labels) ? STAT_SECURE_WILDCARD : STAT_SECURE; /* An attacker can waste a lot of our CPU by setting up a giant DNSKEY RRSET full of failing @@ -699,7 +703,8 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in STAT_NEED_DS DS records to validate a key not found, name in keyname STAT_NEED_KEY DNSKEY records to validate a key not found, name in keyname */ -int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class) +int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, char *name, + char *keyname, int class, int *validate_counter) { unsigned char *psave, *p = (unsigned char *)(header+1); struct crec *crecp, *recp1; @@ -806,6 +811,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch hash->update(ctx, (unsigned int)wire_len, (unsigned char *)name); hash->update(ctx, (unsigned int)rdlen, psave); hash->digest(ctx, hash->digest_size, digest); + (*validate_counter)++; /* computing a hash is a unit of crypto work. */ from_wire(name); @@ -833,7 +839,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch failflags &= ~DNSSEC_FAIL_NOSIG; rc = validate_rrset(now, header, plen, class, T_DNSKEY, sigcnt, rrcnt, name, keyname, - NULL, key, rdlen - 4, algo, keytag, &sig_ttl); + NULL, key, rdlen - 4, algo, keytag, &sig_ttl, validate_counter); if (STAT_ISEQUAL(rc, STAT_ABANDONED)) return STAT_ABANDONED; @@ -958,7 +964,8 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch STAT_ABANDONED resource exhaustion. */ -int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class) +int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char *name, + char *keyname, int class, int *validate_counter) { unsigned char *p = (unsigned char *)(header+1); int qtype, qclass, rc, i, neganswer = 0, nons = 0, servfail = 0, neg_ttl = 0, found_supported = 0; @@ -983,7 +990,7 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char servfail = neganswer = nons = 1; else { - rc = dnssec_validate_reply(now, header, plen, name, keyname, NULL, 0, &neganswer, &nons, &neg_ttl); + rc = dnssec_validate_reply(now, header, plen, name, keyname, NULL, 0, &neganswer, &nons, &neg_ttl, validate_counter); if (STAT_ISEQUAL(rc, STAT_INSECURE)) { @@ -1466,8 +1473,8 @@ static int check_nsec3_coverage(struct dns_header *header, size_t plen, int dige } /* returns 0 on success, or DNSSEC_FAIL_* value on failure. */ -static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, unsigned char **nsecs, int nsec_count, - char *workspace1, char *workspace2, char *name, int type, char *wildname, int *nons) +static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, unsigned char **nsecs, int nsec_count, char *workspace1, + char *workspace2, char *name, int type, char *wildname, int *nons, int *validate_counter) { unsigned char *salt, *p, *digest; int digest_len, i, iterations, salt_len, base32_len, algo = 0; @@ -1551,6 +1558,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns nsecs[i] = nsec3p; } + (*validate_counter)++; if ((digest_len = hash_name(name, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1570,6 +1578,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns if (wildname && hostname_isequal(closest_encloser, wildname)) break; + (*validate_counter)++; if ((digest_len = hash_name(closest_encloser, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1598,6 +1607,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns return DNSSEC_FAIL_NONSEC; /* Look for NSEC3 that proves the non-existence of the next-closest encloser */ + (*validate_counter)++; if ((digest_len = hash_name(next_closest, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1613,6 +1623,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns wildcard--; *wildcard = '*'; + (*validate_counter)++; if ((digest_len = hash_name(wildcard, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1624,7 +1635,8 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns } /* returns 0 on success, or DNSSEC_FAIL_* value on failure. */ -static int prove_non_existence(struct dns_header *header, size_t plen, char *keyname, char *name, int qtype, int qclass, char *wildname, int *nons, int *nsec_ttl) +static int prove_non_existence(struct dns_header *header, size_t plen, char *keyname, char *name, int qtype, int qclass, + char *wildname, int *nons, int *nsec_ttl, int *validate_counter) { static unsigned char **nsecset = NULL, **rrsig_labels = NULL; static int nsecset_sz = 0, rrsig_labels_sz = 0; @@ -1743,7 +1755,7 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key if (type_found == T_NSEC) return prove_non_existence_nsec(header, plen, nsecset, rrsig_labels, nsecs_found, daemon->workspacename, keyname, name, qtype, nons); else if (type_found == T_NSEC3) - return prove_non_existence_nsec3(header, plen, nsecset, nsecs_found, daemon->workspacename, keyname, name, qtype, wildname, nons); + return prove_non_existence_nsec3(header, plen, nsecset, nsecs_found, daemon->workspacename, keyname, name, qtype, wildname, nons, validate_counter); else return DNSSEC_FAIL_NONSEC; } @@ -1850,7 +1862,7 @@ static int zone_status(char *name, int class, char *keyname, time_t now) if the nons argument is non-NULL. */ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, - int *class, int check_unsigned, int *neganswer, int *nons, int *nsec_ttl) + int *class, int check_unsigned, int *neganswer, int *nons, int *nsec_ttl, int *validate_counter) { static unsigned char **targets = NULL; static int target_sz = 0; @@ -2025,7 +2037,7 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch { unsigned long sig_ttl; rc = validate_rrset(now, header, plen, class1, type1, sigcnt, - rrcnt, name, keyname, &wildname, NULL, 0, 0, 0, &sig_ttl); + rrcnt, name, keyname, &wildname, NULL, 0, 0, 0, &sig_ttl, validate_counter); if (STAT_ISEQUAL(rc, STAT_BOGUS) || STAT_ISEQUAL(rc, STAT_NEED_KEY) || STAT_ISEQUAL(rc, STAT_NEED_DS) || STAT_ISEQUAL(rc, STAT_ABANDONED)) { @@ -2061,7 +2073,7 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch That's not a problem since if the RRsets later fail we'll return BOGUS then. */ if (STAT_ISEQUAL(rc, STAT_SECURE_WILDCARD) && - ((rc_nsec = prove_non_existence(header, plen, keyname, name, type1, class1, wildname, NULL, NULL))) != 0) + ((rc_nsec = prove_non_existence(header, plen, keyname, name, type1, class1, wildname, NULL, NULL, validate_counter))) != 0) return STAT_BOGUS | rc_nsec; rc = STAT_SECURE; @@ -2087,7 +2099,7 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch /* For anything other than a DS record, this situation is OK if either the answer is in an unsigned zone, or there's a NSEC records. */ - if ((rc_nsec = prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons, nsec_ttl)) != 0) + if ((rc_nsec = prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons, nsec_ttl, validate_counter)) != 0) { /* Empty DS without NSECS */ if (qtype == T_DS) diff --git a/src/forward.c b/src/forward.c index f8769bc..2ae8c41 100644 --- a/src/forward.c +++ b/src/forward.c @@ -16,6 +16,8 @@ #include "dnsmasq.h" +static int vchwm = 0; /* TODO */ + static struct frec *get_new_frec(time_t now, struct server *serv, int force); static struct frec *lookup_frec(unsigned short id, int fd, void *hash, int *firstp, int *lastp); static struct frec *lookup_frec_by_query(void *hash, unsigned int flags, unsigned int flagmask); @@ -339,6 +341,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, forward->flags |= FREC_AD_QUESTION; #ifdef HAVE_DNSSEC forward->work_counter = DNSSEC_WORK; + forward->validate_counter = 0; if (do_bit) forward->flags |= FREC_DO_QUESTION; #endif @@ -892,6 +895,8 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server static void dnssec_validate(struct frec *forward, struct dns_header *header, ssize_t plen, int status, time_t now) { + struct frec *orig; + daemon->log_display_id = forward->frec_src.log_id; /* We've had a reply already, which we're validating. Ignore this duplicate */ @@ -916,6 +921,9 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, log_query(F_UPSTREAM | F_NOEXTRA, daemon->namebuff, NULL, "truncated", (forward->flags & FREC_DNSKEY_QUERY) ? T_DNSKEY : T_DS); } } + + /* Find the original query that started it all.... */ + for (orig = forward; orig->dependent; orig = orig->dependent); /* As soon as anything returns BOGUS, we stop and unwind, to do otherwise would invite infinite loops, since the answers to DNSKEY and DS queries @@ -923,13 +931,13 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, if (!STAT_ISEQUAL(status, STAT_BOGUS) && !STAT_ISEQUAL(status, STAT_TRUNCATED) && !STAT_ISEQUAL(status, STAT_ABANDONED)) { if (forward->flags & FREC_DNSKEY_QUERY) - status = dnssec_validate_by_ds(now, header, plen, daemon->namebuff, daemon->keyname, forward->class); + status = dnssec_validate_by_ds(now, header, plen, daemon->namebuff, daemon->keyname, forward->class, &orig->validate_counter); else if (forward->flags & FREC_DS_QUERY) - status = dnssec_validate_ds(now, header, plen, daemon->namebuff, daemon->keyname, forward->class); + status = dnssec_validate_ds(now, header, plen, daemon->namebuff, daemon->keyname, forward->class, &orig->validate_counter); else status = dnssec_validate_reply(now, header, plen, daemon->namebuff, daemon->keyname, &forward->class, !option_bool(OPT_DNSSEC_IGN_NS) && (forward->sentto->flags & SERV_DO_DNSSEC), - NULL, NULL, NULL); + NULL, NULL, NULL, &orig->validate_counter); if (STAT_ISEQUAL(status, STAT_ABANDONED)) { @@ -986,15 +994,11 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, else { struct server *server; - struct frec *orig; void *hash; size_t nn; int serverind, fd; struct randfd_list *rfds = NULL; - /* Find the original query that started it all.... */ - for (orig = forward; orig->dependent; orig = orig->dependent); - /* Make sure we don't expire and free the orig frec during the allocation of a new one: third arg of get_new_frec() does that. */ if ((serverind = dnssec_server(forward->sentto, daemon->keyname, NULL, NULL)) != -1 && @@ -1347,6 +1351,11 @@ static void return_reply(time_t now, struct frec *forward, struct dns_header *he log_query(F_SECSTAT, domain, &a, result, 0); } } + + if (forward->validate_counter > vchwm) + vchwm = forward->validate_counter; + if (extract_request(header, n, daemon->namebuff, NULL)) + my_syslog(LOG_INFO, "Validate_counter %s is %d, HWM is %d", daemon->namebuff, forward->validate_counter, vchwm); /* TODO */ #endif if (option_bool(OPT_NO_REBIND)) @@ -2028,7 +2037,7 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet, /* Recurse down the key hierarchy */ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, size_t n, int class, char *name, char *keyname, struct server *server, - int have_mark, unsigned int mark, int *keycount) + int have_mark, unsigned int mark, int *keycount, int *validatecount) { int first, last, start, new_status; unsigned char *packet = NULL; @@ -2043,13 +2052,13 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si if (--(*keycount) == 0) new_status = STAT_ABANDONED; else if (STAT_ISEQUAL(status, STAT_NEED_KEY)) - new_status = dnssec_validate_by_ds(now, header, n, name, keyname, class); + new_status = dnssec_validate_by_ds(now, header, n, name, keyname, class, validatecount); else if (STAT_ISEQUAL(status, STAT_NEED_DS)) - new_status = dnssec_validate_ds(now, header, n, name, keyname, class); + new_status = dnssec_validate_ds(now, header, n, name, keyname, class, validatecount); else new_status = dnssec_validate_reply(now, header, n, name, keyname, &class, !option_bool(OPT_DNSSEC_IGN_NS) && (server->flags & SERV_DO_DNSSEC), - NULL, NULL, NULL); + NULL, NULL, NULL, validatecount); if (STAT_ISEQUAL(new_status, STAT_ABANDONED)) { @@ -2093,7 +2102,8 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si log_query_mysockaddr(F_NOEXTRA | F_DNSSEC | F_SERVER, keyname, &server->addr, STAT_ISEQUAL(new_status, STAT_NEED_KEY) ? "dnssec-query[DNSKEY]" : "dnssec-query[DS]", 0); - new_status = tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, have_mark, mark, keycount); + new_status = tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, + have_mark, mark, keycount, validatecount); daemon->log_display_id = log_save; @@ -2399,8 +2409,9 @@ unsigned char *tcp_request(int confd, time_t now, if (option_bool(OPT_DNSSEC_VALID) && !checking_disabled && (master->flags & SERV_DO_DNSSEC)) { int keycount = DNSSEC_WORK; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */ + int validatecount = 0; /* How many validations we did */ int status = tcp_key_recurse(now, STAT_OK, header, m, 0, daemon->namebuff, daemon->keyname, - serv, have_mark, mark, &keycount); + serv, have_mark, mark, &keycount, &validatecount); char *result, *domain = "result"; union all_addr a; @@ -2426,6 +2437,11 @@ unsigned char *tcp_request(int confd, time_t now, } log_query(F_SECSTAT, domain, &a, result, 0); + + if (validatecount > vchwm) + vchwm = validatecount; + if (extract_request(header, m, daemon->namebuff, NULL)) + my_syslog(LOG_INFO, "Validate_counter %s is %d, HWM is %d", daemon->namebuff, validatecount, vchwm); /* TODO */ } #endif From bfefd6e38c6e31e3e3333b9efab0b95328e958a5 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Tue, 2 Jan 2024 12:25:44 +0000 Subject: [PATCH 05/13] Fix error introduced in 635bc51cac3d5d7dd49ce9e27149cf7e402b7e79 --- src/dnssec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dnssec.c b/src/dnssec.c index cb35175..bc02dad 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -1282,7 +1282,7 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi p += p[1]; } - return 1; + return 0; } else if (rc == -1) { From c5aa221e44a4c5724434ddfca0d4ca38c9594c73 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Tue, 2 Jan 2024 21:43:04 +0000 Subject: [PATCH 06/13] Parameterise work limits for DNSSEC validation. --- src/cache.c | 16 ++++++++- src/config.h | 5 +++ src/dnsmasq.h | 2 ++ src/dnssec.c | 90 +++++++++++++++++++++++++++++---------------------- src/forward.c | 22 ++++++------- src/metrics.c | 1 + src/metrics.h | 1 + src/option.c | 9 +++++- 8 files changed, 94 insertions(+), 52 deletions(-) diff --git a/src/cache.c b/src/cache.c index 69afeca..3ee823e 100644 --- a/src/cache.c +++ b/src/cache.c @@ -834,6 +834,12 @@ void cache_end_insert(void) if (daemon->pipe_to_parent != -1) { ssize_t m = -1; + +#ifdef HAVE_DNSSEC + /* Sneak out possibly updated crypto HWM. */ + m = -1 - daemon->metrics[METRIC_CRYTO_HWM]; +#endif + read_write(daemon->pipe_to_parent, (unsigned char *)&m, sizeof(m), 0); } @@ -859,8 +865,13 @@ int cache_recv_insert(time_t now, int fd) if (!read_write(fd, (unsigned char *)&m, sizeof(m), 1)) return 0; - if (m == -1) + if (m < 0) { +#ifdef HAVE_DNSSEC + /* Sneak in possibly updated crypto HWM. */ + if ((-m - 1) > daemon->metrics[METRIC_CRYTO_HWM]) + daemon->metrics[METRIC_CRYTO_HWM] = -m - 1; +#endif cache_end_insert(); return 1; } @@ -1893,6 +1904,9 @@ void dump_cache(time_t now) #ifdef HAVE_AUTH my_syslog(LOG_INFO, _("queries for authoritative zones %u"), daemon->metrics[METRIC_DNS_AUTH_ANSWERED]); #endif +#ifdef HAVE_DNSSEC + my_syslog(LOG_INFO, _("DNSSEC per-query crypto HWM %u"), daemon->metrics[METRIC_CRYTO_HWM]); +#endif blockdata_report(); my_syslog(LOG_INFO, _("child processes for TCP requests: in use %zu, highest since last SIGUSR1 %zu, max allowed %zu."), diff --git a/src/config.h b/src/config.h index c753390..b987ad1 100644 --- a/src/config.h +++ b/src/config.h @@ -23,6 +23,11 @@ #define SAFE_PKTSZ 1232 /* "go anywhere" UDP packet size, see https://dnsflagday.net/2020/ */ #define KEYBLOCK_LEN 40 /* choose to minimise fragmentation when storing DNSSEC keys */ #define DNSSEC_WORK 50 /* Max number of queries to validate one question */ +#define LIMIT_KEY_FAIL 15 /* Number of keys that can fail DS validate in one an answer. */ +#define LIMIT_DS_FAIL 5 /* Number of DS records that can fail to validate a key in one answer */ +#define LIMIT_SIG_FAIL 10 /* Number of signature that can fail to validate in one answer */ +#define LIMIT_CRYPTO 40 /* max no. of crypto operations to validate one a query. */ +#define LIMIT_NSEC3_ITERS 150 /* Max. number if iterations allow in NSEC3 record. */ #define TIMEOUT 10 /* drop UDP queries after TIMEOUT seconds */ #define SMALL_PORT_RANGE 30 /* If DNS port range is smaller than this, use different allocation. */ #define FORWARD_TEST 50 /* try all servers every 50 queries */ diff --git a/src/dnsmasq.h b/src/dnsmasq.h index ce35fc4..c62cbac 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -759,6 +759,7 @@ struct dyndir { #define DNSSEC_FAIL_NOKEY 0x0100 /* no DNSKEY */ #define DNSSEC_FAIL_NSEC3_ITERS 0x0200 /* too many iterations in NSEC3 */ #define DNSSEC_FAIL_BADPACKET 0x0400 /* bad packet */ +#define DNSSEC_FAIL_WORK 0x0800 /* too much crypto */ #define STAT_ISEQUAL(a, b) (((a) & 0xffff0000) == (b)) @@ -1242,6 +1243,7 @@ extern struct daemon { int rr_status_sz; int dnssec_no_time_check; int back_to_the_future; + int limit_key_fail, limit_ds_fail, limit_sig_fail, limit_crypto, limit_work, limit_nsec3_iters; #endif struct frec *frec_list; struct frec_src *free_frec_src; diff --git a/src/dnssec.c b/src/dnssec.c index bc02dad..1a334ec 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -424,6 +424,17 @@ static int explore_rrset(struct dns_header *header, size_t plen, int class, int return 1; } +int dec_counter(int *counter, char *message) +{ + if ((*counter)-- == 0) + { + my_syslog(LOG_WARNING, "limit exceeded: %s", message ? message : "crypto work"); + return 1; + } + + return 0; +} + /* Validate a single RRset (class, type, name) in the supplied DNS reply Return code: STAT_SECURE if it validates. @@ -468,7 +479,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in rrsetidx = sort_rrset(header, plen, rr_desc, rrsetidx, rrset, daemon->workspacename, keyname); /* Now try all the sigs to try and find one which validates */ - for (sig_fail_cnt = 0, j = 0; j limit_sig_fail, j = 0; j digest_size, algo)) - return STAT_SECURE; + { + if (dec_counter(validate_counter, NULL)) + return STAT_ABANDONED; + + if (verify(key, keylen, sig, sig_len, digest, hash->digest_size, algo)) + return STAT_SECURE; + } } else { @@ -669,21 +683,17 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in crecp->addr.key.keytag == key_tag && crecp->uid == (unsigned int)class) { - (*validate_counter)++; - + if (dec_counter(validate_counter, NULL)) + return STAT_ABANDONED; + if (verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) return (labels < name_labels) ? STAT_SECURE_WILDCARD : STAT_SECURE; /* An attacker can waste a lot of our CPU by setting up a giant DNSKEY RRSET full of failing keys, all of which we have to try. Since many failing keys is not likely for a legitimate domain, set a limit on how many can fail. */ - sig_fail_cnt++; - - if (sig_fail_cnt > 10) /* TODO */ - { - my_syslog(LOG_ERR, "sig_fail_cnt"); - return STAT_ABANDONED; - } + if (dec_counter(&sig_fail_cnt, "SIG fail")) + return STAT_ABANDONED; } } } @@ -733,7 +743,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch } /* NOTE, we need to find ONE DNSKEY which matches the DS */ - for (key_fail_cnt = 0, valid = 0, j = ntohs(header->ancount); j != 0 && !valid; j--) + for (key_fail_cnt = daemon->limit_key_fail, valid = 0, j = ntohs(header->ancount); j != 0 && !valid; j--) { /* Ensure we have type, class TTL and length */ if (!(rc = extract_name(header, plen, &p, name, 0, 10))) @@ -781,8 +791,8 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch /* No zone key flag or malloc failure */ if (!key) continue; - - for (ds_fail_cnt = 0, recp1 = crecp; recp1; recp1 = cache_find_by_name(recp1, name, now, F_DS)) + + for (ds_fail_cnt = daemon->limit_ds_fail, recp1 = crecp; recp1; recp1 = cache_find_by_name(recp1, name, now, F_DS)) { void *ctx; unsigned char *digest, *ds_digest; @@ -801,6 +811,10 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch else failflags &= ~DNSSEC_FAIL_NODSSUP; + /* computing a hash is a unit of crypto work. */ + if (dec_counter(validate_counter, NULL)) + return STAT_ABANDONED; + if (!hash_init(hash, &ctx, &digest)) continue; @@ -811,7 +825,6 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch hash->update(ctx, (unsigned int)wire_len, (unsigned char *)name); hash->update(ctx, (unsigned int)rdlen, psave); hash->digest(ctx, hash->digest_size, digest); - (*validate_counter)++; /* computing a hash is a unit of crypto work. */ from_wire(name); @@ -822,13 +835,8 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch if (memcmp(ds_digest, digest, recp1->addr.ds.keylen) != 0) { /* limit CPU exhaustion attack from large DS x KEY cross-product. */ - ds_fail_cnt++; - - if (ds_fail_cnt > 5) /* TODO */ - { - my_syslog(LOG_ERR, "ds_fail_cnt"); - return STAT_ABANDONED; - } + if (dec_counter(&ds_fail_cnt, "DS fail")) + return STAT_ABANDONED; } else if (explore_rrset(header, plen, class, T_DNSKEY, name, keyname, &sigcnt, &rrcnt) && rrcnt != 0) @@ -858,13 +866,8 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch blockdata_free(key); /* limit CPU exhaustion attack from large DS x KEY cross-product. */ - key_fail_cnt++; - - if (key_fail_cnt > 15) /* TODO */ - { - my_syslog(LOG_ERR, "key_fail_cnt"); - return STAT_ABANDONED; - } + if (dec_counter(&key_fail_cnt, "KEY fail")) + return STAT_ABANDONED; } if (valid) @@ -1511,7 +1514,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns GETSHORT (iterations, p); /* Upper-bound iterations, to avoid DoS. RFC 9276 refers. */ - if (iterations > 150) + if (iterations > daemon->limit_nsec3_iters) return DNSSEC_FAIL_NSEC3_ITERS; salt_len = *p++; @@ -1558,7 +1561,9 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns nsecs[i] = nsec3p; } - (*validate_counter)++; + if (dec_counter(validate_counter, NULL)) + return DNSSEC_FAIL_WORK; + if ((digest_len = hash_name(name, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1578,7 +1583,9 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns if (wildname && hostname_isequal(closest_encloser, wildname)) break; - (*validate_counter)++; + if (dec_counter(validate_counter, NULL)) + return DNSSEC_FAIL_WORK; + if ((digest_len = hash_name(closest_encloser, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1607,7 +1614,9 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns return DNSSEC_FAIL_NONSEC; /* Look for NSEC3 that proves the non-existence of the next-closest encloser */ - (*validate_counter)++; + if (dec_counter(validate_counter, NULL)) + return DNSSEC_FAIL_WORK; + if ((digest_len = hash_name(next_closest, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -1623,7 +1632,9 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns wildcard--; *wildcard = '*'; - (*validate_counter)++; + if (dec_counter(validate_counter, NULL)) + return DNSSEC_FAIL_WORK; + if ((digest_len = hash_name(wildcard, &digest, hash, salt, salt_len, iterations)) == 0) return DNSSEC_FAIL_NONSEC; @@ -2074,7 +2085,7 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch we'll return BOGUS then. */ if (STAT_ISEQUAL(rc, STAT_SECURE_WILDCARD) && ((rc_nsec = prove_non_existence(header, plen, keyname, name, type1, class1, wildname, NULL, NULL, validate_counter))) != 0) - return STAT_BOGUS | rc_nsec; + return (rc_nsec & DNSSEC_FAIL_WORK) ? STAT_ABANDONED : (STAT_BOGUS | rc_nsec); rc = STAT_SECURE; } @@ -2101,6 +2112,9 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch the answer is in an unsigned zone, or there's a NSEC records. */ if ((rc_nsec = prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons, nsec_ttl, validate_counter)) != 0) { + if (rc_nsec & DNSSEC_FAIL_WORK) + return STAT_ABANDONED; + /* Empty DS without NSECS */ if (qtype == T_DS) return STAT_BOGUS | rc_nsec; diff --git a/src/forward.c b/src/forward.c index 2ae8c41..b952b04 100644 --- a/src/forward.c +++ b/src/forward.c @@ -16,8 +16,6 @@ #include "dnsmasq.h" -static int vchwm = 0; /* TODO */ - static struct frec *get_new_frec(time_t now, struct server *serv, int force); static struct frec *lookup_frec(unsigned short id, int fd, void *hash, int *firstp, int *lastp); static struct frec *lookup_frec_by_query(void *hash, unsigned int flags, unsigned int flagmask); @@ -340,8 +338,8 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, if (ad_reqd) forward->flags |= FREC_AD_QUESTION; #ifdef HAVE_DNSSEC - forward->work_counter = DNSSEC_WORK; - forward->validate_counter = 0; + forward->work_counter = daemon->limit_work; + forward->validate_counter = daemon->limit_crypto; if (do_bit) forward->flags |= FREC_DO_QUESTION; #endif @@ -1352,10 +1350,10 @@ static void return_reply(time_t now, struct frec *forward, struct dns_header *he } } - if (forward->validate_counter > vchwm) - vchwm = forward->validate_counter; + if ((daemon->limit_crypto - forward->validate_counter) > daemon->metrics[METRIC_CRYTO_HWM]) + daemon->metrics[METRIC_CRYTO_HWM] = daemon->limit_crypto - forward->validate_counter; if (extract_request(header, n, daemon->namebuff, NULL)) - my_syslog(LOG_INFO, "Validate_counter %s is %d, HWM is %d", daemon->namebuff, forward->validate_counter, vchwm); /* TODO */ + my_syslog(LOG_INFO, "Validate_counter %s is %d", daemon->namebuff, daemon->limit_crypto - forward->validate_counter); /* TODO */ #endif if (option_bool(OPT_NO_REBIND)) @@ -2408,8 +2406,8 @@ unsigned char *tcp_request(int confd, time_t now, #ifdef HAVE_DNSSEC if (option_bool(OPT_DNSSEC_VALID) && !checking_disabled && (master->flags & SERV_DO_DNSSEC)) { - int keycount = DNSSEC_WORK; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */ - int validatecount = 0; /* How many validations we did */ + int keycount = daemon->limit_work; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */ + int validatecount = daemon->limit_crypto; int status = tcp_key_recurse(now, STAT_OK, header, m, 0, daemon->namebuff, daemon->keyname, serv, have_mark, mark, &keycount, &validatecount); char *result, *domain = "result"; @@ -2438,10 +2436,10 @@ unsigned char *tcp_request(int confd, time_t now, log_query(F_SECSTAT, domain, &a, result, 0); - if (validatecount > vchwm) - vchwm = validatecount; + if ((daemon->limit_crypto - validatecount) > daemon->metrics[METRIC_CRYTO_HWM]) + daemon->metrics[METRIC_CRYTO_HWM] = daemon->limit_crypto - validatecount; if (extract_request(header, m, daemon->namebuff, NULL)) - my_syslog(LOG_INFO, "Validate_counter %s is %d, HWM is %d", daemon->namebuff, validatecount, vchwm); /* TODO */ + my_syslog(LOG_INFO, "Validate_counter %s is %d", daemon->namebuff, daemon->limit_crypto - validatecount); /* TODO */ } #endif diff --git a/src/metrics.c b/src/metrics.c index f8b8d9c..da40614 100644 --- a/src/metrics.c +++ b/src/metrics.c @@ -24,6 +24,7 @@ const char * metric_names[] = { "dns_local_answered", "dns_stale_answered", "dns_unanswered", + "max_crypto_use", "bootp", "pxe", "dhcp_ack", diff --git a/src/metrics.h b/src/metrics.h index 839e01d..3ca2173 100644 --- a/src/metrics.h +++ b/src/metrics.h @@ -23,6 +23,7 @@ enum { METRIC_DNS_LOCAL_ANSWERED, METRIC_DNS_STALE_ANSWERED, METRIC_DNS_UNANSWERED_QUERY, + METRIC_CRYTO_HWM, METRIC_BOOTP, METRIC_PXE, METRIC_DHCPACK, diff --git a/src/option.c b/src/option.c index b0d23bc..cc935fd 100644 --- a/src/option.c +++ b/src/option.c @@ -5869,7 +5869,14 @@ void read_opts(int argc, char **argv, char *compile_opts) daemon->randport_limit = 1; daemon->host_index = SRC_AH; daemon->max_procs = MAX_PROCS; - daemon->max_procs_used = 0; +#ifdef HAVE_DNSSEC + daemon->limit_key_fail = LIMIT_KEY_FAIL; + daemon->limit_ds_fail = LIMIT_DS_FAIL; + daemon->limit_sig_fail = LIMIT_SIG_FAIL; + daemon->limit_crypto = LIMIT_CRYPTO; + daemon->limit_work = DNSSEC_WORK; + daemon->limit_nsec3_iters = LIMIT_NSEC3_ITERS; +#endif /* See comment above make_servers(). Optimises server-read code. */ mark_servers(0); From 06945c4b776cc54a5101c04549e666ed29a101b9 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 4 Jan 2024 00:45:31 +0000 Subject: [PATCH 07/13] Update EDE code -> text conversion. --- src/cache.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/cache.c b/src/cache.c index 3ee823e..33dd929 100644 --- a/src/cache.c +++ b/src/cache.c @@ -2066,6 +2066,11 @@ static char *edestr(int ede) case EDE_NO_AUTH: return "no reachable authority"; case EDE_NETERR: return "network error"; case EDE_INVALID_DATA: return "invalid data"; + case EDE_SIG_E_B_V: return "signature expired before valid"; + case EDE_TOO_EARLY: return "too early"; + case EDE_UNS_NS3_ITER: return "unsupported NSEC3 iterations value"; + case EDE_UNABLE_POLICY: return "uanble to conform to policy"; + case EDE_SYNTHESIZED: return "synthesized"; default: return "unknown"; } } From 6f23a0a75eefaf4e6ff3c76ed2503a0575a085a0 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 4 Jan 2024 15:57:43 +0000 Subject: [PATCH 08/13] Rework validate-by-DS to avoid DoS vuln without arbitrary limits. By calculating the hash of a DNSKEY once for each digest algo, we reduce the hashing work from (no. DS) x (no. DNSKEY) to (no. DNSKEY) x (no. distinct digests) The number of distinct digests can never be more than 255 and it's limited by which hashes we implement, so currently only 4. --- src/config.h | 6 +- src/dnsmasq.h | 2 +- src/dnssec.c | 298 ++++++++++++++++++++++++-------------------------- src/forward.c | 8 +- src/option.c | 2 - 5 files changed, 150 insertions(+), 166 deletions(-) diff --git a/src/config.h b/src/config.h index b987ad1..5c0fa1f 100644 --- a/src/config.h +++ b/src/config.h @@ -23,10 +23,8 @@ #define SAFE_PKTSZ 1232 /* "go anywhere" UDP packet size, see https://dnsflagday.net/2020/ */ #define KEYBLOCK_LEN 40 /* choose to minimise fragmentation when storing DNSSEC keys */ #define DNSSEC_WORK 50 /* Max number of queries to validate one question */ -#define LIMIT_KEY_FAIL 15 /* Number of keys that can fail DS validate in one an answer. */ -#define LIMIT_DS_FAIL 5 /* Number of DS records that can fail to validate a key in one answer */ -#define LIMIT_SIG_FAIL 10 /* Number of signature that can fail to validate in one answer */ -#define LIMIT_CRYPTO 40 /* max no. of crypto operations to validate one a query. */ +#define LIMIT_SIG_FAIL 20 /* Number of signature that can fail to validate in one answer */ +#define LIMIT_CRYPTO 200 /* max no. of crypto operations to validate one a query. */ #define LIMIT_NSEC3_ITERS 150 /* Max. number if iterations allow in NSEC3 record. */ #define TIMEOUT 10 /* drop UDP queries after TIMEOUT seconds */ #define SMALL_PORT_RANGE 30 /* If DNS port range is smaller than this, use different allocation. */ diff --git a/src/dnsmasq.h b/src/dnsmasq.h index c62cbac..704bc63 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -1243,7 +1243,7 @@ extern struct daemon { int rr_status_sz; int dnssec_no_time_check; int back_to_the_future; - int limit_key_fail, limit_ds_fail, limit_sig_fail, limit_crypto, limit_work, limit_nsec3_iters; + int limit_sig_fail, limit_crypto, limit_work, limit_nsec3_iters; #endif struct frec *frec_list; struct frec_src *free_frec_src; diff --git a/src/dnssec.c b/src/dnssec.c index 1a334ec..036d560 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -711,39 +711,42 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in or self-sign for DNSKEY RRset is not valid, bad packet. STAT_ABANDONED resource exhaustion. STAT_NEED_DS DS records to validate a key not found, name in keyname - STAT_NEED_KEY DNSKEY records to validate a key not found, name in keyname */ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, char *name, char *keyname, int class, int *validate_counter) { - unsigned char *psave, *p = (unsigned char *)(header+1); + unsigned char *psave, *p = (unsigned char *)(header+1), *keyaddr; struct crec *crecp, *recp1; - int rc, j, qtype, qclass, rdlen, flags, algo, valid, keytag, ds_fail_cnt, key_fail_cnt; + int rc, j, qtype, qclass, rdlen, flags, algo, keytag, sigcnt, rrcnt; unsigned long ttl, sig_ttl; - struct blockdata *key; union all_addr a; - int failflags = DNSSEC_FAIL_NOSIG | DNSSEC_FAIL_NODSSUP | DNSSEC_FAIL_NOZONE | DNSSEC_FAIL_NOKEY; + int failflags = DNSSEC_FAIL_NODSSUP | DNSSEC_FAIL_NOZONE; + char valid_digest[255]; + static unsigned char *cached_digest[255]; - if (ntohs(header->qdcount) != 1 || - RCODE(header) == SERVFAIL || RCODE(header) == REFUSED || - !extract_name(header, plen, &p, name, 1, 4)) + if (ntohs(header->qdcount) != 1 || RCODE(header) != NOERROR || !extract_name(header, plen, &p, name, 1, 4)) return STAT_BOGUS | DNSSEC_FAIL_NOKEY; GETSHORT(qtype, p); GETSHORT(qclass, p); - if (qtype != T_DNSKEY || qclass != class || ntohs(header->ancount) == 0) + if (qtype != T_DNSKEY || qclass != class || + !explore_rrset(header, plen, class, T_DNSKEY, name, keyname, &sigcnt, &rrcnt) || + rrcnt == 0) return STAT_BOGUS | DNSSEC_FAIL_NOKEY; + if (sigcnt == 0) + return STAT_BOGUS | DNSSEC_FAIL_NOSIG; + /* See if we have cached a DS record which validates this key */ if (!(crecp = cache_find_by_name(NULL, name, now, F_DS))) { strcpy(keyname, name); return STAT_NEED_DS; } - + /* NOTE, we need to find ONE DNSKEY which matches the DS */ - for (key_fail_cnt = daemon->limit_key_fail, valid = 0, j = ntohs(header->ancount); j != 0 && !valid; j--) + for (j = ntohs(header->ancount); j != 0; j--) { /* Ensure we have type, class TTL and length */ if (!(rc = extract_name(header, plen, &p, name, 0, 10))) @@ -754,7 +757,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch GETLONG(ttl, p); GETSHORT(rdlen, p); - if (!CHECK_LEN(header, p, plen, rdlen) || rdlen < 4) + if (!CHECK_LEN(header, p, plen, rdlen)) return STAT_BOGUS; /* bad packet */ if (qclass != class || qtype != T_DNSKEY || rc == 2) @@ -762,55 +765,59 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch p += rdlen; continue; } - + + if (rdlen < 5) + return STAT_BOGUS; /* min 1 byte key! */ + psave = p; GETSHORT(flags, p); if (*p++ != 3) - return STAT_BOGUS | DNSSEC_FAIL_NOKEY; + { + p = psave + rdlen; + continue; + } algo = *p++; - keytag = dnskey_keytag(algo, flags, p, rdlen - 4); - key = NULL; + keyaddr = p; + keytag = dnskey_keytag(algo, flags, keyaddr, rdlen - 4); - /* key must have zone key flag set */ - if (flags & 0x100) - { - key = blockdata_alloc((char*)p, rdlen - 4); - failflags &= ~DNSSEC_FAIL_NOZONE; - } - - p = psave; - - if (!ADD_RDLEN(header, p, plen, rdlen)) - { - if (key) - blockdata_free(key); - return STAT_BOGUS; /* bad packet */ - } + p = psave + rdlen; - /* No zone key flag or malloc failure */ - if (!key) + /* key must have zone key flag set */ + if (!(flags & 0x100)) continue; - - for (ds_fail_cnt = daemon->limit_ds_fail, recp1 = crecp; recp1; recp1 = cache_find_by_name(recp1, name, now, F_DS)) + + failflags &= ~DNSSEC_FAIL_NOZONE; + + /* clear digest cache. */ + memset(valid_digest, 0, sizeof(valid_digest)); + + for (recp1 = crecp; recp1; recp1 = cache_find_by_name(recp1, name, now, F_DS)) { void *ctx; unsigned char *digest, *ds_digest; const struct nettle_hash *hash; - int sigcnt, rrcnt; int wire_len; - if (recp1->addr.ds.algo == algo && - recp1->addr.ds.keytag == keytag && - recp1->uid == (unsigned int)class) - { - failflags &= ~DNSSEC_FAIL_NOKEY; + if ((recp1->flags & F_NEG) || + recp1->addr.ds.algo != algo || + recp1->addr.ds.keytag != keytag || + recp1->uid != (unsigned int)class) + continue; + + if (!(hash = hash_find(ds_digest_name(recp1->addr.ds.digest)))) + continue; + + failflags &= ~DNSSEC_FAIL_NODSSUP; - if (!(hash = hash_find(ds_digest_name(recp1->addr.ds.digest)))) - continue; - else - failflags &= ~DNSSEC_FAIL_NODSSUP; + if (recp1->addr.ds.keylen != (int)hash->digest_size || + !(ds_digest = blockdata_retrieve(recp1->addr.ds.keydata, recp1->addr.ds.keylen, NULL))) + continue; + if (valid_digest[recp1->addr.ds.digest]) + digest = cached_digest[recp1->addr.ds.digest]; + else + { /* computing a hash is a unit of crypto work. */ if (dec_counter(validate_counter, NULL)) return STAT_ABANDONED; @@ -821,132 +828,117 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch wire_len = to_wire(name); /* Note that digest may be different between DSs, so - we can't move this outside the loop. */ + we can't move this outside the loop. We keep + copies of each digest we make for this key, + so maximum digest work is O(keys x digests_types) + rather then O(keys x DSs) */ hash->update(ctx, (unsigned int)wire_len, (unsigned char *)name); hash->update(ctx, (unsigned int)rdlen, psave); hash->digest(ctx, hash->digest_size, digest); from_wire(name); - if (!(recp1->flags & F_NEG) && - recp1->addr.ds.keylen == (int)hash->digest_size && - (ds_digest = blockdata_retrieve(recp1->addr.ds.keydata, recp1->addr.ds.keylen, NULL))) + if (!cached_digest[recp1->addr.ds.digest]) + cached_digest[recp1->addr.ds.digest] = whine_malloc(recp1->addr.ds.keylen); + + if (cached_digest[recp1->addr.ds.digest]) { - if (memcmp(ds_digest, digest, recp1->addr.ds.keylen) != 0) - { - /* limit CPU exhaustion attack from large DS x KEY cross-product. */ - if (dec_counter(&ds_fail_cnt, "DS fail")) - return STAT_ABANDONED; - } - else if (explore_rrset(header, plen, class, T_DNSKEY, name, keyname, &sigcnt, &rrcnt) && - rrcnt != 0) - { - if (sigcnt == 0) - continue; - else - failflags &= ~DNSSEC_FAIL_NOSIG; - - rc = validate_rrset(now, header, plen, class, T_DNSKEY, sigcnt, rrcnt, name, keyname, - NULL, key, rdlen - 4, algo, keytag, &sig_ttl, validate_counter); - - if (STAT_ISEQUAL(rc, STAT_ABANDONED)) - return STAT_ABANDONED; - - failflags &= rc; - - if (STAT_ISEQUAL(rc, STAT_SECURE)) - { - valid = 1; - break; - } - } + memcpy(cached_digest[recp1->addr.ds.digest], digest, recp1->addr.ds.keylen); + valid_digest[recp1->addr.ds.digest] = 1; } } - } - blockdata_free(key); - - /* limit CPU exhaustion attack from large DS x KEY cross-product. */ - if (dec_counter(&key_fail_cnt, "KEY fail")) - return STAT_ABANDONED; - } - - if (valid) - { - /* DNSKEY RRset determined to be OK, now cache it. */ - cache_start_insert(); - - p = skip_questions(header, plen); - - for (j = ntohs(header->ancount); j != 0; j--) - { - /* Ensure we have type, class TTL and length */ - if (!(rc = extract_name(header, plen, &p, name, 0, 10))) - return STAT_BOGUS; /* bad packet */ - GETSHORT(qtype, p); - GETSHORT(qclass, p); - GETLONG(ttl, p); - GETSHORT(rdlen, p); - - /* TTL may be limited by sig. */ - if (sig_ttl < ttl) - ttl = sig_ttl; - - if (!CHECK_LEN(header, p, plen, rdlen)) - return STAT_BOGUS; /* bad packet */ - - if (qclass == class && rc == 1) + if (memcmp(ds_digest, digest, recp1->addr.ds.keylen) == 0) { - psave = p; + /* Found the key validated by a DS record. + Now check the self-sig for the entire key RRset using that key. + Note that validate_rrset() will never return STAT_NEED_KEY here, + since we supply the key it will use as an argument. */ + struct blockdata *key; + + if (!(key = blockdata_alloc((char *)keyaddr, rdlen - 4))) + break; + + rc = validate_rrset(now, header, plen, class, T_DNSKEY, sigcnt, rrcnt, name, keyname, + NULL, key, rdlen - 4, algo, keytag, &sig_ttl, validate_counter); - if (qtype == T_DNSKEY) + blockdata_free(key); + + if (STAT_ISEQUAL(rc, STAT_ABANDONED)) + return rc; + + /* can't validate KEY RRset with this key, see if there's another that + will, which is validated by another DS. */ + if (!STAT_ISEQUAL(rc, STAT_SECURE)) + break; + + /* DNSKEY RRset determined to be OK, now cache it. */ + cache_start_insert(); + + p = skip_questions(header, plen); + + for (j = ntohs(header->ancount); j != 0; j--) { - if (rdlen < 4) + /* Ensure we have type, class TTL and length */ + if (!(rc = extract_name(header, plen, &p, name, 0, 10))) return STAT_BOGUS; /* bad packet */ - GETSHORT(flags, p); - if (*p++ != 3) - return STAT_BOGUS; - algo = *p++; - keytag = dnskey_keytag(algo, flags, p, rdlen - 4); + GETSHORT(qtype, p); + GETSHORT(qclass, p); + GETLONG(ttl, p); + GETSHORT(rdlen, p); - if ((key = blockdata_alloc((char*)p, rdlen - 4))) - { - a.key.keylen = rdlen - 4; - a.key.keydata = key; - a.key.algo = algo; - a.key.keytag = keytag; - a.key.flags = flags; - - if (!cache_insert(name, &a, class, now, ttl, F_FORWARD | F_DNSKEY | F_DNSSECOK)) - { - blockdata_free(key); - return STAT_BOGUS; - } - else - { - a.log.keytag = keytag; - a.log.algo = algo; - if (algo_digest_name(algo)) - log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu", 0); - else - log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu (not supported)", 0); - } - } + /* TTL may be limited by sig. */ + if (sig_ttl < ttl) + ttl = sig_ttl; + + if (!CHECK_LEN(header, p, plen, rdlen)) + return STAT_BOGUS; /* bad packet */ + + psave = p; + + if (qclass == class && rc == 1 && qtype == T_DNSKEY) + { + if (rdlen < 4) + return STAT_BOGUS; /* min 1 byte key! */ + + GETSHORT(flags, p); + if (*p++ == 3) + { + algo = *p++; + keytag = dnskey_keytag(algo, flags, p, rdlen - 4); + + if (!(key = blockdata_alloc((char*)p, rdlen - 4))) + return STAT_BOGUS; + + a.key.keylen = rdlen - 4; + a.key.keydata = key; + a.key.algo = algo; + a.key.keytag = keytag; + a.key.flags = flags; + + if (!cache_insert(name, &a, class, now, ttl, F_FORWARD | F_DNSKEY | F_DNSSECOK)) + return STAT_BOGUS; + + a.log.keytag = keytag; + a.log.algo = algo; + if (algo_digest_name(algo)) + log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu", 0); + else + log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DNSKEY keytag %hu, algo %hu (not supported)", 0); + } + } + + p = psave + rdlen; } - - p = psave; + + /* commit cache insert. */ + cache_end_insert(); + return STAT_OK; } - - if (!ADD_RDLEN(header, p, plen, rdlen)) - return STAT_BOGUS; /* bad packet */ } - - /* commit cache insert. */ - cache_end_insert(); - return STAT_OK; } - + log_query(F_NOEXTRA | F_UPSTREAM, name, NULL, "BOGUS DNSKEY", 0); return STAT_BOGUS | failflags; } @@ -1056,7 +1048,7 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char a.log.keytag = keytag; a.log.algo = algo; a.log.digest = digest; - log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DS keytag %hu, algo %hu, digest %hu (not supported)", 0); + log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DS for keytag %hu, algo %hu, digest %hu (not supported)", 0); neg_ttl = ttl; } else if ((key = blockdata_alloc((char*)p, rdlen - 4))) @@ -1077,7 +1069,7 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char a.log.keytag = keytag; a.log.algo = algo; a.log.digest = digest; - log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DS keytag %hu, algo %hu, digest %hu", 0); + log_query(F_NOEXTRA | F_KEYTAG | F_UPSTREAM, name, &a, "DS for keytag %hu, algo %hu, digest %hu", 0); found_supported = 1; } } diff --git a/src/forward.c b/src/forward.c index b952b04..c247877 100644 --- a/src/forward.c +++ b/src/forward.c @@ -1350,10 +1350,8 @@ static void return_reply(time_t now, struct frec *forward, struct dns_header *he } } - if ((daemon->limit_crypto - forward->validate_counter) > daemon->metrics[METRIC_CRYTO_HWM]) + if ((daemon->limit_crypto - forward->validate_counter) > (int)daemon->metrics[METRIC_CRYTO_HWM]) daemon->metrics[METRIC_CRYTO_HWM] = daemon->limit_crypto - forward->validate_counter; - if (extract_request(header, n, daemon->namebuff, NULL)) - my_syslog(LOG_INFO, "Validate_counter %s is %d", daemon->namebuff, daemon->limit_crypto - forward->validate_counter); /* TODO */ #endif if (option_bool(OPT_NO_REBIND)) @@ -2436,10 +2434,8 @@ unsigned char *tcp_request(int confd, time_t now, log_query(F_SECSTAT, domain, &a, result, 0); - if ((daemon->limit_crypto - validatecount) > daemon->metrics[METRIC_CRYTO_HWM]) + if ((daemon->limit_crypto - validatecount) > (int)daemon->metrics[METRIC_CRYTO_HWM]) daemon->metrics[METRIC_CRYTO_HWM] = daemon->limit_crypto - validatecount; - if (extract_request(header, m, daemon->namebuff, NULL)) - my_syslog(LOG_INFO, "Validate_counter %s is %d", daemon->namebuff, daemon->limit_crypto - validatecount); /* TODO */ } #endif diff --git a/src/option.c b/src/option.c index cc935fd..1c20766 100644 --- a/src/option.c +++ b/src/option.c @@ -5870,8 +5870,6 @@ void read_opts(int argc, char **argv, char *compile_opts) daemon->host_index = SRC_AH; daemon->max_procs = MAX_PROCS; #ifdef HAVE_DNSSEC - daemon->limit_key_fail = LIMIT_KEY_FAIL; - daemon->limit_ds_fail = LIMIT_DS_FAIL; daemon->limit_sig_fail = LIMIT_SIG_FAIL; daemon->limit_crypto = LIMIT_CRYPTO; daemon->limit_work = DNSSEC_WORK; From 76bceb06c46618797bbc2c24f73ce1e650c76b31 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Fri, 5 Jan 2024 22:56:47 +0000 Subject: [PATCH 09/13] Overhaul data checking in NSEC code. --- src/dnssec.c | 102 +++++++++++++++++++++++++++++++------------------- src/metrics.c | 4 +- src/metrics.h | 4 +- 3 files changed, 70 insertions(+), 40 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index 036d560..c3246cb 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -918,7 +918,10 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch a.key.flags = flags; if (!cache_insert(name, &a, class, now, ttl, F_FORWARD | F_DNSKEY | F_DNSSECOK)) - return STAT_BOGUS; + { + blockdata_free(key); + return STAT_BOGUS; + } a.log.keytag = keytag; a.log.algo = algo; @@ -1019,6 +1022,8 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char for (i = 0; i < ntohs(header->ancount); i++) { + unsigned char *psave; + if (!(rc = extract_name(header, plen, &p, name, 0, 10))) return STAT_BOGUS; /* bad packet */ @@ -1029,15 +1034,16 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char if (!CHECK_LEN(header, p, plen, rdlen)) return STAT_BOGUS; /* bad packet */ + + psave = p; if (aclass == class && atype == T_DS && rc == 1) { int algo, digest, keytag; - unsigned char *psave = p; struct blockdata *key; - if (rdlen < 4) - return STAT_BOGUS; /* bad packet */ + if (rdlen < 5) + return STAT_BOGUS; /* min 1 byte digest! */ GETSHORT(keytag, p); algo = *p++; @@ -1073,12 +1079,9 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char found_supported = 1; } } - - p = psave; } - - if (!ADD_RDLEN(header, p, plen, rdlen)) - return STAT_BOGUS; /* bad packet */ + + p = psave + rdlen; } cache_end_insert(); @@ -1201,11 +1204,11 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi p = nsecs[i]; if (!extract_name(header, plen, &p, workspace1, 1, 10)) - return 0; + return DNSSEC_FAIL_BADPACKET; p += 8; /* class, type, TTL */ GETSHORT(rdlen, p); psave = p; - if (!extract_name(header, plen, &p, workspace2, 1, 10)) + if (!extract_name(header, plen, &p, workspace2, 1, 0)) return DNSSEC_FAIL_BADPACKET; /* If NSEC comes from wildcard expansion, use original wildcard @@ -1239,7 +1242,8 @@ static int prove_non_existence_nsec(struct dns_header *header, size_t plen, unsi /* NSEC with the same name as the RR we're testing, check that the type in question doesn't appear in the type map */ rdlen -= p - psave; - /* rdlen is now length of type map, and p points to it */ + /* rdlen is now length of type map, and p points to it + packet checked to be as long as rdlen implies in prove_non_existence() */ /* If we can prove that there's no NS record, return that information. */ if (nons && rdlen >= 2 && p[0] == 0 && (p[2] & (0x80 >> T_NS)) != 0) @@ -1368,23 +1372,23 @@ static int check_nsec3_coverage(struct dns_header *header, size_t plen, int dige for (i = 0; i < nsec_count; i++) if ((p = nsecs[i])) { - if (!extract_name(header, plen, &p, workspace1, 1, 0) || + if (!extract_name(header, plen, &p, workspace1, 1, 10) || !(base32_len = base32_decode(workspace1, (unsigned char *)workspace2))) return 0; p += 8; /* class, type, TTL */ GETSHORT(rdlen, p); + psave = p; + + /* packet checked to be as long as implied by rdlen, salt_len and hash_len in prove_non_existence() */ p++; /* algo */ flags = *p++; /* flags */ p += 2; /* iterations */ salt_len = *p++; /* salt_len */ p += salt_len; /* salt */ hash_len = *p++; /* p now points to next hashed name */ - - if (!CHECK_LEN(header, p, plen, hash_len)) - return 0; - + if (digest_len == base32_len && hash_len == base32_len) { int rc = memcmp(workspace2, digest, digest_len); @@ -1392,7 +1396,8 @@ static int check_nsec3_coverage(struct dns_header *header, size_t plen, int dige if (rc == 0) { /* We found an NSEC3 whose hashed name exactly matches the query, so - we just need to check the type map. p points to the RR data for the record. */ + we just need to check the type map. p points to the RR data for the record. + Note we have packet length up to rdlen bytes checked. */ int offset = (type & 0xff) >> 3; int mask = 0x80 >> (type & 0x07); @@ -1400,15 +1405,12 @@ static int check_nsec3_coverage(struct dns_header *header, size_t plen, int dige p += hash_len; /* skip next-domain hash */ rdlen -= p - psave; - if (!CHECK_LEN(header, p, plen, rdlen)) - return 0; - if (rdlen >= 2 && p[0] == 0) { /* If we can prove that there's no NS record, return that information. */ if (nons && (p[2] & (0x80 >> T_NS)) != 0) *nons = 0; - + /* A CNAME answer would also be valid, so if there's a CNAME is should have been returned. */ if ((p[2] & (0x80 >> T_CNAME)) != 0) @@ -1511,9 +1513,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns salt_len = *p++; salt = p; - if (!CHECK_LEN(header, salt, plen, salt_len)) - return DNSSEC_FAIL_BADPACKET; /* bad packet */ - + /* Now prune so we only have NSEC3 records with same iterations, salt and algo */ for (i = 0; i < nsec_count; i++) { @@ -1543,9 +1543,6 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns if (salt_len != *p++) continue; - if (!CHECK_LEN(header, p, plen, salt_len)) - return DNSSEC_FAIL_BADPACKET; /* bad packet */ - if (memcmp(p, salt, salt_len) != 0) continue; @@ -1666,7 +1663,10 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key GETSHORT(class, p); GETLONG(ttl, p); GETSHORT(rdlen, p); - + + if (!CHECK_LEN(header, p, plen, rdlen)) + return DNSSEC_FAIL_BADPACKET; + if (class == qclass && (type == T_NSEC || type == T_NSEC3)) { if (nsec_ttl) @@ -1705,22 +1705,25 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key for (j = ntohs(header->nscount); j != 0; j--) { + unsigned char *psav; + if (!(res = extract_name(header, plen, &p1, daemon->workspacename, 0, 10))) return DNSSEC_FAIL_BADPACKET; - + GETSHORT(type1, p1); GETSHORT(class1, p1); p1 += 4; /* TTL */ GETSHORT(rdlen1, p1); + psav = p1; + if (!CHECK_LEN(header, p1, plen, rdlen1)) return DNSSEC_FAIL_BADPACKET; if (res == 1 && class1 == qclass && type1 == T_RRSIG) { int type_covered; - unsigned char *psav = p1; - + if (rdlen1 < 18) return DNSSEC_FAIL_BADPACKET; /* bad packet */ @@ -1735,24 +1738,47 @@ static int prove_non_existence(struct dns_header *header, size_t plen, char *key rrsig_labels[nsecs_found] = p1; else if (*rrsig_labels[nsecs_found] != *p1) /* algo */ return DNSSEC_FAIL_NONSEC; - } - p1 = psav; + } } - if (!ADD_RDLEN(header, p1, plen, rdlen1)) - return DNSSEC_FAIL_BADPACKET; + p1 = psav + rdlen1; } /* Must have found at least one sig. */ if (!rrsig_labels[nsecs_found]) return DNSSEC_FAIL_NONSEC; } + else if (type == T_NSEC3) + { + /* Decode the packet structure enough to check that rdlen is big enough + to contain everything other than the type bitmap. + (packet checked to be long enough to contain rdlen above) + We don't need to do any further length checks in check_nes3_coverage() + or prove_non_existence_nsec3() */ + + int salt_len, hash_len; + unsigned char *psav = p; + + if (rdlen < 5) + return DNSSEC_FAIL_BADPACKET; + + p += 4; /* algo, flags, iterations */ + salt_len = *p++; /* salt_len */ + if (rdlen < (6 + salt_len)) + return DNSSEC_FAIL_BADPACKET; /* check up to hash_length */ + + p += salt_len; /* salt */ + hash_len = *p++; + if (rdlen < (6 + salt_len + hash_len)) + return DNSSEC_FAIL_BADPACKET; /* check to end of next hashed name */ + + p = psav; + } nsecset[nsecs_found++] = pstart; } - if (!ADD_RDLEN(header, p, plen, rdlen)) - return DNSSEC_FAIL_BADPACKET; + p += rdlen; } if (type_found == T_NSEC) diff --git a/src/metrics.c b/src/metrics.c index da40614..e59e762 100644 --- a/src/metrics.c +++ b/src/metrics.c @@ -24,7 +24,9 @@ const char * metric_names[] = { "dns_local_answered", "dns_stale_answered", "dns_unanswered", - "max_crypto_use", + "dnssec_max_crypto_use", + "dnssec_max_sig_fail", + "dnssec_max_work", "bootp", "pxe", "dhcp_ack", diff --git a/src/metrics.h b/src/metrics.h index 3ca2173..cd85e53 100644 --- a/src/metrics.h +++ b/src/metrics.h @@ -23,7 +23,9 @@ enum { METRIC_DNS_LOCAL_ANSWERED, METRIC_DNS_STALE_ANSWERED, METRIC_DNS_UNANSWERED_QUERY, - METRIC_CRYTO_HWM, + METRIC_CRYTO_HWM, + METRIC_SIG_FAIL_HWM, + METRIC_WORK_HWM, METRIC_BOOTP, METRIC_PXE, METRIC_DHCPACK, From 3c91bca9432e68c21123d374ad54d6f107fa6761 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sat, 6 Jan 2024 16:13:44 +0000 Subject: [PATCH 10/13] Better stats and logging from DNSSEC resource limiting. --- src/cache.c | 37 +++++++++++++++++++-------- src/config.h | 4 +-- src/dnssec.c | 10 +++++--- src/forward.c | 69 ++++++++++++++++++++++++++++++++------------------- src/metrics.h | 2 +- 5 files changed, 80 insertions(+), 42 deletions(-) diff --git a/src/cache.c b/src/cache.c index 33dd929..0eacec9 100644 --- a/src/cache.c +++ b/src/cache.c @@ -835,12 +835,17 @@ void cache_end_insert(void) { ssize_t m = -1; -#ifdef HAVE_DNSSEC - /* Sneak out possibly updated crypto HWM. */ - m = -1 - daemon->metrics[METRIC_CRYTO_HWM]; -#endif - read_write(daemon->pipe_to_parent, (unsigned char *)&m, sizeof(m), 0); + +#ifdef HAVE_DNSSEC + /* Sneak out possibly updated crypto HWM values. */ + m = daemon->metrics[METRIC_CRYPTO_HWM]; + read_write(daemon->pipe_to_parent, (unsigned char *)&m, sizeof(m), 0); + m = daemon->metrics[METRIC_SIG_FAIL_HWM]; + read_write(daemon->pipe_to_parent, (unsigned char *)&m, sizeof(m), 0); + m = daemon->metrics[METRIC_WORK_HWM]; + read_write(daemon->pipe_to_parent, (unsigned char *)&m, sizeof(m), 0); +#endif } new_chain = NULL; @@ -859,18 +864,28 @@ int cache_recv_insert(time_t now, int fd) cache_start_insert(); - while(1) + while (1) { if (!read_write(fd, (unsigned char *)&m, sizeof(m), 1)) return 0; - if (m < 0) + if (m == -1) { #ifdef HAVE_DNSSEC /* Sneak in possibly updated crypto HWM. */ - if ((-m - 1) > daemon->metrics[METRIC_CRYTO_HWM]) - daemon->metrics[METRIC_CRYTO_HWM] = -m - 1; + if (!read_write(fd, (unsigned char *)&m, sizeof(m), 1)) + return 0; + if (m > daemon->metrics[METRIC_CRYPTO_HWM]) + daemon->metrics[METRIC_CRYPTO_HWM] = m; + if (!read_write(fd, (unsigned char *)&m, sizeof(m), 1)) + return 0; + if (m > daemon->metrics[METRIC_SIG_FAIL_HWM]) + daemon->metrics[METRIC_SIG_FAIL_HWM] = m; + if (!read_write(fd, (unsigned char *)&m, sizeof(m), 1)) + return 0; + if (m > daemon->metrics[METRIC_WORK_HWM]) + daemon->metrics[METRIC_WORK_HWM] = m; #endif cache_end_insert(); return 1; @@ -1905,7 +1920,9 @@ void dump_cache(time_t now) my_syslog(LOG_INFO, _("queries for authoritative zones %u"), daemon->metrics[METRIC_DNS_AUTH_ANSWERED]); #endif #ifdef HAVE_DNSSEC - my_syslog(LOG_INFO, _("DNSSEC per-query crypto HWM %u"), daemon->metrics[METRIC_CRYTO_HWM]); + my_syslog(LOG_INFO, _("DNSSEC per-query subqueries HWM %u"), daemon->metrics[METRIC_WORK_HWM]); + my_syslog(LOG_INFO, _("DNSSEC per-query crypto work HWM %u"), daemon->metrics[METRIC_CRYPTO_HWM]); + my_syslog(LOG_INFO, _("DNSSEC per-RRSet signature fails HWM %u"), daemon->metrics[METRIC_SIG_FAIL_HWM]); #endif blockdata_report(); diff --git a/src/config.h b/src/config.h index 5c0fa1f..d137d90 100644 --- a/src/config.h +++ b/src/config.h @@ -24,8 +24,8 @@ #define KEYBLOCK_LEN 40 /* choose to minimise fragmentation when storing DNSSEC keys */ #define DNSSEC_WORK 50 /* Max number of queries to validate one question */ #define LIMIT_SIG_FAIL 20 /* Number of signature that can fail to validate in one answer */ -#define LIMIT_CRYPTO 200 /* max no. of crypto operations to validate one a query. */ -#define LIMIT_NSEC3_ITERS 150 /* Max. number if iterations allow in NSEC3 record. */ +#define LIMIT_CRYPTO 200 /* max no. of crypto operations to validate one query. */ +#define LIMIT_NSEC3_ITERS 150 /* Max. number if iterations allowed in NSEC3 record. */ #define TIMEOUT 10 /* drop UDP queries after TIMEOUT seconds */ #define SMALL_PORT_RANGE 30 /* If DNS port range is smaller than this, use different allocation. */ #define FORWARD_TEST 50 /* try all servers every 50 queries */ diff --git a/src/dnssec.c b/src/dnssec.c index c3246cb..4401908 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -428,7 +428,7 @@ int dec_counter(int *counter, char *message) { if ((*counter)-- == 0) { - my_syslog(LOG_WARNING, "limit exceeded: %s", message ? message : "crypto work"); + my_syslog(LOG_WARNING, "limit exceeded: %s", message ? message : _("per-query crypto work")); return 1; } @@ -686,14 +686,16 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in if (dec_counter(validate_counter, NULL)) return STAT_ABANDONED; - if (verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) + if (verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo)) return (labels < name_labels) ? STAT_SECURE_WILDCARD : STAT_SECURE; /* An attacker can waste a lot of our CPU by setting up a giant DNSKEY RRSET full of failing keys, all of which we have to try. Since many failing keys is not likely for a legitimate domain, set a limit on how many can fail. */ - if (dec_counter(&sig_fail_cnt, "SIG fail")) - return STAT_ABANDONED; + if ((daemon->limit_sig_fail - (sig_fail_cnt + 1)) > (int)daemon->metrics[METRIC_SIG_FAIL_HWM]) + daemon->metrics[METRIC_SIG_FAIL_HWM] = daemon->limit_sig_fail - (sig_fail_cnt + 1); + if (dec_counter(&sig_fail_cnt, _("per-RRSet signature fails"))) + return STAT_ABANDONED; } } } diff --git a/src/forward.c b/src/forward.c index c247877..02a4630 100644 --- a/src/forward.c +++ b/src/forward.c @@ -894,6 +894,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, ssize_t plen, int status, time_t now) { struct frec *orig; + int log_resource = 0; daemon->log_display_id = forward->frec_src.log_id; @@ -936,16 +937,10 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, status = dnssec_validate_reply(now, header, plen, daemon->namebuff, daemon->keyname, &forward->class, !option_bool(OPT_DNSSEC_IGN_NS) && (forward->sentto->flags & SERV_DO_DNSSEC), NULL, NULL, NULL, &orig->validate_counter); - - if (STAT_ISEQUAL(status, STAT_ABANDONED)) - { - /* Log the actual validation that made us barf. */ - unsigned char *p = (unsigned char *)(header+1); - if (extract_name(header, plen, &p, daemon->namebuff, 0, 4) == 1) - my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), - daemon->namebuff[0] ? daemon->namebuff : "."); - } } + + if (STAT_ISEQUAL(status, STAT_ABANDONED)) + log_resource = 1; /* Can't validate, as we're missing key data. Put this answer aside, whilst we get that. */ @@ -989,6 +984,11 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, return; } } + else if (orig->work_counter-- == 0) + { + my_syslog(LOG_WARNING, _("limit exceeded: per-query subqueries")); + log_resource = 1; + } else { struct server *server; @@ -1005,7 +1005,6 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, daemon->keyname, forward->class, STAT_ISEQUAL(status, STAT_NEED_KEY) ? T_DNSKEY : T_DS, server->edns_pktsz)) && (hash = hash_questions(header, nn, daemon->namebuff)) && - --orig->work_counter != 0 && (fd = allocate_rfd(&rfds, server)) != -1 && (new = get_new_frec(now, server, 1))) { @@ -1071,6 +1070,15 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, status = STAT_ABANDONED; } + if (log_resource) + { + /* Log the actual validation that made us barf. */ + unsigned char *p = (unsigned char *)(header+1); + if (extract_name(header, plen, &p, daemon->namebuff, 0, 4) == 1) + my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), + daemon->namebuff[0] ? daemon->namebuff : "."); + } + #ifdef HAVE_DUMPFILE if (STAT_ISEQUAL(status, STAT_BOGUS) || STAT_ISEQUAL(status, STAT_ABANDONED)) dump_packet_udp((forward->flags & (FREC_DNSKEY_QUERY | FREC_DS_QUERY)) ? DUMP_SEC_BOGUS : DUMP_BOGUS, @@ -1350,8 +1358,11 @@ static void return_reply(time_t now, struct frec *forward, struct dns_header *he } } - if ((daemon->limit_crypto - forward->validate_counter) > (int)daemon->metrics[METRIC_CRYTO_HWM]) - daemon->metrics[METRIC_CRYTO_HWM] = daemon->limit_crypto - forward->validate_counter; + if ((daemon->limit_crypto - forward->validate_counter) > (int)daemon->metrics[METRIC_CRYPTO_HWM]) + daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit_crypto - forward->validate_counter; + + if ((daemon->limit_work - forward->work_counter) > (int)daemon->metrics[METRIC_WORK_HWM]) + daemon->metrics[METRIC_WORK_HWM] = daemon->limit_work - forward->work_counter; #endif if (option_bool(OPT_NO_REBIND)) @@ -2045,9 +2056,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si int log_save; /* limit the amount of work we do, to avoid cycling forever on loops in the DNS */ - if (--(*keycount) == 0) - new_status = STAT_ABANDONED; - else if (STAT_ISEQUAL(status, STAT_NEED_KEY)) + if (STAT_ISEQUAL(status, STAT_NEED_KEY)) new_status = dnssec_validate_by_ds(now, header, n, name, keyname, class, validatecount); else if (STAT_ISEQUAL(status, STAT_NEED_DS)) new_status = dnssec_validate_ds(now, header, n, name, keyname, class, validatecount); @@ -2056,6 +2065,15 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si !option_bool(OPT_DNSSEC_IGN_NS) && (server->flags & SERV_DO_DNSSEC), NULL, NULL, NULL, validatecount); + if (!STAT_ISEQUAL(new_status, STAT_NEED_DS) && !STAT_ISEQUAL(new_status, STAT_NEED_KEY) && !STAT_ISEQUAL(new_status, STAT_ABANDONED)) + break; + + if ((*keycount)-- == 0) + { + my_syslog(LOG_WARNING, _("limit exceeded: per-query subqueries")); + new_status = STAT_ABANDONED; + } + if (STAT_ISEQUAL(new_status, STAT_ABANDONED)) { /* Log the actual validation that made us barf. */ @@ -2063,11 +2081,9 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si if (extract_name(header, n, &p, daemon->namebuff, 0, 4) == 1) my_syslog(LOG_WARNING, _("validation of %s failed: resource limit exceeded."), daemon->namebuff[0] ? daemon->namebuff : "."); + break; } - - if (!STAT_ISEQUAL(new_status, STAT_NEED_DS) && !STAT_ISEQUAL(new_status, STAT_NEED_KEY)) - break; - + /* Can't validate because we need a key/DS whose name now in keyname. Make query for same, and recurse to validate */ if (!packet) @@ -2081,7 +2097,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si new_status = STAT_ABANDONED; break; } - + m = dnssec_generate_query(new_header, ((unsigned char *) new_header) + 65536, keyname, class, STAT_ISEQUAL(new_status, STAT_NEED_KEY) ? T_DNSKEY : T_DS, server->edns_pktsz); @@ -2096,11 +2112,11 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si daemon->log_display_id = ++daemon->log_id; log_query_mysockaddr(F_NOEXTRA | F_DNSSEC | F_SERVER, keyname, &server->addr, - STAT_ISEQUAL(new_status, STAT_NEED_KEY) ? "dnssec-query[DNSKEY]" : "dnssec-query[DS]", 0); - + STAT_ISEQUAL(new_status, STAT_NEED_KEY) ? "dnssec-query[DNSKEY]" : "dnssec-query[DS]", 0); + new_status = tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, have_mark, mark, keycount, validatecount); - + daemon->log_display_id = log_save; if (!STAT_ISEQUAL(new_status, STAT_OK)) @@ -2434,8 +2450,11 @@ unsigned char *tcp_request(int confd, time_t now, log_query(F_SECSTAT, domain, &a, result, 0); - if ((daemon->limit_crypto - validatecount) > (int)daemon->metrics[METRIC_CRYTO_HWM]) - daemon->metrics[METRIC_CRYTO_HWM] = daemon->limit_crypto - validatecount; + if ((daemon->limit_crypto - validatecount) > (int)daemon->metrics[METRIC_CRYPTO_HWM]) + daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit_crypto - validatecount; + + if ((daemon->limit_work - keycount) > (int)daemon->metrics[METRIC_WORK_HWM]) + daemon->metrics[METRIC_WORK_HWM] = daemon->limit_work - keycount; } #endif diff --git a/src/metrics.h b/src/metrics.h index cd85e53..67cb3bf 100644 --- a/src/metrics.h +++ b/src/metrics.h @@ -23,7 +23,7 @@ enum { METRIC_DNS_LOCAL_ANSWERED, METRIC_DNS_STALE_ANSWERED, METRIC_DNS_UNANSWERED_QUERY, - METRIC_CRYTO_HWM, + METRIC_CRYPTO_HWM, METRIC_SIG_FAIL_HWM, METRIC_WORK_HWM, METRIC_BOOTP, From 39de57499e5e06c177392045c27d3a77b3ae39da Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sat, 6 Jan 2024 20:51:13 +0000 Subject: [PATCH 11/13] Better allocation code for DS digest cache. --- src/dnssec.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index 4401908..a3e60ba 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -724,7 +724,8 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch union all_addr a; int failflags = DNSSEC_FAIL_NODSSUP | DNSSEC_FAIL_NOZONE; char valid_digest[255]; - static unsigned char *cached_digest[255]; + static unsigned char **cached_digest; + static size_t cached_digest_size = 0; if (ntohs(header->qdcount) != 1 || RCODE(header) != NOERROR || !extract_name(header, plen, &p, name, 1, 4)) return STAT_BOGUS | DNSSEC_FAIL_NOKEY; @@ -839,14 +840,35 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch hash->digest(ctx, hash->digest_size, digest); from_wire(name); - - if (!cached_digest[recp1->addr.ds.digest]) - cached_digest[recp1->addr.ds.digest] = whine_malloc(recp1->addr.ds.keylen); - - if (cached_digest[recp1->addr.ds.digest]) + + if (recp1->addr.ds.digest >= cached_digest_size) { - memcpy(cached_digest[recp1->addr.ds.digest], digest, recp1->addr.ds.keylen); - valid_digest[recp1->addr.ds.digest] = 1; + unsigned char **new; + + /* whine_malloc zeros memory */ + if ((new = whine_malloc((recp1->addr.ds.digest + 5) * sizeof(unsigned char *)))) + { + if (cached_digest_size != 0) + { + memcpy(new, cached_digest, cached_digest_size * sizeof(unsigned char *)); + free(cached_digest); + } + + cached_digest_size = recp1->addr.ds.digest + 5; + cached_digest = new; + } + } + + if (recp1->addr.ds.digest < cached_digest_size) + { + if (!cached_digest[recp1->addr.ds.digest]) + cached_digest[recp1->addr.ds.digest] = whine_malloc(recp1->addr.ds.keylen); + + if (cached_digest[recp1->addr.ds.digest]) + { + memcpy(cached_digest[recp1->addr.ds.digest], digest, recp1->addr.ds.keylen); + valid_digest[recp1->addr.ds.digest] = 1; + } } } From 3ae7f1ab0d847b17fdc95fd0e5b3120f66dc3af9 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 7 Jan 2024 22:47:30 +0000 Subject: [PATCH 12/13] Add --dnssec-limits option. --- man/dnsmasq.8 | 9 +++++++++ src/config.h | 8 ++++---- src/dnsmasq.h | 8 +++++++- src/dnssec.c | 8 ++++---- src/forward.c | 24 ++++++++++++------------ src/option.c | 29 +++++++++++++++++++++++++---- 6 files changed, 61 insertions(+), 25 deletions(-) diff --git a/man/dnsmasq.8 b/man/dnsmasq.8 index 80b3a38..32bdeff 100644 --- a/man/dnsmasq.8 +++ b/man/dnsmasq.8 @@ -931,6 +931,15 @@ Authenticated Data bit correctly in all cases is not technically possible. If th when using this option, then the cache should be disabled using --cache-size=0. In most cases, enabling DNSSEC validation within dnsmasq is a better option. See --dnssec for details. .TP +.B --dnssec-limits=[,.......] +Override the default resource limits applied to DNSSEC validation. Cryptographic operations are expensive and crafted domains +can DoS a DNSSEC validator by forcing it to do hundreds of thousands of such operations. To avoid this, the dnsmasq validation code +applies limits on how much work will be expended in validation. If any of the limits are exceeded, the validation will fail and the +domain treated as BOGUS. There are four limits, in order(default values in parens): number a signature validation fails per RRset(20), number of signature validations and +hash computations per query(200), number of sub-queries to fetch DS and DNSKEY RRsets per query(40), and the number of iterations in a NSEC3 record(150). +The maximum values reached during validation are stored, and dumped as part of the stats generated by SIGUSR1. Supplying a limit value of 0 leaves the default in place, so +\fB--dnssec-limits=0,0,20\fP sets the number of sub-queries to 20 whilst leaving the other limits at default values. +.TP .B --dnssec-debug Set debugging mode for the DNSSEC validation, set the Checking Disabled bit on upstream queries, and don't convert replies which do not validate to responses with diff --git a/src/config.h b/src/config.h index d137d90..e722e98 100644 --- a/src/config.h +++ b/src/config.h @@ -22,10 +22,10 @@ #define EDNS_PKTSZ 1232 /* default max EDNS.0 UDP packet from from /dnsflagday.net/2020 */ #define SAFE_PKTSZ 1232 /* "go anywhere" UDP packet size, see https://dnsflagday.net/2020/ */ #define KEYBLOCK_LEN 40 /* choose to minimise fragmentation when storing DNSSEC keys */ -#define DNSSEC_WORK 50 /* Max number of queries to validate one question */ -#define LIMIT_SIG_FAIL 20 /* Number of signature that can fail to validate in one answer */ -#define LIMIT_CRYPTO 200 /* max no. of crypto operations to validate one query. */ -#define LIMIT_NSEC3_ITERS 150 /* Max. number if iterations allowed in NSEC3 record. */ +#define DNSSEC_LIMIT_WORK 40 /* Max number of queries to validate one question */ +#define DNSSEC_LIMIT_SIG_FAIL 20 /* Number of signature that can fail to validate in one answer */ +#define DNSSEC_LIMIT_CRYPTO 200 /* max no. of crypto operations to validate one query. */ +#define DNSSEC_LIMIT_NSEC3_ITERS 150 /* Max. number if iterations allowed in NSEC3 record. */ #define TIMEOUT 10 /* drop UDP queries after TIMEOUT seconds */ #define SMALL_PORT_RANGE 30 /* If DNS port range is smaller than this, use different allocation. */ #define FORWARD_TEST 50 /* try all servers every 50 queries */ diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 704bc63..e455c3f 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -834,6 +834,12 @@ struct frec { #define LEASE_HAVE_HWADDR 128 /* Have set hwaddress */ #define LEASE_EXP_CHANGED 256 /* Lease expiry time changed */ +#define LIMIT_SIG_FAIL 0 +#define LIMIT_CRYPTO 1 +#define LIMIT_WORK 2 +#define LIMIT_NSEC3_ITERS 3 +#define LIMIT_MAX 4 + struct dhcp_lease { int clid_len; /* length of client identifier */ unsigned char *clid; /* clientid */ @@ -1243,7 +1249,7 @@ extern struct daemon { int rr_status_sz; int dnssec_no_time_check; int back_to_the_future; - int limit_sig_fail, limit_crypto, limit_work, limit_nsec3_iters; + int limit[LIMIT_MAX]; #endif struct frec *frec_list; struct frec_src *free_frec_src; diff --git a/src/dnssec.c b/src/dnssec.c index a3e60ba..ed2f53f 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -479,7 +479,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in rrsetidx = sort_rrset(header, plen, rr_desc, rrsetidx, rrset, daemon->workspacename, keyname); /* Now try all the sigs to try and find one which validates */ - for (sig_fail_cnt = daemon->limit_sig_fail, j = 0; j limit[LIMIT_SIG_FAIL], j = 0; j limit_sig_fail - (sig_fail_cnt + 1)) > (int)daemon->metrics[METRIC_SIG_FAIL_HWM]) - daemon->metrics[METRIC_SIG_FAIL_HWM] = daemon->limit_sig_fail - (sig_fail_cnt + 1); + if ((daemon->limit[LIMIT_SIG_FAIL] - (sig_fail_cnt + 1)) > (int)daemon->metrics[METRIC_SIG_FAIL_HWM]) + daemon->metrics[METRIC_SIG_FAIL_HWM] = daemon->limit[LIMIT_SIG_FAIL] - (sig_fail_cnt + 1); if (dec_counter(&sig_fail_cnt, _("per-RRSet signature fails"))) return STAT_ABANDONED; } @@ -1532,7 +1532,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns GETSHORT (iterations, p); /* Upper-bound iterations, to avoid DoS. RFC 9276 refers. */ - if (iterations > daemon->limit_nsec3_iters) + if (iterations > daemon->limit[LIMIT_NSEC3_ITERS]) return DNSSEC_FAIL_NSEC3_ITERS; salt_len = *p++; diff --git a/src/forward.c b/src/forward.c index 02a4630..32f37e4 100644 --- a/src/forward.c +++ b/src/forward.c @@ -338,8 +338,8 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, if (ad_reqd) forward->flags |= FREC_AD_QUESTION; #ifdef HAVE_DNSSEC - forward->work_counter = daemon->limit_work; - forward->validate_counter = daemon->limit_crypto; + forward->work_counter = daemon->limit[LIMIT_WORK]; + forward->validate_counter = daemon->limit[LIMIT_CRYPTO]; if (do_bit) forward->flags |= FREC_DO_QUESTION; #endif @@ -1358,11 +1358,11 @@ static void return_reply(time_t now, struct frec *forward, struct dns_header *he } } - if ((daemon->limit_crypto - forward->validate_counter) > (int)daemon->metrics[METRIC_CRYPTO_HWM]) - daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit_crypto - forward->validate_counter; + if ((daemon->limit[LIMIT_CRYPTO] - forward->validate_counter) > (int)daemon->metrics[METRIC_CRYPTO_HWM]) + daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit[LIMIT_CRYPTO] - forward->validate_counter; - if ((daemon->limit_work - forward->work_counter) > (int)daemon->metrics[METRIC_WORK_HWM]) - daemon->metrics[METRIC_WORK_HWM] = daemon->limit_work - forward->work_counter; + if ((daemon->limit[LIMIT_WORK] - forward->work_counter) > (int)daemon->metrics[METRIC_WORK_HWM]) + daemon->metrics[METRIC_WORK_HWM] = daemon->limit[LIMIT_WORK] - forward->work_counter; #endif if (option_bool(OPT_NO_REBIND)) @@ -2420,8 +2420,8 @@ unsigned char *tcp_request(int confd, time_t now, #ifdef HAVE_DNSSEC if (option_bool(OPT_DNSSEC_VALID) && !checking_disabled && (master->flags & SERV_DO_DNSSEC)) { - int keycount = daemon->limit_work; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */ - int validatecount = daemon->limit_crypto; + int keycount = daemon->limit[LIMIT_WORK]; /* Limit to number of DNSSEC questions, to catch loops and avoid filling cache. */ + int validatecount = daemon->limit[LIMIT_CRYPTO]; int status = tcp_key_recurse(now, STAT_OK, header, m, 0, daemon->namebuff, daemon->keyname, serv, have_mark, mark, &keycount, &validatecount); char *result, *domain = "result"; @@ -2450,11 +2450,11 @@ unsigned char *tcp_request(int confd, time_t now, log_query(F_SECSTAT, domain, &a, result, 0); - if ((daemon->limit_crypto - validatecount) > (int)daemon->metrics[METRIC_CRYPTO_HWM]) - daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit_crypto - validatecount; + if ((daemon->limit[LIMIT_CRYPTO] - validatecount) > (int)daemon->metrics[METRIC_CRYPTO_HWM]) + daemon->metrics[METRIC_CRYPTO_HWM] = daemon->limit[LIMIT_CRYPTO] - validatecount; - if ((daemon->limit_work - keycount) > (int)daemon->metrics[METRIC_WORK_HWM]) - daemon->metrics[METRIC_WORK_HWM] = daemon->limit_work - keycount; + if ((daemon->limit[LIMIT_WORK] - keycount) > (int)daemon->metrics[METRIC_WORK_HWM]) + daemon->metrics[METRIC_WORK_HWM] = daemon->limit[LIMIT_WORK] - keycount; } #endif diff --git a/src/option.c b/src/option.c index 1c20766..f4ff7c0 100644 --- a/src/option.c +++ b/src/option.c @@ -191,6 +191,7 @@ struct myoption { #define LOPT_NO_DHCP6 382 #define LOPT_NO_DHCP4 383 #define LOPT_MAX_PROCS 384 +#define LOPT_DNSSEC_LIMITS 385 #ifdef HAVE_GETOPT_LONG static const struct option opts[] = @@ -364,6 +365,7 @@ static const struct myoption opts[] = { "dnssec-check-unsigned", 2, 0, LOPT_DNSSEC_CHECK }, { "dnssec-no-timecheck", 0, 0, LOPT_DNSSEC_TIME }, { "dnssec-timestamp", 1, 0, LOPT_DNSSEC_STAMP }, + { "dnssec-limits", 1, 0, LOPT_DNSSEC_LIMITS }, { "dhcp-relay", 1, 0, LOPT_RELAY }, { "ra-param", 1, 0, LOPT_RA_PARAM }, { "quiet-dhcp", 0, 0, LOPT_QUIET_DHCP }, @@ -568,6 +570,7 @@ static struct { { LOPT_DNSSEC_CHECK, ARG_DUP, NULL, gettext_noop("Ensure answers without DNSSEC are in unsigned zones."), NULL }, { LOPT_DNSSEC_TIME, OPT_DNSSEC_TIME, NULL, gettext_noop("Don't check DNSSEC signature timestamps until first cache-reload"), NULL }, { LOPT_DNSSEC_STAMP, ARG_ONE, "", gettext_noop("Timestamp file to verify system clock for DNSSEC"), NULL }, + { LOPT_DNSSEC_LIMITS, ARG_ONE, ",..", gettext_noop("Set resource limits for DNSSEC validation"), NULL }, { LOPT_RA_PARAM, ARG_DUP, ",[mtu:||off,][,][,]", gettext_noop("Set MTU, priority, resend-interval and router-lifetime"), NULL }, { LOPT_QUIET_DHCP, OPT_QUIET_DHCP, NULL, gettext_noop("Do not log routine DHCP."), NULL }, { LOPT_QUIET_DHCP6, OPT_QUIET_DHCP6, NULL, gettext_noop("Do not log routine DHCPv6."), NULL }, @@ -5258,6 +5261,24 @@ err: } #ifdef HAVE_DNSSEC + case LOPT_DNSSEC_LIMITS: + { + int lim, val; + + for (lim = LIMIT_SIG_FAIL; arg && lim < LIMIT_MAX ; lim++, arg = comma) + { + comma = split(arg); + + if (!atoi_check(arg, &val)) + ret_err(gen_err); + + if (val != 0) + daemon->limit[lim] = val; + } + + break; + } + case LOPT_DNSSEC_STAMP: /* --dnssec-timestamp */ daemon->timestamp_file = opt_string_alloc(arg); break; @@ -5870,10 +5891,10 @@ void read_opts(int argc, char **argv, char *compile_opts) daemon->host_index = SRC_AH; daemon->max_procs = MAX_PROCS; #ifdef HAVE_DNSSEC - daemon->limit_sig_fail = LIMIT_SIG_FAIL; - daemon->limit_crypto = LIMIT_CRYPTO; - daemon->limit_work = DNSSEC_WORK; - daemon->limit_nsec3_iters = LIMIT_NSEC3_ITERS; + daemon->limit[LIMIT_SIG_FAIL] = DNSSEC_LIMIT_SIG_FAIL; + daemon->limit[LIMIT_CRYPTO] = DNSSEC_LIMIT_CRYPTO; + daemon->limit[LIMIT_WORK] = DNSSEC_LIMIT_WORK; + daemon->limit[LIMIT_NSEC3_ITERS] = DNSSEC_LIMIT_NSEC3_ITERS; #endif /* See comment above make_servers(). Optimises server-read code. */ From 9621c16a788cb6eeae1388148f6ea3ba0e8e6d79 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Mon, 12 Feb 2024 22:07:33 +0000 Subject: [PATCH 13/13] Add CHANGELOG entry for DNSSEC security fixes. --- CHANGELOG | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 2d46ae4..713b785 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -32,6 +32,38 @@ version 2.90 --filter-rr=ANY has a special meaning: it filters the answers to queries for the ANY RR-type. + Add limits on the resources used to do DNSSEC validation. + DNSSEC introduces a potential CPU DoS, because a crafted domain + can force a validator to a large number of cryptographic + operations whilst attempting to do validation. When using TCP + transport a DNSKEY RRset contain thousands of members and any + RRset can have thousands of signatures. The potential number + of signature validations to follow the RFC for validation + for one RRset is the cross product of the keys and signatures, + so millions. In practice, the actual numbers are much lower, + so attacks can be mitigated by limiting the amount of + cryptographic "work" to a much lower amount. The actual + limits are number a signature validation fails per RRset(20), + number of signature validations and hash computations + per query(200), number of sub-queries to fetch DS and DNSKEY + RRsets per query(40), and the number of iterations in a + NSEC3 record(150). These values are sensible, but there is, as yet, + no standardisation on the values for a "conforming" domain, so a + new option --dnssec-limit is provided should they need to be altered. + The algorithm to validate DS records has also been altered to reduce + the maximum work from cross product of the number of DS records and + number of DNSKEYs to the cross product of the number of DS records + and supported DS digest types. As the number of DS digest types + is in single figures, this reduces the exposure. + + Credit is due to Elias Heftrig, Haya Schulmann, Niklas Vogel, + and Michael Waidner from the German National Research Center for + Applied Cybersecurity ATHENE for finding this vulnerability. + + CVE 2023-50387 and CVE 2023-50868 apply. + Note that the is a security vulnerablity only when DNSSEC validation + is enabled. + version 2.89 Fix bug introduced in 2.88 (commit fe91134b) which can result