From 105c25e56135f99570ccf2de6fc0ca16442edbd5 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sat, 15 Mar 2025 09:05:47 +0000 Subject: [PATCH] Fix DNSSEC and DNAME. Do the correct things to validate replies which include a DNAME record. Thanks to Graham Clinch for pointing this out. --- src/dnsmasq.c | 1 + src/dnsmasq.h | 2 +- src/dnssec.c | 130 +++++++++++++++++++++++++++++++++++++++++++------- src/util.c | 2 +- 4 files changed, 117 insertions(+), 18 deletions(-) diff --git a/src/dnsmasq.c b/src/dnsmasq.c index 41a62cd..944b4cc 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -133,6 +133,7 @@ int main (int argc, char **argv) '.' or NAME_ESCAPE then all would have to be escaped, so the presentation format would be twice as long as the spec. */ daemon->keyname = safe_malloc((MAXDNAME * 2) + 1); + daemon->cname = safe_malloc((MAXDNAME * 2) + 1); /* one char flag per possible RR in answer section (may get extended). */ daemon->rr_status_sz = 64; daemon->rr_status = safe_malloc(sizeof(*daemon->rr_status) * daemon->rr_status_sz); diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 86f0eaf..2e6b0b7 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -1248,7 +1248,7 @@ extern struct daemon { char *namebuff; /* MAXDNAME size buffer */ char *workspacename; #ifdef HAVE_DNSSEC - char *keyname; /* MAXDNAME size buffer */ + char *keyname, *cname; /* MAXDNAME size buffer */ unsigned long *rr_status; /* ceiling in TTL from DNSSEC or zero for insecure */ int rr_status_sz; int dnssec_no_time_check; diff --git a/src/dnssec.c b/src/dnssec.c index 0c2e74f..e4a268a 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -1942,11 +1942,13 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch static unsigned char **targets = NULL; static int target_sz = 0; - unsigned char *ans_start, *p1, *p2; - int type1, class1, rdlen1 = 0, type2, class2, rdlen2, qclass, qtype, targetidx; - int i, j, rc = STAT_INSECURE; + unsigned char *ans_start, *p1, *p2, *p3; + int type1, class1, rdlen1 = 0, type2, class2, rdlen2, qclass, qtype, targetidx, gotdname; + int i, j, k, rc = STAT_INSECURE; int secure = STAT_SECURE; int rc_nsec; + unsigned long ttl; + /* extend rr_status if necessary */ if (daemon->rr_status_sz < ntohs(header->ancount) + ntohs(header->nscount)) { @@ -1993,27 +1995,119 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch /* Can't validate an RRSIG query */ if (qtype == T_RRSIG) return STAT_INSECURE; + + /* Find CNAME targets. */ + for (gotdname = i = 0; i < ntohs(header->ancount); i++) + { + if (!(p1 = skip_name(p1, header, plen, 10))) + return STAT_BOGUS; /* bad packet */ + + GETSHORT(type1, p1); + GETSHORT(class1, p1); + p1 += 4; /* TTL */ + GETSHORT(rdlen1, p1); + + if (type1 == T_DNAME) + gotdname = 1; + + if (qtype != T_CNAME && qtype != T_ANY && type1 == T_CNAME && class1 == qclass) + { + if (!expand_workspace(&targets, &target_sz, targetidx)) + return STAT_BOGUS; + + targets[targetidx++] = p1; /* pointer to target name */ + } + + if (!ADD_RDLEN(header, p1, plen, rdlen1)) + return STAT_BOGUS; + } - if (qtype != T_CNAME && qtype != T_ANY) - for (j = ntohs(header->ancount); j != 0; j--) + /* A DNAME capable of sythesising a CNAME means we don't need to validate the CNAME, + we can just assume that it's valid. RFC 4035 3.2.3 */ + if (gotdname) + for (p1 = ans_start, i = 0; i < ntohs(header->ancount); i++) { - if (!(p1 = skip_name(p1, header, plen, 10))) + if (!extract_name(header, plen, &p1, name, EXTR_NAME_EXTRACT, 10)) return STAT_BOGUS; /* bad packet */ - GETSHORT(type2, p1); - p1 += 6; /* class, TTL */ - GETSHORT(rdlen2, p1); + GETSHORT(type1, p1); + GETSHORT(class1, p1); + p1 += 4; /* TTL */ + GETSHORT(rdlen1, p1); - if (type2 == T_CNAME) + if (type1 != T_DNAME) { - if (!expand_workspace(&targets, &target_sz, targetidx)) + if (!ADD_RDLEN(header, p1, plen, rdlen1)) return STAT_BOGUS; - - targets[targetidx++] = p1; /* pointer to target name */ } - - if (!ADD_RDLEN(header, p1, plen, rdlen2)) - return STAT_BOGUS; + else + { + if (!extract_name(header, plen, &p1, keyname, EXTR_NAME_EXTRACT, 0)) + return STAT_BOGUS; /* bad packet */ + + /* We now have the name of the DNAME in name, and the target in keyname. + Look for any CNAMEs which could have been synthesised from this DNAME + and pre-qualify them. */ + for (p2 = ans_start, j = 0; j < ntohs(header->ancount); j++) + { + if (!extract_name(header, plen, &p2, daemon->cname, EXTR_NAME_EXTRACT, 10)) + return STAT_BOGUS; /* bad packet */ + + GETSHORT(type2, p2); + GETSHORT(class2, p2); + GETLONG(ttl, p2); + GETSHORT(rdlen2, p2); + + if (type2 != T_CNAME || class2 != class1) + { + if (!ADD_RDLEN(header, p2, plen, rdlen2)) + return STAT_BOGUS; + } + else + { + size_t name_prefix_len = strlen(daemon->cname) - strlen(name); + + if (!extract_name(header, plen, &p2, daemon->workspacename, EXTR_NAME_EXTRACT, 0)) + return STAT_BOGUS; /* bad packet */ + + /* We have the name of the CNAME in daemon->cname, and the target in daemon->workspacename. + See if the CNAME was sythesised from the DNAME. + CNAME must be . + CNAME target must be . + s must match for name and target. */ + if (hostname_issubdomain(name, daemon->cname) == 1 && + hostname_issubdomain(keyname, daemon->workspacename) == 1 && + name_prefix_len == strlen(daemon->workspacename) - strlen(keyname)) + { + char save = daemon->cname[name_prefix_len]; + daemon->cname[name_prefix_len] = 0; + daemon->workspacename[name_prefix_len] = 0; + + if (hostname_isequal(daemon->cname, daemon->workspacename)) + { + /* pre-qualify this as validated */ + daemon->rr_status[j] = ttl > 0 ? ttl : 1; + + /* and remove it from the targets we need to have validated answers to. */ + if (class2 == qclass) + { + daemon->cname[name_prefix_len] = save; + for (k = 0; k cname, EXTR_NAME_COMPARE, 0))) + return STAT_BOGUS; /* bad packet */ + + if (rc1 == 1) + targets[k] = NULL; + } + } + } + } + } + } + } } for (p1 = ans_start, i = 0; i < ntohs(header->ancount) + ntohs(header->nscount); i++) @@ -2032,6 +2126,10 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch /* Don't try and validate RRSIGs! */ if (type1 == T_RRSIG) continue; + + /* Pre-validated by DNAME above don't validate. */ + if (daemon->rr_status[i] != 0) + continue; /* Check if we've done this RRset already */ for (p2 = ans_start, j = 0; j < i; j++) diff --git a/src/util.c b/src/util.c index 226e8a1..764db97 100644 --- a/src/util.c +++ b/src/util.c @@ -426,7 +426,7 @@ int hostname_order(const char *a, const char *b) int hostname_isequal(const char *a, const char *b) { - return hostname_order(a, b) == 0; + return strlen(a) == strlen(b) && hostname_order(a, b) == 0; } /* is b equal to or a subdomain of a return 2 for equal, 1 for subdomain */