First, I'm happy to this subject considered again, even if I don't use DNSSEC these days I think it makes sense to provide this support in libc.
On Sat, Nov 20 2021, Florian Obser <flor...@openbsd.org> wrote: > On 2021-11-20 14:40 +01, Otto Moerbeek <o...@drijf.net> wrote: >> On Sat, Nov 20, 2021 at 12:20:32PM +0100, Florian Obser wrote: >>> 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: >> > > That is a very good point. And hints at that the diff is not complete. > I think we should fiddle with the header. I think res_send_async_run() > should unset the AD flag in ASR_STATE_PACKET. The api exposes the raw > packet in the res_query(3) family of functions. And this is actually > what glibc does. Indeed, my previous proposal used to do this (I don't remember if I tested the invalidation though). >> 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. >> > > I wanted to focus less on the technical details (AD flag) and more on > what this means. Of course I failed at that ;) > I think we should mention the AD flag so that people who know how DNSSEC > works can find it, but we need to better explain when random user should > set the flag (never!). > > I'll rework the diff. > >>> +This is the default for nameservers on localhost. >>> .El >>> .El >>> .Pp Here's a refreshed version of the diff initially sent in this thread: https://marc.info/?l=openbsd-tech&m=159567881530990&w=2 It's quite similar to Florian's diff so I'd like to re-submit it. I like a lot the "automatic localhost trust" from Florian so I incorporated his improvement here, but I can leave it out from at commit time. In the proposal below resolv.conf(5) says: +This option should be used only if the transport between the +.Ic nameserver +and the resolver is secure. +It is enabled by default if +.Nm resolv.conf +only lists nameservers on localhost. which I think is more accurate regarding nameservers on localhost. The wording in res_init(3) is unchanged from my first proposal, it should also be updated if localhost is to be automatically trusted. The diff also adds trust-ad support in asr_debug.c. How do you folks like this? Index: include/resolv.h =================================================================== RCS file: /home/cvs/src/include/resolv.h,v retrieving revision 1.22 diff -u -p -r1.22 resolv.h --- include/resolv.h 14 Jan 2019 06:23:06 -0000 1.22 +++ include/resolv.h 20 Nov 2021 14:24:08 -0000 @@ -186,6 +186,8 @@ struct __res_state_ext { #define RES_INSECURE2 0x00000800 /* type 2 security disabled */ #define RES_NOALIASES 0x00001000 /* shuts off HOSTALIASES feature */ #define RES_USE_INET6 0x00002000 /* use/map IPv6 in gethostbyname() */ +/* GNU extension: use higher bit to avoid conflict with ISC use */ +#define RES_TRUSTAD 0x80000000 /* query with AD, trust AD in reply */ /* KAME extensions: use higher bit to avoid conflict with ISC use */ #define RES_USE_EDNS0 0x40000000 /* use EDNS0 */ /* DNSSEC extensions: use higher bit to avoid conflict with ISC use */ Index: share/man/man5/resolv.conf.5 =================================================================== RCS file: /home/cvs/src/share/man/man5/resolv.conf.5,v retrieving revision 1.62 diff -u -p -r1.62 resolv.conf.5 --- share/man/man5/resolv.conf.5 24 Aug 2021 07:30:32 -0000 1.62 +++ share/man/man5/resolv.conf.5 20 Nov 2021 15:58:39 -0000 @@ -259,6 +259,18 @@ first as an absolute name before any sea .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 +Sets the AD flag in DNS queries, and preserve the value of the AD flag +in DNS replies (this flag is stripped by default). +Useful when applications want to ensure that the DNS resources they +look up have been signed with DNSSEC and validated by the +.Ic nameserver . +This option should be used only if the transport between the +.Ic nameserver +and the resolver is secure. +It is enabled by default if +.Nm resolv.conf +only lists nameservers on localhost. .El .El .Pp Index: lib/libc/asr/asr.c =================================================================== RCS file: /home/cvs/src/lib/libc/asr/asr.c,v retrieving revision 1.66 diff -u -p -r1.66 asr.c --- lib/libc/asr/asr.c 5 Nov 2021 13:08:58 -0000 1.66 +++ lib/libc/asr/asr.c 20 Nov 2021 15:59:32 -0000 @@ -656,6 +656,8 @@ pass0(char **tok, int n, struct asr_ctx ac->ac_options |= RES_USEVC; else if (!strcmp(tok[i], "edns0")) ac->ac_options |= RES_USE_EDNS0; + else if (!strcmp(tok[i], "trust-ad")) + ac->ac_options |= RES_TRUSTAD; else if ((!strncmp(tok[i], "ndots:", 6))) { e = NULL; d = strtonum(tok[i] + 6, 1, 16, &e); @@ -672,7 +674,10 @@ pass0(char **tok, int n, struct asr_ctx static int asr_ctx_from_string(struct asr_ctx *ac, const char *str) { - char buf[512], *ch; + char buf[512], *ch; + struct sockaddr_in6 *sin6; + struct sockaddr_in *sin; + int i, trustad; asr_ctx_parse(ac, str); @@ -701,6 +706,27 @@ asr_ctx_from_string(struct asr_ctx *ac, if (ch && asr_ndots(++ch) == 0) 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); } Index: lib/libc/asr/asr_debug.c =================================================================== RCS file: /home/cvs/src/lib/libc/asr/asr_debug.c,v retrieving revision 1.27 diff -u -p -r1.27 asr_debug.c --- lib/libc/asr/asr_debug.c 2 Apr 2021 07:00:30 -0000 1.27 +++ lib/libc/asr/asr_debug.c 20 Nov 2021 14:24:58 -0000 @@ -286,6 +286,7 @@ _asr_dump_config(FILE *f, struct asr *a) PRINTOPT(RES_USE_EDNS0, "USE_EDNS0"); PRINTOPT(RES_USE_DNSSEC, "USE_DNSSEC"); PRINTOPT(RES_USE_CD, "USE_CD"); + PRINTOPT(RES_TRUSTAD, "TRUSTAD"); if (o) fprintf(f, " 0x%08x", o); fprintf(f, "\n"); Index: lib/libc/asr/getrrsetbyname_async.c =================================================================== RCS file: /home/cvs/src/lib/libc/asr/getrrsetbyname_async.c,v retrieving revision 1.11 diff -u -p -r1.11 getrrsetbyname_async.c --- lib/libc/asr/getrrsetbyname_async.c 23 Feb 2017 17:04:02 -0000 1.11 +++ lib/libc/asr/getrrsetbyname_async.c 20 Nov 2021 14:24:08 -0000 @@ -32,7 +32,8 @@ #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_query *, struct asr_result *, const char *, + int); struct asr_query * getrrsetbyname_async(const char *hostname, unsigned int rdclass, @@ -150,7 +151,7 @@ getrrsetbyname_async_run(struct asr_quer break; } - get_response(ar, ar->ar_data, ar->ar_datalen); + get_response(as, ar, ar->ar_data, ar->ar_datalen); free(ar->ar_data); async_set_state(as, ASR_STATE_HALT); break; @@ -255,7 +256,8 @@ static void free_dns_response(struct dns 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_query *as, struct asr_result *ar, const char *pkt, + int pktlen) { struct rrsetinfo *rrset = NULL; struct dns_response *response = NULL; @@ -287,7 +289,7 @@ get_response(struct asr_result *ar, cons rrset->rri_nrdatas = response->header.ancount; /* check for authenticated data */ - if (response->header.ad == 1) + if (as->as_ctx->ac_options & RES_TRUSTAD && response->header.ad == 1) rrset->rri_flags |= RRSET_VALIDATED; /* copy name from answer section */ Index: lib/libc/asr/res_mkquery.c =================================================================== RCS file: /home/cvs/src/lib/libc/asr/res_mkquery.c,v retrieving revision 1.13 diff -u -p -r1.13 res_mkquery.c --- lib/libc/asr/res_mkquery.c 14 Jan 2019 06:49:42 -0000 1.13 +++ lib/libc/asr/res_mkquery.c 20 Nov 2021 14:24:08 -0000 @@ -62,6 +62,9 @@ res_mkquery(int op, const char *dname, i h.flags |= RD_MASK; if (ac->ac_options & RES_USE_CD) h.flags |= CD_MASK; + if ((ac->ac_options & RES_TRUSTAD) && + !(ac->ac_options & RES_USE_DNSSEC)) + h.flags |= AD_MASK; h.qdcount = 1; if (ac->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC)) h.arcount = 1; Index: lib/libc/asr/res_send_async.c =================================================================== RCS file: /home/cvs/src/lib/libc/asr/res_send_async.c,v retrieving revision 1.39 diff -u -p -r1.39 res_send_async.c --- lib/libc/asr/res_send_async.c 28 Sep 2019 11:21:07 -0000 1.39 +++ lib/libc/asr/res_send_async.c 20 Nov 2021 14:24:08 -0000 @@ -378,6 +378,9 @@ setup_query(struct asr_query *as, const 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) && + !(as->as_ctx->ac_options & RES_USE_DNSSEC)) + h.flags |= AD_MASK; h.qdcount = 1; if (as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC)) h.arcount = 1; @@ -700,6 +703,9 @@ validate_packet(struct asr_query *as) /* Must be a response */ if ((h.flags & QR_MASK) == 0) goto inval; + /* Zap the AD flag unless the server is considered trusted. */ + if (!(as->as_ctx->ac_options & RES_TRUSTAD)) + h.flags &= ~(AD_MASK); as->as.dns.rcode = RCODE(h.flags); as->as.dns.ancount = h.ancount; Index: lib/libc/net/res_init.3 =================================================================== RCS file: /home/cvs/src/lib/libc/net/res_init.3,v retrieving revision 1.4 diff -u -p -r1.4 res_init.3 --- lib/libc/net/res_init.3 25 Apr 2020 21:06:17 -0000 1.4 +++ lib/libc/net/res_init.3 20 Nov 2021 14:24:08 -0000 @@ -179,6 +179,17 @@ This option has no effect. In the past, it turned off the legacy .Ev HOSTALIASES feature. +.It Dv RES_TRUSTAD +If set, the resolver routines will set the AD flag in DNS queries and +preserve the value of the AD flag in DNS replies (this flag is stripped +by default). +Useful when applications want to ensure that the DNS resources they +look up have been signed with DNSSEC and validated by the name server. +Direct use of this option to enable AD bit processing is discouraged. +Instead the use of trusted name servers should be annotated with +.Dq options trust-ad +in +.Xr resolv.conf 5 . .It Dv RES_USE_INET6 With this option .Xr gethostbyname 3 -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
