On Sat, Feb 25, 2017 at 07:24:48PM +0100, 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.

Right. It has to be set a the query level.

> 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.
> 

Well, I like the as_flags cleanup part of the diff. Internal flags should
indeed be on the asr_query structure directly, so they are not confused
with query-specific parameters.

For the flags inheritance, I'm not so sure.  I would rather keep the current
internal API for now, and set the flags explicitely for this specific case.

Eric.

Index: getrrsetbyname_async.c
===================================================================
RCS file: /cvs/src/lib/libc/asr/getrrsetbyname_async.c,v
retrieving revision 1.11
diff -u -p -r1.11 getrrsetbyname_async.c
--- getrrsetbyname_async.c      23 Feb 2017 17:04:02 -0000      1.11
+++ getrrsetbyname_async.c      26 Feb 2017 09:33:47 -0000
@@ -104,6 +104,7 @@ getrrsetbyname_async_run(struct asr_quer
                        async_set_state(as, ASR_STATE_HALT);
                        break;
                }
+               as->as_subq->as_flags |= ASYNC_DNSSEC;
 
                async_set_state(as, ASR_STATE_SUBQUERY);
                break;

Reply via email to