From c5aa221e44a4c5724434ddfca0d4ca38c9594c73 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Tue, 2 Jan 2024 21:43:04 +0000 Subject: [PATCH] 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);