Eric Faurot <[email protected]> writes:
> 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.
Fine with me, here's a diff to introduce as_flags only, still on top of
the RES_USE_DNSSEC diff.
diff -x CVS -pruN asr.1/asr.c asr/asr.c
--- asr.1/asr.c Sat Feb 25 17:57:40 2017
+++ asr/asr.c Sun Feb 26 22:43:02 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 -x CVS -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 Sun Feb 26 22:44:06 2017
@@ -165,6 +165,7 @@ 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 +184,6 @@ struct asr_query {
union {
struct {
- int flags;
uint16_t reqid;
int class;
int type;
@@ -206,7 +206,6 @@ struct asr_query {
} dns;
struct {
- int flags;
int class;
int type;
char *name;
@@ -249,7 +248,6 @@ struct asr_query {
char *fqdn;
struct addrinfo *aifirst;
struct addrinfo *ailast;
- int flags;
} ai;
struct {
diff -x CVS -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 Sun Feb 26 22:43:02 2017
@@ -356,11 +356,11 @@ 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;
}
@@ -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 -x CVS -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 Sun Feb 26 22:43:02 2017
@@ -169,9 +169,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 +181,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 -x CVS -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 Sun Feb 26 22:48:50 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;
@@ -346,7 +346,7 @@ setup_query(struct asr_query *as, const char *name, co
char fqdn[MAXDNAME];
char dname[MAXDNAME];
- if (as->as.dns.flags & ASYNC_EXTOBUF) {
+ if (as->as_flags & ASYNC_EXTOBUF) {
errno = EINVAL;
DPRINT("attempting to write in user packet");
return (-1);
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
