Skip to content

Commit

Permalink
Better stats and logging from DNSSEC resource limiting.
Browse files Browse the repository at this point in the history
Signed-off-by: DL6ER <dl6er@dl6er.de>
  • Loading branch information
simonkelley authored and DL6ER committed Feb 13, 2024
1 parent c32b467 commit a389bcc
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 41 deletions.
35 changes: 26 additions & 9 deletions src/dnsmasq/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -851,12 +851,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;
Expand All @@ -875,18 +880,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;
Expand Down Expand Up @@ -1953,7 +1968,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();
Expand Down
4 changes: 2 additions & 2 deletions src/dnsmasq/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 1000 /* try all servers every 1000 queries */
Expand Down
10 changes: 6 additions & 4 deletions src/dnsmasq/dnssec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
}
}
Expand Down
69 changes: 44 additions & 25 deletions src/dnsmasq/forward.c
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,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;

Expand Down Expand Up @@ -980,16 +981,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. */
Expand Down Expand Up @@ -1033,6 +1028,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;
Expand All @@ -1049,7 +1049,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)))
{
Expand Down Expand Up @@ -1115,6 +1114,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,
Expand Down Expand Up @@ -1396,8 +1404,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))
Expand Down Expand Up @@ -2141,9 +2152,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);
Expand All @@ -2152,18 +2161,25 @@ 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. */
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 : ".");
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)
Expand All @@ -2177,7 +2193,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);

Expand All @@ -2192,11 +2208,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))
Expand Down Expand Up @@ -2568,8 +2584,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

Expand Down
2 changes: 1 addition & 1 deletion src/dnsmasq/metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit a389bcc

Please sign in to comment.