Pi-Hole/dnsmasq
1
0
Fork 0
mirror of https://github.com/pi-hole/dnsmasq.git synced 2025-12-19 18:28:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix caching logic for validated answers.

Browse Source 
The current logic is naive in the case that there is more than
one RRset in an answer (Typically, when a non-CNAME query is answered
by one or more CNAME RRs, and then then an answer RRset.)

If all the RRsets validate, then they are cached and marked as validated,
but if any RRset doesn't validate, then the AD flag is not set (good) and
ALL the RRsets are cached marked as not validated.

This breaks when, eg, the answer contains a validated CNAME, pointing
to a non-validated answer. A subsequent query for the CNAME without do
will get an answer with the AD flag wrongly reset, and worse, the same
query with do will get a cached answer without RRSIGS, rather than
being forwarded.

The code now records the validation of individual RRsets and that
is used to correctly set the "validated" bits in the cache entries.
This commit is contained in:
Simon Kelley
2017-10-25 17:48:19 +01:00
parent c366717e66
commit a6004d7f17
5 changed files with 161 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
src/rfc1035.c
View File

@@ -584,7 +584,8 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *doc
expired and cleaned out that way.
Return 1 if we reject an address because it look like part of dns-rebinding attack. */
int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t now,
char **ipsets, int is_sign, int check_rebind, int no_cache_dnssec, int secure, int *doctored)
char **ipsets, int is_sign, int check_rebind, int no_cache_dnssec,
int secure, int *doctored, char *rr_status)
{
unsigned char *p, *p1, *endrr, *namep;
int i, j, qtype, qclass, aqtype, aqclass, ardlen, res, searched_soa = 0;
@@ -595,6 +596,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
#else
(void)ipsets; /* unused */
#endif
cache_start_insert();
@@ -603,10 +605,16 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
{
searched_soa = 1;
ttl = find_soa(header, qlen, name, doctored);
#ifdef HAVE_DNSSEC
if (*doctored && secure)
return 0;
#endif
if (*doctored)
{
if (secure)
return 0;
if (rr_status)
for (i = 0; i < ntohs(header->ancount); i++)
if (rr_status[i])
return 0;
}
}
/* go through the questions. */
@@ -617,7 +625,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
int found = 0, cname_count = CNAME_CHAIN;
struct crec *cpp = NULL;
int flags = RCODE(header) == NXDOMAIN ? F_NXDOMAIN : 0;
int secflag = secure ? F_DNSSECOK : 0;
int cname_short = 0;
unsigned long cttl = ULONG_MAX, attl;
namep = p;
@@ -645,8 +653,9 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
if (!(p1 = skip_questions(header, qlen)))
return 0;
for (j = ntohs(header->ancount); j != 0; j--)
for (j = 0; j < ntohs(header->ancount); j++)
{
int secflag = 0;
unsigned char *tmp = namep;
/* the loop body overwrites the original name, so get it back here. */
if (!extract_name(header, qlen, &tmp, name, 1, 0) ||
@@ -672,11 +681,21 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
{
if (!extract_name(header, qlen, &p1, name, 1, 0))
return 0;
if (rr_status && rr_status[j])
{
/* validated RR anywhere in CNAME chain, don't cache. */
if (cname_short || aqtype == T_CNAME)
return 0;
secflag = F_DNSSECOK;
}
if (aqtype == T_CNAME)
{
if (!cname_count-- || secure)
return 0; /* looped CNAMES, or DNSSEC, which we can't cache. */
if (!cname_count--)
return 0; /* looped CNAMES, we can't cache. */
cname_short = 1;
goto cname_loop;
}
@@ -698,7 +717,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
ttl = find_soa(header, qlen, NULL, doctored);
}
if (ttl)
cache_insert(NULL, &addr, now, ttl, name_encoding | F_REVERSE | F_NEG | flags | secflag);
cache_insert(NULL, &addr, now, ttl, name_encoding | F_REVERSE | F_NEG | flags | (secure ? F_DNSSECOK : 0));
}
}
else
@@ -726,8 +745,10 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
if (!(p1 = skip_questions(header, qlen)))
return 0;
for (j = ntohs(header->ancount); j != 0; j--)
for (j = 0; j < ntohs(header->ancount); j++)
{
int secflag = 0;
if (!(res = extract_name(header, qlen, &p1, name, 0, 10)))
return 0; /* bad packet */
@@ -744,6 +765,10 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
if (aqclass == C_IN && res != 2 && (aqtype == T_CNAME || aqtype == qtype))
{
#ifdef HAVE_DNSSEC
if (rr_status && rr_status[j])
secflag = F_DNSSECOK;
#endif
if (aqtype == T_CNAME)
{
if (!cname_count--)
@@ -835,7 +860,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
pointing at this, inherit its TTL */
if (ttl || cpp)
{
newc = cache_insert(name, NULL, now, ttl ? ttl : cttl, F_FORWARD | F_NEG | flags | secflag);
newc = cache_insert(name, NULL, now, ttl ? ttl : cttl, F_FORWARD | F_NEG | flags | (secure ? F_DNSSECOK : 0));
if (newc && cpp)
{
cpp->addr.cname.target.cache = newc;
@@ -950,7 +975,7 @@ int check_for_local_domain(char *name, time_t now)
/* Note: the call to cache_find_by_name is intended to find any record which matches
ie A, AAAA, CNAME. */
if ((crecp = cache_find_by_name(NULL, name, now, F_IPV4 | F_IPV6 | F_CNAME |F_NO_RR)) &&
if ((crecp = cache_find_by_name(NULL, name, now, F_IPV4 | F_IPV6 | F_CNAME | F_NO_RR)) &&
(crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)))
return 1;
@@ -1736,8 +1761,9 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
if (qtype == T_CNAME || qtype == T_ANY)
{
if ((crecp = cache_find_by_name(NULL, name, now, F_CNAME)) &&
(qtype == T_CNAME || (crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG | (dryrun ? F_NO_RR : 0)))))
if ((crecp = cache_find_by_name(NULL, name, now, F_CNAME | (dryrun ? F_NO_RR : 0))) &&
(qtype == T_CNAME || (crecp->flags & F_CONFIG)) &&
((crecp->flags & F_CONFIG) || !do_bit || !(crecp->flags & F_DNSSECOK)))
{
if (!(crecp->flags & F_DNSSECOK))
sec_data = 0;