Hi!I have raised filtering topic on DNS-OARC chat. One of proposals were to mark at least filtered records by EDE status, which current dnsmasq supports already. I like it. We create fake answer on when --filter-A or --filter-AAAA options is used. It should be marked somehow.
There is also proposal for more verbose error and contact information [1], but at least marking the response somehow synthetized is a good start. I attached a change to rrfilter to report number of modified records. Then it marks any filtered response with Filtered EDE code. I expect the same should be possible for any other record type filtered, except EDNS0 and DNSSEC records.
Credits for the idea goes to Vladimír Čunát. It might allow potential DNSSEC validator to not emit SERVFAIL on bogus answer we made. If that would trust our response for any reason.
What do you think?By the way, maybe we should strip also RRSIG for those records if present. It looks like a bug to me. But would not make validating resolvers more happy anyway.
; <<>> DiG 9.18.12 <<>> -4 @localhost -p 2053 example.org a +dnssec ; (2 servers found) ;; global options: +cmd ;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 21029 ;; flags: qr rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 1 ;; OPT PSEUDOSECTION: ; EDNS: version: 0, flags: do; udp: 1220 ; COOKIE: b2ad85a9275d948e02176a79641381dce6990a257f089ec5 (good) ; EDE: 17 (Filtered) ;; QUESTION SECTION: ;example.org. IN A ;; ANSWER SECTION:example.org. 32748 IN RRSIG A 8 2 86400 20230323193411 20230302075235 43798 example.org. QwrK73kR5vStRzG6IPOpYU2exzSIOatl1p8DffKi4PP2Ig8yAL43AhVu 2bsA0I0EFINH3xvF2IiM7eyR/fMm8rfeAsG1pokOFOOhlYQQHhglgfu6 mgNJnFrHUs3M+JNBNyAay42aSSDt5gXcvk77nx32uWv40pfknU7wH2Xc rP4=
[1] https://datatracker.ietf.org/doc/draft-ietf-dnsop-structured-dns-error/ -- Petr Menšík Software Engineer, RHEL Red Hat, http://www.redhat.com/ PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From acb66a570a5e338a79160a8dd2b9e072ab8c5a81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Thu, 16 Mar 2023 21:15:40 +0100
Subject: [PATCH] Report number of modified records from rrfilter()
Set EDE_FILTERED when filter-A or filter-AAAA caused some change. Pass
length value by pointer instead returning its new value. It assigned
into the same variable in all uses anyway.
---
src/dnsmasq.h | 2 +-
src/edns0.c | 2 +-
src/forward.c | 12 ++++++++----
src/rrfilter.c | 46 ++++++++++++++++++++++++----------------------
4 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index fe9aa07..92cc291 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1811,7 +1811,7 @@ void poll_listen(int fd, short event);
int do_poll(int timeout);
/* rrfilter.c */
-size_t rrfilter(struct dns_header *header, size_t plen, int mode);
+size_t rrfilter(struct dns_header *header, size_t *plen, int mode);
u16 *rrfilter_desc(int type);
int expand_workspace(unsigned char ***wkspc, int *szp, int new);
/* modes. */
diff --git a/src/edns0.c b/src/edns0.c
index c498eb1..567101b 100644
--- a/src/edns0.c
+++ b/src/edns0.c
@@ -178,7 +178,7 @@ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *l
memcpy(buff, datap, rdlen);
/* now, delete OPT RR */
- plen = rrfilter(header, plen, RRFILTER_EDNS0);
+ rrfilter(header, &plen, RRFILTER_EDNS0);
/* Now, force addition of a new one */
p = NULL;
diff --git a/src/forward.c b/src/forward.c
index 0f03818..b95aa80 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -721,7 +721,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
if (added_pheader)
{
/* client didn't send EDNS0, we added one, strip it off before returning answer. */
- n = rrfilter(header, n, RRFILTER_EDNS0);
+ rrfilter(header, &n, RRFILTER_EDNS0);
pheader = NULL;
}
else
@@ -814,11 +814,15 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
/* Before extract_addresses() */
if (rcode == NOERROR)
{
+ size_t modified = 0;
if (option_bool(OPT_FILTER_A))
- n = rrfilter(header, n, RRFILTER_A);
+ modified = rrfilter(header, &n, RRFILTER_A);
if (option_bool(OPT_FILTER_AAAA))
- n = rrfilter(header, n, RRFILTER_AAAA);
+ modified += rrfilter(header, &n, RRFILTER_AAAA);
+
+ if (modified > 0)
+ ede = EDE_FILTERED;
}
switch (extract_addresses(header, n, daemon->namebuff, now, ipsets, nftsets, is_sign, check_rebind, no_cache, cache_secure, &doctored))
@@ -860,7 +864,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
/* If the requestor didn't set the DO bit, don't return DNSSEC info. */
if (!do_bit)
- n = rrfilter(header, n, RRFILTER_DNSSEC);
+ rrfilter(header, &n, RRFILTER_DNSSEC);
}
#endif
diff --git a/src/rrfilter.c b/src/rrfilter.c
index 42d9c21..3a5547a 100644
--- a/src/rrfilter.c
+++ b/src/rrfilter.c
@@ -156,41 +156,43 @@ static int check_rrs(unsigned char *p, struct dns_header *header, size_t plen, i
}
-/* mode may be remove EDNS0 or DNSSEC RRs or remove A or AAAA from answer section. */
-size_t rrfilter(struct dns_header *header, size_t plen, int mode)
+/* mode may be remove EDNS0 or DNSSEC RRs or remove A or AAAA from answer section.
+ * returns number of modified records. */
+size_t rrfilter(struct dns_header *header, size_t *plen, int mode)
{
static unsigned char **rrs = NULL;
static int rr_sz = 0;
unsigned char *p = (unsigned char *)(header+1);
- int i, rdlen, qtype, qclass, rr_found, chop_an, chop_ns, chop_ar;
+ size_t rr_found = 0;
+ int i, rdlen, qtype, qclass, chop_an, chop_ns, chop_ar;
if (ntohs(header->qdcount) != 1 ||
- !(p = skip_name(p, header, plen, 4)))
- return plen;
+ !(p = skip_name(p, header, *plen, 4)))
+ return 0;
GETSHORT(qtype, p);
GETSHORT(qclass, p);
/* First pass, find pointers to start and end of all the records we wish to elide:
records added for DNSSEC, unless explicitly queried for */
- for (rr_found = 0, chop_ns = 0, chop_an = 0, chop_ar = 0, i = 0;
+ for (chop_ns = 0, chop_an = 0, chop_ar = 0, i = 0;
i < ntohs(header->ancount) + ntohs(header->nscount) + ntohs(header->arcount);
i++)
{
unsigned char *pstart = p;
int type, class;
- if (!(p = skip_name(p, header, plen, 10)))
- return plen;
+ if (!(p = skip_name(p, header, *plen, 10)))
+ return rr_found;
GETSHORT(type, p);
GETSHORT(class, p);
p += 4; /* TTL */
GETSHORT(rdlen, p);
- if (!ADD_RDLEN(header, p, plen, rdlen))
- return plen;
+ if (!ADD_RDLEN(header, p, *plen, rdlen))
+ return rr_found;
if (mode == RRFILTER_EDNS0) /* EDNS */
{
@@ -225,7 +227,7 @@ size_t rrfilter(struct dns_header *header, size_t plen, int mode)
}
if (!expand_workspace(&rrs, &rr_sz, rr_found + 1))
- return plen;
+ return rr_found;
rrs[rr_found++] = pstart;
rrs[rr_found++] = p;
@@ -240,7 +242,7 @@ size_t rrfilter(struct dns_header *header, size_t plen, int mode)
/* Nothing to do. */
if (rr_found == 0)
- return plen;
+ return rr_found;
/* Second pass, look for pointers in names in the records we're keeping and make sure they don't
point to records we're going to elide. This is theoretically possible, but unlikely. If
@@ -248,38 +250,38 @@ size_t rrfilter(struct dns_header *header, size_t plen, int mode)
p = (unsigned char *)(header+1);
/* question first */
- if (!check_name(&p, header, plen, 0, rrs, rr_found))
- return plen;
+ if (!check_name(&p, header, *plen, 0, rrs, rr_found))
+ return rr_found;
p += 4; /* qclass, qtype */
/* Now answers and NS */
- if (!check_rrs(p, header, plen, 0, rrs, rr_found))
- return plen;
+ if (!check_rrs(p, header, *plen, 0, rrs, rr_found))
+ return rr_found;
/* Third pass, actually fix up pointers in the records */
p = (unsigned char *)(header+1);
- check_name(&p, header, plen, 1, rrs, rr_found);
+ check_name(&p, header, *plen, 1, rrs, rr_found);
p += 4; /* qclass, qtype */
- check_rrs(p, header, plen, 1, rrs, rr_found);
+ check_rrs(p, header, *plen, 1, rrs, rr_found);
/* Fourth pass, elide records */
- for (p = rrs[0], i = 1; i < rr_found; i += 2)
+ for (p = rrs[0], i = 1; (unsigned)i < rr_found; i += 2)
{
unsigned char *start = rrs[i];
- unsigned char *end = (i != rr_found - 1) ? rrs[i+1] : ((unsigned char *)header) + plen;
+ unsigned char *end = ((unsigned)i != rr_found - 1) ? rrs[i+1] : ((unsigned char *)header) + *plen;
memmove(p, start, end-start);
p += end-start;
}
- plen = p - (unsigned char *)header;
+ *plen = p - (unsigned char *)header;
header->ancount = htons(ntohs(header->ancount) - chop_an);
header->nscount = htons(ntohs(header->nscount) - chop_ns);
header->arcount = htons(ntohs(header->arcount) - chop_ar);
- return plen;
+ return rr_found;
}
/* This is used in the DNSSEC code too, hence it's exported */
--
2.39.2
OpenPGP_0x4931CA5B6C9FC5CB.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
