On Fri, Jul 17 2020, Jesper Wallin <jes...@ifconfig.se> wrote: > Hi all, > > I recently decided to add SSHFP records for my servers, since I never > memorize or write down my key fingerprints. I learned that if I > want ssh(1) to trust these records, DNSSEC needs to be enabled for my > zone. To validate these records, ssh(1) is using getrrsetbyname(3), > which checks if the AD (Authentic Data) flag is set in the response. > > To get a response with the AD flag set, the request itself also needs > to have the AD flag set. It turns out that getrrsetbyname(3) doesn't > set this and will therefore never get a validated response, unless the > resolver is configured to always send it, no matter what the client > requested. > > It seems like unwind(8) behaves this way but it also responds with the > RRSIG records, which is extra overhead and ignored by getrrsetbyname(3). > > This was mentioned a few years ago [0] and the solution suggested was > to add the RES_USE_DNSSEC to _res.options, which also makes the resolver > respond with the extra RRSIG records. > > Instead, by only setting the AD flag, both the request and the response > has the same size as without the flag set. The patch below will add > RES_USE_AD as an option to _res.options and set it by default. > This is also the default behaviour in dig(1), which I understand is a > bit different, but that sure added some confusion while debugging this. > > This let you run unbound(8) or any other validating resolver on your > local network and getrrsetbyname(3) will trust it. Do read the CAVEATS > in the manual of getrrsetbyname(3) though. > > As a side note, I noticed that the default value of _res.options was the > same value as RES_DEFAULT, so I changed it to RES_DEFAULT instead, for > the sake of consistency. > > Thoughts?
Thanks for addressing this longstanding issue. I think using the AD bit in queries is a good idea. IIRC Peter J. Philipp (cc'd) suggested using it but I was not thrilled because: 1. the meaning of the AD bit in queries is relatively recent (2013 I think[0]) 2. getrrsetbyname also collects signature records, and for this you need EDNS + the DO bit set. Implementing this in was not 100% trivial, I think we had something working but Eric or I were not 100% happy with it. 1. is probably not a concern, after all you're supposed to use a trustworthy resolver, which should be modern and understand the purpose of the AD bit in queries. 2. is probably not a concern either. I guess that all getrrsetbyname(3) callers only care about the target records and the AD bit, not about the signature records. (Why would they use it for anyway?). In the base system, only ssh(1) and traceroute(8) use getrrsetbyname(3). AFAIK no other system provides getrrsetbyname(3), and ISC has removed getrrsetbyname and the whole lwres API in 2017[1]. So I'd say we're free to improve our version of getrrsetbyname(3) as we see fit. Now let's discuss the implementation. There are two main things I dislike with your diff: - it affects all DNS queries done through libc, even if the user doesn't care at all about DNSSEC. I suspect there's potential for fallout here, but I have no data to back this. - trusting the AD bit by default looks a bit too easy. The documentation says that people should only trust the AD bit / RRSET_VALIDATED if the communication between libc and the resolver is secure, but who actually reads the documentation? For those two reasons I think the feature should be opt-in. That's what Florian Weimer did in glibc, with the implementation of RES_TRUSTAD[2][3]. The diff below implements the same semantics: - by default, don't send queries the AD flag, and don't trust its value in replies - if "options trust-ad" is set in resolv.conf(5), set the AD flag in queries and allow applications to look at the AD flag in replies. There's one more thing I'd like to check in the res_* API, but I'm sitting on this idea since too long already, and I'm happy with my test results so far. Thoughts? Comments? [0] https://tools.ietf.org/html/rfc6840#section-5.7 [1] https://gitlab.isc.org/isc-projects/bind9/-/commit/8eb88aafee951859264e36c315b1289cd8c2088b [2] https://bugzilla.redhat.com/show_bug.cgi?id=1164339 [3] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=446997ff1433d33452b81dfa9e626b8dccf101a4 Index: include/resolv.h =================================================================== RCS file: /d/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 23 Jul 2020 13:28:15 -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: lib/libc/net/res_init.3 =================================================================== RCS file: /d/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 25 Jul 2020 02:09:58 -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 Index: share/man/man5/resolv.conf.5 =================================================================== RCS file: /d/cvs/src/share/man/man5/resolv.conf.5,v retrieving revision 1.60 diff -u -p -r1.60 resolv.conf.5 --- share/man/man5/resolv.conf.5 25 Apr 2020 14:22:04 -0000 1.60 +++ share/man/man5/resolv.conf.5 25 Jul 2020 02:08:17 -0000 @@ -285,6 +285,15 @@ 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. .El .El .Pp Index: lib/libc/asr/asr.c =================================================================== RCS file: /d/cvs/src/lib/libc/asr/asr.c,v retrieving revision 1.64 diff -u -p -r1.64 asr.c --- lib/libc/asr/asr.c 6 Jul 2020 13:33:05 -0000 1.64 +++ lib/libc/asr/asr.c 23 Jul 2020 23:36:06 -0000 @@ -640,6 +640,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); Index: lib/libc/asr/asr_debug.c =================================================================== RCS file: /d/cvs/src/lib/libc/asr/asr_debug.c,v retrieving revision 1.26 diff -u -p -r1.26 asr_debug.c --- lib/libc/asr/asr_debug.c 3 Jul 2019 03:24:03 -0000 1.26 +++ lib/libc/asr/asr_debug.c 23 Jul 2020 23:36:06 -0000 @@ -285,6 +285,7 @@ _asr_dump_config(FILE *f, struct asr *a) PRINTOPT(RES_NOALIASES, "NOALIASES"); PRINTOPT(RES_USE_EDNS0, "USE_EDNS0"); PRINTOPT(RES_USE_DNSSEC, "USE_DNSSEC"); + PRINTOPT(RES_TRUSTAD, "TRUSTAD"); if (o) fprintf(f, " 0x%08x", o); fprintf(f, "\n"); Index: lib/libc/asr/getrrsetbyname_async.c =================================================================== RCS file: /d/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 23 Jul 2020 23:36:06 -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: /d/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 24 Jul 2020 12:34:27 -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: /d/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 24 Jul 2020 13:02:38 -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; -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
