NetBSD Problem Report #32373
From greg@bitlynx.com Fri Dec 23 20:54:29 2005
Return-Path: <greg@bitlynx.com>
Received: from S0106004005d7a305.vc.shawcable.net (S0106004005d7a305.vc.shawcable.net [24.84.70.65])
by narn.netbsd.org (Postfix) with ESMTP id E3D0D63B91E
for <gnats-bugs@gnats.NetBSD.org>; Fri, 23 Dec 2005 20:54:27 +0000 (UTC)
Message-Id: <a6a72cd45e8b6c270b2a9cf0920d4cfd@bitlynx.com>
Date: Fri, 23 Dec 2005 12:54:25 -0800
From: Gregory McGarry <greg@bitlynx.com>
Reply-To:
To: gnats-bugs@netbsd.org
Subject: Re: net/25827 (DNS service discovery in getaddrinfo())
>Number: 32373
>Category: lib
>Synopsis: Re: net/25827 (DNS service discovery in getaddrinfo())
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: dyoung
>State: closed
>Class: sw-bug
>Submitter-Id: unknown
>Arrival-Date: Fri Dec 23 20:55:00 +0000 2005
>Closed-Date: Mon Jun 02 14:26:13 +0000 2014
>Last-Modified: Mon Jun 02 14:26:13 +0000 2014
>Originator: Gregory McGarry <greg@bitlynx.com>
>Release:
>Organization:
>Environment:
>Description:
--Apple-Mail-16-297759556
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=US-ASCII;
format=flowed
Here is a revision of the previous patch. Changes include:
- KNF
- refactored to introduce _dns_query(), making everything easier to
comprehend
- removed random shuffling of answers based on equal SRV weights
- applies cleanly to revision 1.75, which includes sorting of addrinfo
results
I removed the random shuffling of weights, since I think it's quite
heavy to read from /dev/urandom, for little additional benefit. The
feature is easy to add back, if necessary.
I encourage this patch to be applied and try and get widespread
acceptance across the other BSDs. This feature is already available
(and used) on Mac OS X. The original NetBSD patch provided the AI_SRV
hint to control SRV lookups. Apple's interface always does SRV
lookups. I think NetBSD should adopt the OS X precedent and always do
SRV lookups too. This is achieved by not applying the patch to
include/netdb.h (only apply this patch).
See http://mail-index.netbsd.org/tech-net/2004/06/ for the discussion.
I know there was some critical comments on this change, but the change
does satisfy requirements as the RFCs currently stand. Nobody said the
change should not be accepted.
--Apple-Mail-16-297759556
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream;
x-unix-mode=0644;
name="gai.diff"
Content-Disposition: attachment;
filename=gai.diff
Index: net/getaddrinfo.c
===================================================================
RCS file: /cvsroot/src/lib/libc/net/getaddrinfo.c,v
retrieving revision 1.75
diff -u -r1.75 getaddrinfo.c
--- net/getaddrinfo.c 2 Dec 2005 11:22:09 -0000 1.75
+++ net/getaddrinfo.c 23 Dec 2005 20:50:50 -0000
@@ -1,4 +1,4 @@
-/* $NetBSD: getaddrinfo.c,v 1.75 2005/12/02 11:22:09 yamt Exp $ */
+/* $NetBSD$ */
/* $KAME: getaddrinfo.c,v 1.29 2000/08/31 17:26:57 itojun Exp $ */
/*
@@ -100,6 +100,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <fcntl.h>
#include <syslog.h>
#include <stdarg.h>
@@ -212,6 +213,13 @@
int n; /* result length */
};
+struct srvinfo {
+ struct srvinfo *next;
+ char name[MAXDNAME];
+ int port, pri, weight;
+};
+
+static int gai_srvok (const char *);
static int str2number(const char *);
static int explore_fqdn(const struct addrinfo *, const char *,
const char *, struct addrinfo **);
@@ -235,11 +243,17 @@
static struct addrinfo *getanswer(const querybuf *, int, const char *, int,
const struct addrinfo *);
static void aisort(struct addrinfo *s, res_state res);
+static struct addrinfo * _dns_query(struct res_target *,
+ const struct addrinfo *, res_state, int);
+static struct addrinfo * _dns_srv_lookup(const char *, const char *,
+ const struct addrinfo *);
+static struct addrinfo * _dns_host_lookup(const char *,
+ const struct addrinfo *);
static int _dns_getaddrinfo(void *, void *, va_list);
static void _sethtent(FILE **);
static void _endhtent(FILE **);
static struct addrinfo *_gethtent(FILE **, const char *,
- const struct addrinfo *);
+ const struct addrinfo *);
static int _files_getaddrinfo(void *, void *, va_list);
#ifdef YP
static struct addrinfo *_yphostent(char *, const struct addrinfo *);
@@ -335,6 +349,43 @@
} while (ai);
}
+#define PERIOD 0x2e
+#define hyphenchar(c) ((c) == 0x2d)
+#define periodchar(c) ((c) == PERIOD)
+#define underschar(c) ((c) == 0x5f)
+#define alphachar(c) (((c) >= 0x41 && (c) <= 0x5a) \
+ || ((c) >= 0x61 && (c) <= 0x7a))
+#define digitchar(c) ((c) >= 0x30 && (c) <= 0x39)
+
+#define firstchar(c) (alphachar(c) || digitchar(c) || underschar(c))
+#define lastchar(c) (alphachar(c) || digitchar(c))
+#define middlechar(c) (lastchar(c) || hyphenchar(c))
+
+static int
+gai_srvok(const char *dn)
+{
+ int pch = PERIOD, ch = *dn++;
+
+ while (ch != '\0') {
+ int nch = *dn++;
+
+ if (periodchar(ch)) {
+ ;
+ } else if (periodchar(pch)) {
+ if (!firstchar(ch))
+ return (0);
+ } else if (periodchar(nch) || nch == '\0') {
+ if (!lastchar(ch))
+ return (0);
+ } else {
+ if (!middlechar(ch))
+ return (0);
+ }
+ pch = ch, ch = nch;
+ }
+ return (1);
+}
+
static int
str2number(const char *p)
{
@@ -586,7 +637,7 @@
return 0;
switch (nsdispatch(&result, dtab, NSDB_HOSTS, "getaddrinfo",
- default_dns_files, hostname, pai)) {
+ default_dns_files, hostname, pai, servname)) {
case NS_TRYAGAIN:
error = EAI_AGAIN;
goto free;
@@ -599,6 +650,17 @@
case NS_SUCCESS:
error = 0;
for (cur = result; cur; cur = cur->ai_next) {
+ /* Check for already filled port. */
+ switch (cur->ai_family) {
+ case AF_INET:
+ if (((struct sockaddr_in*)(void*)cur->ai_addr)->sin_port)
+ continue;
+ break;
+ case AF_INET6:
+ if (((struct sockaddr_in6*)(void*)cur->ai_addr)->sin6_port)
+ continue;
+ break;
+ }
GET_PORT(cur, servname);
/* canonname should be filled already */
}
@@ -1054,7 +1116,7 @@
const struct addrinfo *pai)
{
struct addrinfo sentinel, *cur;
- struct addrinfo ai;
+ struct addrinfo ai, *aip;
const struct afd *afd;
char *canonname;
const HEADER *hp;
@@ -1067,6 +1129,8 @@
char tbuf[MAXDNAME];
int (*name_ok) (const char *);
char hostbuf[8*1024];
+ int port, pri, weight;
+ struct srvinfo *srvlist, *srv, *csrv;
_DIAGASSERT(answer != NULL);
_DIAGASSERT(qname != NULL);
@@ -1083,6 +1147,9 @@
case T_ANY: /*use T_ANY only for T_A/T_AAAA lookup*/
name_ok = res_hnok;
break;
+ case T_SRV:
+ name_ok = gai_srvok;
+ break;
default:
return NULL; /* XXX should be abort(); */
}
@@ -1122,6 +1189,7 @@
}
haveanswer = 0;
had_error = 0;
+ srvlist = NULL;
while (ancount-- > 0 && cp < eom && !had_error) {
n = dn_expand(answer->buf, eom, cp, bp, ep - bp);
if ((n < 0) || !(*name_ok)(bp)) {
@@ -1221,17 +1289,130 @@
cur = cur->ai_next;
cp += n;
break;
+ case T_SRV:
+ /* Add to SRV list. Insertion sort on priority. */
+ pri = _getshort(cp);
+ cp += INT16SZ;
+ weight = _getshort(cp);
+ cp += INT16SZ;
+ port = _getshort(cp);
+ cp += INT16SZ;
+ n = dn_expand(answer->buf, eom, cp, tbuf, sizeof tbuf);
+ if ((n < 0) || !res_hnok(tbuf)) {
+ had_error++;
+ continue;
+ }
+ cp += n;
+ n = strlen(tbuf) + 1; /* for the \0 */
+ if (n >= MAXDNAME) {
+ had_error++;
+ continue;
+ }
+ srv = malloc(sizeof(struct srvinfo));
+ if (!srv) {
+ had_error++;
+ continue;
+ }
+ strlcpy(srv->name, tbuf, sizeof (srv->name));
+ srv->pri = pri;
+ srv->weight = weight;
+ srv->port = port;
+ /* Weight 0 is sorted before other weights. */
+ if (!srvlist
+ || srv->pri < srvlist->pri
+ || (srv->pri == srvlist->pri && (!srv->weight || srvlist->weight))) {
+ srv->next = srvlist;
+ srvlist = srv;
+ } else {
+ for (csrv = srvlist;
+ csrv->next && csrv->next->pri <= srv->pri;
+ csrv = csrv->next) {
+ if (csrv->next->pri == srv->pri
+ && (!srv->weight || csrv->next->weight))
+ break;
+ }
+ srv->next = csrv->next;
+ csrv->next = srv;
+ }
+ continue; /* Don't add to haveanswer yet. */
default:
abort();
}
if (!had_error)
haveanswer++;
}
+
+ if (srvlist) {
+
+ res_state res;
+
+ /*
+ * Check for explicit rejection.
+ */
+ if (!srvlist->next && !srvlist->name[0]) {
+ free(srvlist);
+ h_errno = HOST_NOT_FOUND;
+ return NULL;
+ }
+
+ res = __res_get_state();
+ if (res == NULL) {
+ h_errno = NETDB_INTERNAL;
+ return NULL;
+ }
+
+ while (srvlist) {
+
+ struct res_target q, q2;
+
+ srv = srvlist;
+ srvlist = srvlist->next;
+
+ /*
+ * Since res_* doesn't give the additional
+ * section, we always look up.
+ */
+ memset(&q, 0, sizeof(q));
+ memset(&q2, 0, sizeof(q2));
+
+ q.name = srv->name;
+ q.qclass = C_IN;
+ q.qtype = T_AAAA;
+ q.next = &q2;
+ q2.name = srv->name;
+ q2.qclass = C_IN;
+ q2.qtype = T_A;
+
+ aip = _dns_query(&q, pai, res, 0);
+
+ if (aip != NULL) {
+
+ cur->ai_next = aip;
+ while (cur && cur->ai_next) {
+ cur = cur->ai_next;
+ switch (cur->ai_family) {
+ case AF_INET:
+ ((struct sockaddr_in*)(void*)cur->ai_addr)->sin_port = htons(srv->port);
+ break;
+ case AF_INET6:
+ ((struct sockaddr_in6*)(void*)cur->ai_addr)->sin6_port = htons(srv->port);
+ break;
+ }
+ haveanswer++;
+ }
+ }
+
+ free(srv);
+ }
+ }
if (haveanswer) {
- if (!canonname)
- (void)get_canonname(pai, sentinel.ai_next, qname);
- else
- (void)get_canonname(pai, sentinel.ai_next, canonname);
+ if (!sentinel.ai_next->ai_canonname)
+ {
+ if (!canonname)
+ (void)get_canonname(pai, sentinel.ai_next, qname);
+ else
+ (void)get_canonname(pai, sentinel.ai_next, canonname);
+ }
h_errno = NETDB_SUCCESS;
return sentinel.ai_next;
}
@@ -1271,37 +1452,176 @@
s->ai_next = head.ai_next;
}
-/*ARGSUSED*/
-static int
-_dns_getaddrinfo(void *rv, void *cb_data, va_list ap)
+
+static struct addrinfo *
+_dns_query(struct res_target *q,
+ const struct addrinfo *pai,
+ res_state res,
+ int dosearch)
{
- struct addrinfo *ai;
+ struct res_target *q2 = q->next;
querybuf *buf, *buf2;
- const char *name;
- const struct addrinfo *pai;
- struct addrinfo sentinel, *cur;
- struct res_target q, q2;
- res_state res;
+ struct addrinfo sentinel, *cur, *ai;
- name = va_arg(ap, char *);
- pai = va_arg(ap, const struct addrinfo *);
-
- memset(&q, 0, sizeof(q2));
- memset(&q2, 0, sizeof(q2));
- memset(&sentinel, 0, sizeof(sentinel));
- cur = &sentinel;
+#ifdef DEBUG
+ struct res_target *iter;
+ for (iter = q; iter; iter = iter->next)
+ printf("Query type %d for %s\n", iter->qtype, iter->name);
+#endif
buf = malloc(sizeof(*buf));
if (buf == NULL) {
h_errno = NETDB_INTERNAL;
- return NS_NOTFOUND;
+ return NULL;
}
buf2 = malloc(sizeof(*buf2));
if (buf2 == NULL) {
free(buf);
h_errno = NETDB_INTERNAL;
- return NS_NOTFOUND;
+ return NULL;
+ }
+
+ memset(&sentinel, 0, sizeof(sentinel));
+ cur = &sentinel;
+
+ q->answer = buf->buf;
+ q->anslen = sizeof(buf->buf);
+ if (q2) {
+ q2->answer = buf2->buf;
+ q2->anslen = sizeof(buf2->buf);
+ }
+
+ if (dosearch) {
+
+ if (res_searchN(q->name, q, res) < 0) {
+ free(buf);
+ free(buf2);
+ return NULL;
+ }
+
+ } else {
+
+ if (res_queryN(q->name, q, res) < 0) {
+ free(buf);
+ free(buf2);
+ return NULL;
+ }
+
+ }
+
+ ai = getanswer(buf, q->n, q->name, q->qtype, pai);
+ if (ai) {
+ cur->ai_next = ai;
+ while (cur && cur->ai_next)
+ cur = cur->ai_next;
+ }
+ if (q2) {
+ ai = getanswer(buf2, q2->n, q2->name, q2->qtype, pai);
+ if (ai)
+ cur->ai_next = ai;
}
+ free(buf);
+ free(buf2);
+
+ return sentinel.ai_next;
+}
+
+/*ARGSUSED*/
+static struct addrinfo *
+_dns_srv_lookup(const char *name, const char *servname, const struct addrinfo *pai)
+{
+ const char * const srvprotos[] = { "tcp", "udp" };
+ const int srvnottype[] = { SOCK_DGRAM, SOCK_STREAM };
+ const int nsrvprotos = 2;
+ struct addrinfo sentinel, *cur, *ai;
+ struct servent *serv;
+ struct res_target q;
+ res_state res;
+ char *tname;
+ int i;
+
+ res = __res_get_state();
+ if (res == NULL)
+ return NULL;
+
+ memset(&sentinel, 0, sizeof(sentinel));
+ cur = &sentinel;
+
+ /*
+ * Iterate over supported SRV protocols.
+ * (currently UDP and TCP only)
+ */
+ for (i = 0; i < nsrvprotos; i++) {
+
+ /*
+ * Check that the caller didn't specify a hint
+ * which precludes this protocol.
+ */
+ if (pai->ai_socktype == srvnottype[i])
+ continue;
+
+ /*
+ * If the caller specified a port,
+ * then lookup the database for the
+ * official service name.
+ */
+ serv = getservbyname(servname, srvprotos[i]);
+ if (serv == NULL)
+ continue;
+
+ tname = malloc(strlen(name) + strlen(serv->s_proto) + strlen(serv->s_name) + 5);
+ if (tname == NULL)
+ continue;
+
+ /*
+ * Construct service DNS name.
+ */
+ strcpy(tname, "_");
+ strcat(tname, serv->s_name);
+ strcat(tname, "._");
+ strcat(tname, serv->s_proto);
+ strcat(tname, ".");
+ strcat(tname, name);
+
+ memset(&q, 0, sizeof(q));
+ q.name = tname;
+ q.qclass = C_IN;
+ q.qtype = T_SRV;
+
+ /*
+ * Do SRV query.
+ */
+ ai = _dns_query(&q, pai, res, 1);
+ if (ai) {
+ cur->ai_next = ai;
+ while (cur && cur->ai_next)
+ cur = cur->ai_next;
+ }
+ free(tname);
+ }
+
+ if (res->nsort)
+ aisort(&sentinel, res);
+
+ __res_put_state(res);
+
+ return sentinel.ai_next;
+}
+
+/*ARGSUSED*/
+static struct addrinfo *
+_dns_host_lookup(const char *name, const struct addrinfo *pai)
+{
+ struct res_target q, q2;
+ struct addrinfo sentinel, *ai;
+ res_state res;
+
+ res = __res_get_state();
+ if (res == NULL)
+ return NULL;
+
+ memset(&q, 0, sizeof(q2));
+ memset(&q2, 0, sizeof(q2));
switch (pai->ai_family) {
case AF_UNSPEC:
@@ -1309,79 +1629,90 @@
q.name = name;
q.qclass = C_IN;
q.qtype = T_AAAA;
- q.answer = buf->buf;
- q.anslen = sizeof(buf->buf);
q.next = &q2;
q2.name = name;
q2.qclass = C_IN;
q2.qtype = T_A;
- q2.answer = buf2->buf;
- q2.anslen = sizeof(buf2->buf);
break;
case AF_INET:
q.name = name;
q.qclass = C_IN;
q.qtype = T_A;
- q.answer = buf->buf;
- q.anslen = sizeof(buf->buf);
break;
case AF_INET6:
q.name = name;
q.qclass = C_IN;
q.qtype = T_AAAA;
- q.answer = buf->buf;
- q.anslen = sizeof(buf->buf);
break;
default:
- free(buf);
- free(buf2);
- return NS_UNAVAIL;
+ h_errno = NETDB_INTERNAL;
+ return NULL;
}
- res = __res_get_state();
- if (res == NULL) {
- free(buf);
- free(buf2);
- return NS_NOTFOUND;
- }
+ ai = _dns_query(&q, pai, res, 1);
- if (res_searchN(name, &q, res) < 0) {
- __res_put_state(res);
- free(buf);
- free(buf2);
- return NS_NOTFOUND;
- }
- ai = getanswer(buf, q.n, q.name, q.qtype, pai);
- if (ai) {
- cur->ai_next = ai;
- while (cur && cur->ai_next)
- cur = cur->ai_next;
- }
- if (q.next) {
- ai = getanswer(buf2, q2.n, q2.name, q2.qtype, pai);
- if (ai)
- cur->ai_next = ai;
- }
- free(buf);
- free(buf2);
- if (sentinel.ai_next == NULL) {
- __res_put_state(res);
- switch (h_errno) {
- case HOST_NOT_FOUND:
- return NS_NOTFOUND;
- case TRY_AGAIN:
- return NS_TRYAGAIN;
- default:
- return NS_UNAVAIL;
- }
- }
+ memset(&sentinel, 0, sizeof(sentinel));
+ sentinel.ai_next = ai;
- if (res->nsort)
- aisort(&sentinel, res);
+ if (ai != NULL && res->nsort)
+ aisort(&sentinel, res);
__res_put_state(res);
- *((struct addrinfo **)rv) = sentinel.ai_next;
+ return sentinel.ai_next;
+}
+
+/*ARGSUSED*/
+static int
+_dns_getaddrinfo(void *rv, void *cb_data, va_list ap)
+{
+ struct addrinfo *ai = NULL;
+ const char *name, *servname;
+ const struct addrinfo *pai;
+
+ name = va_arg(ap, char *);
+ pai = va_arg(ap, const struct addrinfo *);
+ servname = va_arg(ap, char *);
+
+ /*
+ * Try doing SRV lookup on service first.
+ */
+ if (servname
+#ifdef AI_SRV
+ && (pai->ai_flags & AI_SRV)
+#endif
+ && !(pai->ai_flags & AI_NUMERICSERV)
+ && str2number(servname) == -1) {
+
+#ifdef DEBUG
+ printf("_dns_getaddrinfo: try SRV lookup\n");
+#endif
+ ai = _dns_srv_lookup(name, servname, pai);
+ }
+
+ /*
+ * Do lookup on name.
+ */
+ if (ai == NULL) {
+
+#ifdef DEBUG
+ printf("_dns_getaddrinfo: try HOST lookup\n");
+#endif
+ ai = _dns_host_lookup(name, pai);
+
+ if (ai == NULL) {
+ switch (h_errno) {
+ case HOST_NOT_FOUND:
+ return NS_NOTFOUND;
+ case TRY_AGAIN:
+ return NS_TRYAGAIN;
+ default:
+ return NS_UNAVAIL;
+ }
+ }
+ }
+
+ *((struct addrinfo **)rv) = ai;
return NS_SUCCESS;
}
--Apple-Mail-16-297759556--
>How-To-Repeat:
>Fix:
Unknown
>Release-Note:
>Audit-Trail:
State-Changed-From-To: open->closed
State-Changed-By: spz@netbsd.org
State-Changed-When: Wed, 27 Sep 2006 18:25:10 +0000
State-Changed-Why:
should have been an addendum to 25827
Responsible-Changed-From-To: gnats-admin->dyoung
Responsible-Changed-By: dyoung@NetBSD.org
Responsible-Changed-When: Tue, 03 Mar 2009 01:03:45 +0000
Responsible-Changed-Why:
take.
State-Changed-From-To: closed->open
State-Changed-By: dyoung@NetBSD.org
State-Changed-When: Tue, 03 Mar 2009 01:03:45 +0000
State-Changed-Why:
This PR contains a valuable patch.
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/32373 CVS commit: src/lib/libc/net
Date: Fri, 3 May 2013 15:31:14 -0400
Module Name: src
Committed By: christos
Date: Fri May 3 19:31:13 UTC 2013
Modified Files:
src/lib/libc/net: getaddrinfo.c
Log Message:
PR/32373, PR/25827: Add SRV lookup in getaddrinfo(3)
Per DNS-SD (RFC 2782), but only enabled if AI_SRV is set.
To generate a diff of this commit:
cvs rdiff -u -r1.102 -r1.103 src/lib/libc/net/getaddrinfo.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/32373 CVS commit: src/include
Date: Fri, 3 May 2013 15:34:13 -0400
Module Name: src
Committed By: christos
Date: Fri May 3 19:34:13 UTC 2013
Modified Files:
src/include: netdb.h
Log Message:
PR/25827, PR/32373: addr AI_SRV
To generate a diff of this commit:
cvs rdiff -u -r1.66 -r1.67 src/include/netdb.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 07 Oct 2013 01:15:00 +0000
State-Changed-Why:
Christos committed it; ok to close?
State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 02 Jun 2014 14:26:13 +0000
State-Changed-Why:
Feedback timeout.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home