Fix logic error in signed RR handling.

In extract_addresses() the "secure" argument is only set if the
whole reply is validated (ie the AD bit can be set). Even without
that, some records may be validated, and should be marked
as such in the cache.

Related, the DNS doctor code has to update the flags for individual
RRs as it works, not the global "secure" flag.
This commit is contained in:
Simon Kelley
2024-02-02 21:36:56 +00:00
parent 86c15032ba
commit d4a6f3a93e
3 changed files with 44 additions and 36 deletions

View File

@@ -1804,7 +1804,7 @@ static int zone_status(char *name, int class, char *keyname, time_t now)
When validating replies to DS records, we're only interested in the NSEC{3} RRs in the auth section.
Other RRs in that section missing sigs will not cause am INSECURE reply. We determine this mode
is the nons argument is non-NULL.
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)

View File

@@ -814,9 +814,18 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
}
}
if (do_doctor(header, n))
if (!bogusanswer)
{
if (daemon->doctors && !do_doctor(header, n))
{
/* do_doctors found malformed answer. */
munged = 1;
SET_RCODE(header, SERVFAIL);
cache_secure = 0;
ede = EDE_OTHER;
}
if (RCODE(header) != SERVFAIL)
switch (extract_addresses(header, n, daemon->namebuff, now, ipsets, nftsets, is_sign, check_rebind, no_cache, cache_secure))
{
case 1:
@@ -834,8 +843,9 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
ede = EDE_OTHER;
break;
}
}
if (rcode == NOERROR && rrfilter(header, &n, RRFILTER_CONF) > 0)
if (RCODE(header) == NOERROR && rrfilter(header, &n, RRFILTER_CONF) > 0)
ede = EDE_FILTERED;
}

View File

@@ -388,10 +388,6 @@ int do_doctor(struct dns_header *header, size_t qlen)
{
unsigned char *p;
int i, qtype, qclass, rdlen;
int doctored = 0;
if (!daemon->doctors)
return 0;
if (!(p = skip_questions(header, qlen)))
return 0;
@@ -436,7 +432,11 @@ int do_doctor(struct dns_header *header, size_t qlen)
addr.addr4.s_addr |= (doctor->out.s_addr & doctor->mask.s_addr);
/* Since we munged the data, the server it came from is no longer authoritative */
header->hb3 &= ~HB3_AA;
doctored = 1;
#ifdef HAVE_DNSSEC
/* remove validated flag from this RR, since we changed it! */
if (option_bool(OPT_DNSSEC_VALID) && i < ntohs(header->ancount))
daemon->rr_status[i] = 0;
#endif
memcpy(p, &addr.addr4, INADDRSZ);
log_query(F_FORWARD | F_CONFIG | F_IPV4, daemon->workspacename, &addr, NULL, 0);
break;
@@ -447,14 +447,14 @@ int do_doctor(struct dns_header *header, size_t qlen)
return 0; /* bad packet */
}
return doctored;
return 1;
}
/* Find SOA RR in auth section to get TTL for negative caching of name.
Cache said SOA and return the difference in length between name and the name of the
SOA RR so we can look it up again.
*/
static int find_soa(struct dns_header *header, size_t qlen, char *name, int *substring, int no_cache, int secure, time_t now)
static int find_soa(struct dns_header *header, size_t qlen, char *name, int *substring, int no_cache, time_t now)
{
unsigned char *p, *psave;
int qtype, qclass, rdlen;
@@ -463,8 +463,6 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *sub
size_t name_len, soa_len, len;
union all_addr addr;
(void)secure; /* warning */
/* first move to NS section and find TTL from SOA RR */
if (!(p = skip_questions(header, qlen)) ||
!(p = skip_section(p, ntohs(header->ancount), header, qlen)))
@@ -547,7 +545,7 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *sub
int secflag = 0;
#ifdef HAVE_DNSSEC
if (option_bool(OPT_DNSSEC_VALID) && secure && daemon->rr_status[i + ntohs(header->ancount)] != 0)
if (option_bool(OPT_DNSSEC_VALID) && daemon->rr_status[i + ntohs(header->ancount)] != 0)
{
secflag = F_DNSSECOK;
@@ -700,7 +698,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
if (aqclass == C_IN && res != 2 && (aqtype == T_CNAME || aqtype == T_PTR))
{
#ifdef HAVE_DNSSEC
if (option_bool(OPT_DNSSEC_VALID) && secure && daemon->rr_status[j] != 0)
if (option_bool(OPT_DNSSEC_VALID) && daemon->rr_status[j] != 0)
{
/* validated RR anywhere in CNAME chain, don't cache. */
if (cname_short || aqtype == T_CNAME)
@@ -750,7 +748,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
if (!found && !option_bool(OPT_NO_NEG))
{
/* don't cache SOAs for negative PTR records */
ttl = find_soa(header, qlen, name, NULL, 1, 0, now);
ttl = find_soa(header, qlen, name, NULL, 1, now);
flags |= F_NEG | (secure ? F_DNSSECOK : 0);
if (name_encoding && ttl)
@@ -815,7 +813,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
}
#ifdef HAVE_DNSSEC
if (option_bool(OPT_DNSSEC_VALID) && secure && daemon->rr_status[j] != 0)
if (option_bool(OPT_DNSSEC_VALID) && daemon->rr_status[j] != 0)
{
secflag = F_DNSSECOK;
@@ -1054,7 +1052,7 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
/* If there's no SOA to get the TTL from, but there is a CNAME
pointing at this, inherit its TTL */
if (insert && !option_bool(OPT_NO_NEG) && ((ttl = find_soa(header, qlen, name, &substring, no_cache_dnssec, secure, now)) || cpp))
if (insert && !option_bool(OPT_NO_NEG) && ((ttl = find_soa(header, qlen, name, &substring, no_cache_dnssec, now)) || cpp))
{
addr.rrdata.datalen = substring;
addr.rrdata.rrtype = qtype;