Hi,

I'm not the best in reading patches, so I'm going to query you.  Does
your patch check for the "AD" flag from the resolver?  As basically a
DNSSEC able recursive nameserver should set this meaning it has
authenticated the data.  I wrote a patch for DNSSEC (possibly erroneous
by comparing it to you) and posted it to #opensmtpd in hopes that eric
would see it.  Much of that functionality is superfluous now but it does
have an "AD_MASK" check.

Here is my patch from last year, which I gave up on, feel free to cherry
pick anything needed out of it.  You'll see some similarities but they
are different enough to show two different peoples work.

http://centroid.eu/private/dnssec.patch.txt

Yours is a lot more complete of course.

Cheers,

-peter

On 02/25/17 19:24, Jeremie Courreges-Anglas wrote:
> Jeremie Courreges-Anglas <j...@wxcvbn.org> writes:
>
>> This flag is useful for software that wants to rely on the resolver to
>> perform DNSSEC validation.  Among the use cases there are DANE and SSHFP
>> records, and the obvious interfaces that I think are useful are
>> res_mkquery and getrrsetbyname.  The latter still doesn't support
>> DNSSEC, another diff will follow.
> Diff on top of the previous one.  I'd like to make getrrsetbyname(3)
> "use" RES_USE_DNSSEC again.  This would improve support for ssh -o
> VerifyHostKeyDNS (SSHFP records).
>
> The "easy" way would be something like:
>
> Index: getrrsetbyname_async.c
> ===================================================================
> RCS file: /d/cvs/src/lib/libc/asr/getrrsetbyname_async.c,v
> retrieving revision 1.11
> diff -u -p -p -u -r1.11 getrrsetbyname_async.c
> --- getrrsetbyname_async.c    23 Feb 2017 17:04:02 -0000      1.11
> +++ getrrsetbyname_async.c    25 Feb 2017 17:25:25 -0000
> @@ -42,6 +42,7 @@ getrrsetbyname_async(const char *hostnam
>       struct asr_query *as;
>  
>       ac = _asr_use_resolver(asr);
> +     ac->ac_options |= RES_USE_DNSSEC;
>       if ((as = _asr_async_new(ac, ASR_GETRRSETBYNAME)) == NULL)
>               goto abort; /* errno set */
>       as->as_run = getrrsetbyname_async_run;
>
> IIUC this means that we modify the thread-local resolver options used by
> subsequent queries.  Cleaning up by resetting the flag before returning
> doesn't work in all cases because you could have overlap between two
> active getrrsetbyname_async and eg getaddrinfo_async contexts.
>
> The diff below instead adds an as_flags member to struct asr_query, and
> merges the flags of the various union members.  struct rrset and struct
> ni keep their flags member, as they are a different kind of flags.
> A subset of as_flags is passed down to the child ASR_SEND subq, the only
> flag that is inherited right now is ASYNC_DNSSEC, which allows
> getrrsetbyname_async to communicate its intent.
>
> That's a bit of churn for a small improvement, maybe there is a simpler
> diff?
>
> Comments welcome.
>
>
> diff -pruN asr.1/asr.c asr/asr.c
> --- asr.1/asr.c       Sat Feb 25 17:57:40 2017
> +++ asr/asr.c Sat Feb 25 17:58:10 2017
> @@ -244,7 +244,7 @@ _asr_async_free(struct asr_query *as)
>       case ASR_SEND:
>               if (as->as_fd != -1)
>                       close(as->as_fd);
> -             if (as->as.dns.obuf && !(as->as.dns.flags & ASYNC_EXTOBUF))
> +             if (as->as.dns.obuf && !(as->as_flags & ASYNC_EXTOBUF))
>                       free(as->as.dns.obuf);
>               if (as->as.dns.ibuf)
>                       free(as->as.dns.ibuf);
> diff -pruN asr.1/asr_private.h asr/asr_private.h
> --- asr.1/asr_private.h       Sat Feb 25 17:57:40 2017
> +++ asr/asr_private.h Sat Feb 25 18:12:23 2017
> @@ -156,15 +156,19 @@ struct asr {
>  #define      ASYNC_NODATA            0x00000100
>  #define      ASYNC_AGAIN             0x00000200
>  
> +#define      ASYNC_DNSSEC            0x00001000
>  #define      ASYNC_EXTOBUF           0x00002000
>  
>  #define      ASYNC_NO_INET           0x00010000
>  #define      ASYNC_NO_INET6          0x00020000
>  
> +#define      ASYNC_ASR_SEND_MASK     (ASYNC_DNSSEC)
> +
>  struct asr_query {
>       int             (*as_run)(struct asr_query *, struct asr_result *);
>       struct asr_ctx  *as_ctx;
>       int              as_type;
> +     int              as_flags;
>       int              as_state;
>  
>       /* cond */
> @@ -183,7 +187,6 @@ struct asr_query {
>  
>       union {
>               struct {
> -                     int              flags;
>                       uint16_t         reqid;
>                       int              class;
>                       int              type;
> @@ -206,7 +209,6 @@ struct asr_query {
>               } dns;
>  
>               struct {
> -                     int              flags;
>                       int              class;
>                       int              type;
>                       char            *name;
> @@ -249,7 +251,6 @@ struct asr_query {
>                       char            *fqdn;
>                       struct addrinfo *aifirst;
>                       struct addrinfo *ailast;
> -                     int              flags;
>               } ai;
>  
>               struct {
> @@ -319,7 +320,8 @@ int _asr_iter_db(struct asr_query *);
>  int _asr_parse_namedb_line(FILE *, char **, int, char *, size_t);
>  
>  /* *_async.c */
> -struct asr_query *_res_query_async_ctx(const char *, int, int, struct 
> asr_ctx *);
> +struct asr_query *_res_query_async_ctx(const char *, int, int,
> +    struct asr_ctx *, int);
>  struct asr_query *_res_search_async_ctx(const char *, int, int, struct 
> asr_ctx *);
>  struct asr_query *_gethostbyaddr_async_ctx(const void *, socklen_t, int,
>      struct asr_ctx *);
> diff -pruN asr.1/getaddrinfo_async.c asr/getaddrinfo_async.c
> --- asr.1/getaddrinfo_async.c Sat Feb 25 17:57:40 2017
> +++ asr/getaddrinfo_async.c   Sat Feb 25 18:09:17 2017
> @@ -356,18 +356,18 @@ getaddrinfo_async_run(struct asr_query *as, struct asr
>                           AS_FAMILY(as) : as->as.ai.hints.ai_family;
>  
>                       if (family == AF_INET &&
> -                         as->as.ai.flags & ASYNC_NO_INET) {
> +                         as->as_flags & ASYNC_NO_INET) {
>                               async_set_state(as, ASR_STATE_NEXT_FAMILY);
>                               break;
>                       } else if (family == AF_INET6 &&
> -                         as->as.ai.flags & ASYNC_NO_INET6) {
> +                         as->as_flags & ASYNC_NO_INET6) {
>                               async_set_state(as, ASR_STATE_NEXT_FAMILY);
>                               break;
>                       }
>  
>                       as->as_subq = _res_query_async_ctx(as->as.ai.fqdn,
>                           C_IN, (family == AF_INET6) ? T_AAAA : T_A,
> -                         as->as_ctx);
> +                         as->as_ctx, as->as_flags);
>  
>                       if (as->as_subq == NULL) {
>                               if (errno == ENOMEM)
> @@ -431,7 +431,7 @@ getaddrinfo_async_run(struct asr_query *as, struct asr
>  
>       case ASR_STATE_NOT_FOUND:
>               /* No result found. Maybe we can try again. */
> -             if (as->as.ai.flags & ASYNC_AGAIN)
> +             if (as->as_flags & ASYNC_AGAIN)
>                       ar->ar_gai_errno = EAI_AGAIN;
>               else
>                       ar->ar_gai_errno = EAI_NODATA;
> @@ -684,7 +684,7 @@ addrconfig_setup(struct asr_query *as)
>       if (getifaddrs(&ifa0) == -1)
>               return (-1);
>  
> -     as->as.ai.flags |= ASYNC_NO_INET | ASYNC_NO_INET6;
> +     as->as_flags |= ASYNC_NO_INET | ASYNC_NO_INET6;
>  
>       for (ifa = ifa0; ifa != NULL; ifa = ifa->ifa_next) {
>               if (ifa->ifa_addr == NULL)
> @@ -697,7 +697,7 @@ addrconfig_setup(struct asr_query *as)
>                       if (sinp->sin_addr.s_addr == htonl(INADDR_LOOPBACK))
>                               continue;
>  
> -                     as->as.ai.flags &= ~ASYNC_NO_INET;
> +                     as->as_flags &= ~ASYNC_NO_INET;
>                       break;
>               case PF_INET6:
>                       sin6p = (struct sockaddr_in6 *)ifa->ifa_addr;
> @@ -708,7 +708,7 @@ addrconfig_setup(struct asr_query *as)
>                       if (IN6_IS_ADDR_LINKLOCAL(&sin6p->sin6_addr))
>                               continue;
>  
> -                     as->as.ai.flags &= ~ASYNC_NO_INET6;
> +                     as->as_flags &= ~ASYNC_NO_INET6;
>                       break;
>               }
>       }
> diff -pruN asr.1/gethostnamadr_async.c asr/gethostnamadr_async.c
> --- asr.1/gethostnamadr_async.c       Sat Feb 25 17:57:40 2017
> +++ asr/gethostnamadr_async.c Sat Feb 25 18:09:34 2017
> @@ -223,7 +223,8 @@ gethostnamadr_async_run(struct asr_query *as, struct a
>                                   as->as.hostnamadr.family,
>                                   name, sizeof(name));
>                               as->as_subq = _res_query_async_ctx(
> -                                 name, C_IN, T_PTR, as->as_ctx);
> +                                 name, C_IN, T_PTR, as->as_ctx,
> +                                 as->as_flags);
>                       }
>  
>                       if (as->as_subq == NULL) {
> diff -pruN asr.1/getnetnamadr_async.c asr/getnetnamadr_async.c
> --- asr.1/getnetnamadr_async.c        Sat Feb 25 17:57:40 2017
> +++ asr/getnetnamadr_async.c  Sat Feb 25 18:09:41 2017
> @@ -166,7 +166,7 @@ getnetnamadr_async_run(struct asr_query *as, struct as
>                                   as->as.netnamadr.family,
>                                   dname, sizeof(dname));
>                               as->as_subq = _res_query_async_ctx(
> -                                 name, C_IN, type, as->as_ctx);
> +                                 name, C_IN, type, as->as_ctx, as->as_flags);
>                       }
>  
>                       if (as->as_subq == NULL) {
> diff -pruN asr.1/getrrsetbyname_async.c asr/getrrsetbyname_async.c
> --- asr.1/getrrsetbyname_async.c      Sat Feb 25 17:57:40 2017
> +++ asr/getrrsetbyname_async.c        Sat Feb 25 18:09:57 2017
> @@ -45,6 +45,7 @@ getrrsetbyname_async(const char *hostname, unsigned in
>       if ((as = _asr_async_new(ac, ASR_GETRRSETBYNAME)) == NULL)
>               goto abort; /* errno set */
>       as->as_run = getrrsetbyname_async_run;
> +     as->as_flags |= ASYNC_DNSSEC;
>  
>       as->as.rrset.flags = flags;
>       as->as.rrset.class = rdclass;
> @@ -98,7 +99,8 @@ getrrsetbyname_async_run(struct asr_query *as, struct 
>                   as->as.rrset.name,
>                   as->as.rrset.class,
>                   as->as.rrset.type,
> -                 as->as_ctx);
> +                 as->as_ctx,
> +                 as->as_flags);
>               if (as->as_subq == NULL) {
>                       ar->ar_rrset_errno = ERRSET_FAIL;
>                       async_set_state(as, ASR_STATE_HALT);
> diff -pruN asr.1/res_search_async.c asr/res_search_async.c
> --- asr.1/res_search_async.c  Sat Feb 25 17:57:40 2017
> +++ asr/res_search_async.c    Sat Feb 25 18:10:11 2017
> @@ -121,7 +121,8 @@ res_search_async_run(struct asr_query *as, struct asr_
>                       break;
>               }
>               as->as_subq = _res_query_async_ctx(fqdn,
> -                 as->as.search.class, as->as.search.type, as->as_ctx);
> +                 as->as.search.class, as->as.search.type, as->as_ctx,
> +                 as->as_flags);
>               if (as->as_subq == NULL) {
>                       ar->ar_errno = errno;
>                       if (errno == EINVAL)
> @@ -169,9 +170,9 @@ res_search_async_run(struct asr_query *as, struct asr_
>  
>               if (as->as_dom_flags & ASYNC_DOM_DOMAIN) {
>                       if (ar->ar_h_errno == NO_DATA)
> -                             as->as.search.flags |= ASYNC_NODATA;
> +                             as->as_flags |= ASYNC_NODATA;
>                       else if (ar->ar_h_errno == TRY_AGAIN)
> -                             as->as.search.flags |= ASYNC_AGAIN;
> +                             as->as_flags |= ASYNC_AGAIN;
>               }
>  
>               async_set_state(as, ASR_STATE_NEXT_DOMAIN);
> @@ -181,9 +182,9 @@ res_search_async_run(struct asr_query *as, struct asr_
>  
>               if (as->as.search.saved_h_errno != HERRNO_UNSET)
>                       ar->ar_h_errno = as->as.search.saved_h_errno;
> -             else if (as->as.search.flags & ASYNC_NODATA)
> +             else if (as->as_flags & ASYNC_NODATA)
>                       ar->ar_h_errno = NO_DATA;
> -             else if (as->as.search.flags & ASYNC_AGAIN)
> +             else if (as->as_flags & ASYNC_AGAIN)
>                       ar->ar_h_errno = TRY_AGAIN;
>               /*
>                * Else, we got the ar_h_errno value set by res_query_async()
> diff -pruN asr.1/res_send_async.c asr/res_send_async.c
> --- asr.1/res_send_async.c    Sat Feb 25 17:57:40 2017
> +++ asr/res_send_async.c      Sat Feb 25 19:13:13 2017
> @@ -67,7 +67,7 @@ res_send_async(const unsigned char *buf, int buflen, v
>       }
>       as->as_run = res_send_async_run;
>  
> -     as->as.dns.flags |= ASYNC_EXTOBUF;
> +     as->as_flags |= ASYNC_EXTOBUF;
>       as->as.dns.obuf = (unsigned char *)buf;
>       as->as.dns.obuflen = buflen;
>       as->as.dns.obufsize = buflen;
> @@ -110,7 +110,7 @@ res_query_async(const char *name, int class, int type,
>       DPRINT("asr: res_query_async(\"%s\", %i, %i)\n", name, class, type);
>  
>       ac = _asr_use_resolver(asr);
> -     as = _res_query_async_ctx(name, class, type, ac);
> +     as = _res_query_async_ctx(name, class, type, ac, 0);
>       _asr_ctx_unref(ac);
>  
>       return (as);
> @@ -118,7 +118,8 @@ res_query_async(const char *name, int class, int type,
>  DEF_WEAK(res_query_async);
>  
>  struct asr_query *
> -_res_query_async_ctx(const char *name, int class, int type, struct asr_ctx 
> *a_ctx)
> +_res_query_async_ctx(const char *name, int class, int type,
> +    struct asr_ctx *a_ctx, int a_flags)
>  {
>       struct asr_query        *as;
>  
> @@ -127,6 +128,7 @@ _res_query_async_ctx(const char *name, int class, int 
>       if ((as = _asr_async_new(a_ctx, ASR_SEND)) == NULL)
>               return (NULL); /* errno set */
>       as->as_run = res_send_async_run;
> +     as->as_flags = a_flags & ASYNC_ASR_SEND_MASK;
>  
>       /* This adds a "." to name if it doesn't already has one.
>        * That's how res_query() behaves (through res_mkquery").
> @@ -345,8 +347,9 @@ setup_query(struct asr_query *as, const char *name, co
>       struct asr_dns_header   h;
>       char                    fqdn[MAXDNAME];
>       char                    dname[MAXDNAME];
> +     int                     dnssec_do;
>  
> -     if (as->as.dns.flags & ASYNC_EXTOBUF) {
> +     if (as->as_flags & ASYNC_EXTOBUF) {
>               errno = EINVAL;
>               DPRINT("attempting to write in user packet");
>               return (-1);
> @@ -372,20 +375,22 @@ setup_query(struct asr_query *as, const char *name, co
>       }
>       as->as.dns.obuflen = 0;
>  
> +     dnssec_do = (as->as_ctx->ac_options & RES_USE_DNSSEC) ||
> +             (as->as_flags & ASYNC_DNSSEC);
> +
>       memset(&h, 0, sizeof h);
>       h.id = res_randomid();
>       if (as->as_ctx->ac_options & RES_RECURSE)
>               h.flags |= RD_MASK;
>       h.qdcount = 1;
> -     if (as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC))
> +     if (as->as_ctx->ac_options & RES_USE_EDNS0 || dnssec_do)
>               h.arcount = 1;
>  
>       _asr_pack_init(&p, as->as.dns.obuf, as->as.dns.obufsize);
>       _asr_pack_header(&p, &h);
>       _asr_pack_query(&p, type, class, dname);
> -     if (as->as_ctx->ac_options & (RES_USE_EDNS0 | RES_USE_DNSSEC))
> -             _asr_pack_edns0(&p, MAXPACKETSZ,
> -                 as->as_ctx->ac_options & RES_USE_DNSSEC);
> +     if (as->as_ctx->ac_options & RES_USE_EDNS0 || dnssec_do)
> +             _asr_pack_edns0(&p, MAXPACKETSZ, dnssec_do);
>       if (p.err) {
>               DPRINT("error packing query");
>               errno = EINVAL;
>
>

Reply via email to