[Dnsmasq-discuss] [PATCH] Fix DNSSEC handling of CNAME records.
Simon Kelley
simon at thekelleys.org.uk
Thu May 26 13:45:09 UTC 2022
On 21/04/2022 23:46, Chris via Dnsmasq-discuss wrote:
> From: Chris Staite <chris at yourdreamnet.co.uk>
>
> Fixes the case where a CNAME is valid and unsigned and the target of the
> CNAME is returned with the CNAME but with no RRSIG (due to the CNAME not
> being in a signed zone).
>
> Since the CNAME is unsigned, there is no additional security risk returning
> an insecure response for the A/AAAA records that should have an RRSIG.
I'm not sure there is no additional security risk: consider the case
where goodguy.com is signed and has an A record sever.goodguy.com
I trick you into visiting badguy.com which is not signed and return
badguy.com CNAME server.gooddguy.com
server.goodguy.com A <evil-IPv4>
Both of those get cached.
Now, when you go to server.goodguy.com you'll get an answer from the
cache which gets you into trouble. It won't have the AD flag set, but
that's not a problem for the client, it's relying on a SERVFAIL answer
for data which doesn't validate in a signed domain.
At the very least, we're going to have to avoid caching the
server.goodguy.com data, but there's nothing in our answer that allows
another cache further down the chain to do that.
Do you have a real-life example that's giving this sort of answer?
Cheers,
Simon.
>
> There's still the outstanding question of whether dnsmasq handles the case
> where a secure CNAME points to a secure A/AAAA in a different zone. I'm not
> sure whether it ought to perform another query to to target SOA to get the
> RRSIG for that in addition.
> ---
> src/dnsmasq.c | 1 +
> src/dnsmasq.h | 1 +
> src/dnssec.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/src/dnsmasq.c b/src/dnsmasq.c
> index 7cfb493..57e2e24 100644
> --- a/src/dnsmasq.c
> +++ b/src/dnsmasq.c
> @@ -136,6 +136,7 @@ int main (int argc, char **argv)
> daemon->namebuff = safe_malloc(MAXDNAME * 2);
> daemon->keyname = safe_malloc(MAXDNAME * 2);
> daemon->workspacename = safe_malloc(MAXDNAME * 2);
> + daemon->workspacename2 = safe_malloc(MAXDNAME * 2);
> /* 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 bfc0fd4..9c10df8 100644
> --- a/src/dnsmasq.h
> +++ b/src/dnsmasq.h
> @@ -1195,6 +1195,7 @@ extern struct daemon {
> char *workspacename;
> #endif
> #ifdef HAVE_DNSSEC
> + char *workspacename2;
> char *keyname; /* MAXDNAME size buffer */
> unsigned long *rr_status; /* ceiling in TTL from DNSSEC or zero for insecure */
> int rr_status_sz;
> diff --git a/src/dnssec.c b/src/dnssec.c
> index 9965eea..5000ed2 100644
> --- a/src/dnssec.c
> +++ b/src/dnssec.c
> @@ -1841,6 +1841,51 @@ static int zone_status(char *name, int class, char *keyname, time_t now)
>
> return STAT_SECURE;
> }
> +
> +/* Find a matching CNAME and check if it is insecure
> + Return code:
> + STAT_SECURE if the CNAME for the given name is secure
> + STAT_INSECURE if the CNAME for the given name is insecure
> + STAT_BOGUS something went wrong
> + STAT_NEED_KEY need DNSKEY to complete validation (name is returned in keyname, class in *class)
> + STAT_NEED_DS need DS to complete validation (name is returned in keyname)
> +*/
> +int cname_insecure(struct dns_header *header, size_t plen, char* name, char* keyname, int* oclass, time_t now)
> +{
> + int i, type, class, length;
> + int rc = STAT_BOGUS;
> + unsigned char* p1 = (unsigned char *)(header+1);
> +
> + if (!(p1 = skip_name(p1, header, plen, 10)))
> + return STAT_BOGUS; /* bad packet */
> +
> + p1 += 4; /* skip type and class */
> +
> + for (i = 0; i < ntohs(header->ancount); ++i)
> + {
> + if (!extract_name(header, plen, &p1, daemon->workspacename2, 1, 0))
> + return STAT_BOGUS;
> +
> + GETSHORT(type, p1);
> + GETSHORT(class, p1);
> + p1 += 4; /* TTL */
> + GETSHORT(length, p1);
> +
> + if (!extract_name(header, plen, &p1, keyname, 1, 0))
> + return STAT_BOGUS;
> +
> + if (type == T_CNAME && hostname_isequal(name, keyname))
> + {
> + strcpy(name, keyname);
> + rc = zone_status(daemon->workspacename2, class, keyname, now);
> + if ((STAT_ISEQUAL(rc, STAT_NEED_DS) || STAT_ISEQUAL(rc, STAT_NEED_KEY)) && oclass)
> + *oclass = class;
> + return rc;
> + }
> + }
> +
> + return rc;
> +}
>
> /* Validate all the RRsets in the answer and authority sections of the reply (4035:3.2.3)
> Return code:
> @@ -2001,13 +2046,24 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch
> if (check_unsigned && i < ntohs(header->ancount))
> {
> rc = zone_status(name, class1, keyname, now);
> + if (STAT_ISEQUAL(rc, STAT_SECURE) && (type1 == T_A || type1 == T_AAAA))
> + {
> + /* There can't be an RRSIG over a CNAME and A record if the
> + CNAME is insecure, so simply allow in that case */
> + my_syslog(LOG_INFO, "Check name for %s", name);
> + rc = cname_insecure(header, plen, name, keyname, class, now);
> + }
> +
> if (STAT_ISEQUAL(rc, STAT_SECURE))
> - rc = STAT_BOGUS | DNSSEC_FAIL_NOSIG;
> -
> - if (class)
> - *class = class1; /* Class for NEED_DS or NEED_KEY */
> + {
> + rc = STAT_BOGUS | DNSSEC_FAIL_NOSIG;
> + }
> + else if (type1 != T_A && type1 != T_AAAA && class)
> + {
> + *class = class1; /* Class for NEED_DS or NEED_KEY */
> + }
> }
> - else
> + else
> rc = STAT_INSECURE;
>
> if (!STAT_ISEQUAL(rc, STAT_INSECURE))
More information about the Dnsmasq-discuss
mailing list