On Sat, Feb 25, 2017 at 07:24:48PM +0100, Jeremie Courreges-Anglas wrote:
> Jeremie Courreges-Anglas <[email protected]> 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;