On Sat, Nov 20, 2021 at 02:40:59PM +0100, Otto Moerbeek wrote:
> On Sat, Nov 20, 2021 at 12:20:32PM +0100, Florian Obser wrote:
>
> > The Authentic Data (AD) flag indicates that the nameserver validated
> > the response using DNSSEC. For clients to trust this the nameserver
> > and the path to the nameserver must be trusted. In the general case
> > this is not true.
> >
> > We can trust localhost so we set the AD flag on queries to request
> > validation and preserve the AD flag in answers. (*)
> >
> > If, and only if, trusted nameservers (that are not on localhost) have
> > been added to resolv.conf and the path to them is secure the trust-ad
> > flag may be used to request validation from them and trust answers with
> > the AD flag set.
> >
> > The trust-ad option first appeared in glibc 2.31.
> > ( https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/461 and
> > https://man7.org/linux/man-pages/man5/resolv.conf.5.html )
> >
> > Thomas Habets (thomas at habets.se) pointed out on bugs@ that
> > VerifyHostKeyDNS in ssh only works with unwind (which is good) but
> > only by accident (which is bad).
> > https://marc.info/?t=163717495900003&r=1&w=2
> >
> > *) This is for people running unwind, unbound or some other validating
> > resolver on localhost. Yes, it is possible that someone set up some sort
> > of forwarder where they trust the DNS answers but not that they are
> > DNSSEC validated. This feels contrived and a case of DON'T DO THAT!
> >
> > OK?
>
> I like this much better than the sketch I posted on bugs@
>
> Two comment wrt the docs inline.
>
> Code looks and tests good.
>
> -Otto
>
> >
> > diff --git include/resolv.h include/resolv.h
> > index fb02483871e..2422deb5484 100644
> > --- include/resolv.h
> > +++ include/resolv.h
> > @@ -191,6 +191,7 @@ struct __res_state_ext {
> > /* DNSSEC extensions: use higher bit to avoid conflict with ISC use */
> > #define RES_USE_DNSSEC 0x20000000 /* use DNSSEC using OK bit in
> > OPT */
> > #define RES_USE_CD 0x10000000 /* set Checking Disabled flag */
> > +#define RES_TRUSTAD 0x80000000 /* Request AD, keep it in
> > responses. */
> >
> > #define RES_DEFAULT (RES_RECURSE | RES_DEFNAMES | RES_DNSRCH)
> >
> > diff --git lib/libc/asr/asr.c lib/libc/asr/asr.c
> > index 8bcb61b6000..77bc3854420 100644
> > --- lib/libc/asr/asr.c
> > +++ lib/libc/asr/asr.c
> > @@ -661,7 +661,8 @@ pass0(char **tok, int n, struct asr_ctx *ac)
> > d = strtonum(tok[i] + 6, 1, 16, &e);
> > if (e == NULL)
> > ac->ac_ndots = d;
> > - }
> > + } else if (!strcmp(tok[i], "trust-ad"))
> > + ac->ac_options |= RES_TRUSTAD;
> > }
> > }
> > }
> > @@ -672,7 +673,10 @@ pass0(char **tok, int n, struct asr_ctx *ac)
> > static int
> > asr_ctx_from_string(struct asr_ctx *ac, const char *str)
> > {
> > - char buf[512], *ch;
> > + struct sockaddr_in6 *sin6;
> > + struct sockaddr_in *sin;
> > + int i, trustad;
> > + char buf[512], *ch;
> >
> > asr_ctx_parse(ac, str);
> >
> > @@ -702,6 +706,27 @@ asr_ctx_from_string(struct asr_ctx *ac, const char
> > *str)
> > break;
> > }
> >
> > + trustad = 1;
> > + for (i = 0; i < ac->ac_nscount && trustad; i++) {
> > + switch (ac->ac_ns[i]->sa_family) {
> > + case AF_INET:
> > + sin = (struct sockaddr_in *)ac->ac_ns[i];
> > + if (sin->sin_addr.s_addr != htonl(INADDR_LOOPBACK))
> > + trustad = 0;
> > + break;
> > + case AF_INET6:
> > + sin6 = (struct sockaddr_in6 *)ac->ac_ns[i];
> > + if (!IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr))
> > + trustad = 0;
> > + break;
> > + default:
> > + trustad = 0;
> > + break;
> > + }
> > + }
> > + if (trustad)
> > + ac->ac_options |= RES_TRUSTAD;
> > +
> > return (0);
> > }
> >
> > diff --git lib/libc/asr/getrrsetbyname_async.c
> > lib/libc/asr/getrrsetbyname_async.c
> > index e5e7c23c261..06a998b0381 100644
> > --- lib/libc/asr/getrrsetbyname_async.c
> > +++ lib/libc/asr/getrrsetbyname_async.c
> > @@ -32,7 +32,7 @@
> > #include "asr_private.h"
> >
> > static int getrrsetbyname_async_run(struct asr_query *, struct asr_result
> > *);
> > -static void get_response(struct asr_result *, const char *, int);
> > +static void get_response(struct asr_result *, const char *, int, int);
> >
> > struct asr_query *
> > getrrsetbyname_async(const char *hostname, unsigned int rdclass,
> > @@ -150,7 +150,8 @@ getrrsetbyname_async_run(struct asr_query *as, struct
> > asr_result *ar)
> > break;
> > }
> >
> > - get_response(ar, ar->ar_data, ar->ar_datalen);
> > + get_response(ar, ar->ar_data, ar->ar_datalen,
> > + as->as_ctx->ac_options & RES_TRUSTAD);
> > free(ar->ar_data);
> > async_set_state(as, ASR_STATE_HALT);
> > break;
> > @@ -255,7 +256,7 @@ static void free_dns_response(struct dns_response *);
> > static int count_dns_rr(struct dns_rr *, u_int16_t, u_int16_t);
> >
> > static void
> > -get_response(struct asr_result *ar, const char *pkt, int pktlen)
> > +get_response(struct asr_result *ar, const char *pkt, int pktlen, int
> > trustad)
> > {
> > struct rrsetinfo *rrset = NULL;
> > struct dns_response *response = NULL;
> > @@ -287,7 +288,7 @@ get_response(struct asr_result *ar, const char *pkt,
> > int pktlen)
> > rrset->rri_nrdatas = response->header.ancount;
> >
> > /* check for authenticated data */
> > - if (response->header.ad == 1)
> > + if (response->header.ad == 1 && trustad)
> > rrset->rri_flags |= RRSET_VALIDATED;
> >
> > /* copy name from answer section */
> > diff --git lib/libc/asr/res_mkquery.c lib/libc/asr/res_mkquery.c
> > index c3d5af30f29..97b965e4c2a 100644
> > --- lib/libc/asr/res_mkquery.c
> > +++ lib/libc/asr/res_mkquery.c
> > @@ -62,6 +62,8 @@ res_mkquery(int op, const char *dname, int class, int
> > type,
> > h.flags |= RD_MASK;
> > if (ac->ac_options & RES_USE_CD)
> > h.flags |= CD_MASK;
> > + if (ac->ac_options & RES_TRUSTAD)
> > + h.flags |= AD_MASK;
> > h.qdcount = 1;
> > if (ac->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC))
> > h.arcount = 1;
> > diff --git lib/libc/asr/res_send_async.c lib/libc/asr/res_send_async.c
> > index c5cc41f56df..d6fba28b6f7 100644
> > --- lib/libc/asr/res_send_async.c
> > +++ lib/libc/asr/res_send_async.c
> > @@ -378,6 +378,9 @@ setup_query(struct asr_query *as, const char *name,
> > const char *dom,
> > h.flags |= RD_MASK;
> > if (as->as_ctx->ac_options & RES_USE_CD)
> > h.flags |= CD_MASK;
> > + if (as->as_ctx->ac_options & RES_TRUSTAD)
> > + h.flags |= AD_MASK;
> > +
> > h.qdcount = 1;
> > if (as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC))
> > h.arcount = 1;
> > diff --git share/man/man5/resolv.conf.5 share/man/man5/resolv.conf.5
> > index 8d3b91c0832..ac64d3e6fd6 100644
> > --- share/man/man5/resolv.conf.5
> > +++ share/man/man5/resolv.conf.5
> > @@ -259,6 +259,12 @@ first as an absolute name before any search list
> > elements are appended to it.
> > .It Cm tcp
> > Forces the use of TCP for queries.
> > Normal behaviour is to query via UDP but fall back to TCP on failure.
> > +.It Cm trust-ad
> > +Request DNSSEC validated data from the nameservers and preserve the
> > +Authentic Data (AD) flag in responses.
>
> > +Otherwise the Authentic Data (AD) flag is removed from responses.
>
> This is not what happens (though the DNS header itself is not exposed
> in the API). Maybe describe it as:
>
> Request DNSSEC validated data from the nameservers and evaluate the AD
> flag in responses.
>
> > +The nameservers and the network path to them must be trusted.
>
> Maybe:
>
> Only set this flag if the nameservers and the network paths to them are
> trusted.
The idea being that it shoud be clear that asr is not making the
trust decision, but the system admin is.
-Otto
