Proposed fix for getaddrinfo() issue

Proposed fix for getaddrinfo() issue

[Lutz Jaenicke]

The proposed patch provides a solution for the issue of getaddrinfo()
failing to resolv numeric port numbers if neither the socket type
or the protocol have been provided.
Observed with uClibc, other libraries may or may not have this
issue:
  http://gitorious.org/0xdroid/bionic/blobs/9ab75d4cc803e91b7f1b656ffbe2ad32c52a86f9/libc/netbsd/net/getaddrinfo.c
  -> look for "ANDROID-SPECIFIC CHANGE TO MATCH GLIBC" :-)

Comments are welcome.
  [PATCH] Provide family and protocol to make getaddrinfo happy

[PATCH] Provide family and protocol to make getaddrinfo happy

[Lutz Jaenicke]

getaddrinfo() will fail for numeric port numbers if neither
the socket type (stream/datagram) nor the protocol is
provided.
Since matches on ports only make sense if the protocol is known
we "just" have to derive the protocol number from the information
already collected.

Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com>
---
 xtoptions.c |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/xtoptions.c b/xtoptions.c
index ac0601f..e38b7ef 100644
--- a/xtoptions.c
+++ b/xtoptions.c
@@ -556,12 +556,15 @@ static void xtopt_parse_host(struct xt_option_call *cb)
  * Resolve a port name to a number. Returns the port number in integral
  * form on success, or <0 on error. (errno will not be set.)
  */
-static int xtables_getportbyname(const char *name)
+static int xtables_getportbyname(const char *name, int family, int protocol)
 {
-	struct addrinfo *res = NULL, *p;
+	struct addrinfo hints, *res = NULL, *p;
 	int ret;
 
-	ret = getaddrinfo(NULL, name, NULL, &res);
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_family = family;
+	hints.ai_protocol = protocol;
+	ret = getaddrinfo(NULL, name, &hints, &res);
 	if (ret < 0)
 		return -1;
 	ret = -1;
@@ -598,9 +601,19 @@ static void xtopt_parse_protocol(struct xt_option_call *cb)
 static void xtopt_parse_port(struct xt_option_call *cb)
 {
 	const struct xt_option_entry *entry = cb->entry;
+	struct ipt_entry *fw = cb->xt_entry;
+	struct ip6t_entry *fw6 = cb->xt_entry;
+	int family, protocol;
 	int ret;
 
-	ret = xtables_getportbyname(cb->arg);
+	if (afinfo->family == NFPROTO_IPV4) {
+		family = AF_INET;
+		protocol = fw->ip.proto;
+	} else {
+		family = AF_INET6;
+		protocol = fw6->ipv6.proto;
+	}
+	ret = xtables_getportbyname(cb->arg, family, protocol);
 	if (ret < 0)
 		xt_params->exit_err(PARAMETER_PROBLEM,
 			"Port \"%s\" does not resolve to anything.\n",
@@ -616,10 +629,21 @@ static void xtopt_parse_mport(struct xt_option_call *cb)
 {
 	static const size_t esize = sizeof(uint16_t);
 	const struct xt_option_entry *entry = cb->entry;
+	struct ipt_entry *fw = cb->xt_entry;
+	struct ip6t_entry *fw6 = cb->xt_entry;
 	char *lo_arg, *wp_arg, *arg;
 	unsigned int maxiter;
+	int family, protocol;
 	int value;
 
+	if (afinfo->family == NFPROTO_IPV4) {
+		family = AF_INET;
+		protocol = fw->ip.proto;
+	} else {
+		family = AF_INET6;
+		protocol = fw6->ipv6.proto;
+	}
+
 	wp_arg = lo_arg = strdup(cb->arg);
 	if (lo_arg == NULL)
 		xt_params->exit_err(RESOURCE_PROBLEM, "strdup");
@@ -645,7 +669,7 @@ static void xtopt_parse_mport(struct xt_option_call *cb)
 			continue;
 		}
 
-		value = xtables_getportbyname(arg);
+		value = xtables_getportbyname(arg, family, protocol);
 		if (value < 0)
 			xt_params->exit_err(PARAMETER_PROBLEM,
 				"Port \"%s\" does not resolve to "
-- 
1.7.2.5

Re: [PATCH] Provide family and protocol to make getaddrinfo happy

[Jan Engelhardt]

On Wednesday 2011-05-25 14:16, Lutz Jaenicke wrote:

>getaddrinfo() will fail for numeric port numbers if neither
>the socket type (stream/datagram) nor the protocol is
>provided.
>Since matches on ports only make sense if the protocol is known
>we "just" have to derive the protocol number from the information
>already collected.

Not quite; ports are also possible with -m ipvs and -m conntrack, which 
do not require -p tcp/udp.

If someone speicifies a numeric port to getaddrinfo, I just want the 
thing encoded in struct sockaddr_XX.sXX_port - it does not need to know 
the particular L4 proto, or the service's text-based name to do that - 
which is most likely why glibc allows it.

I would like to have an opinion on that from µclibc preferably..
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Provide family and protocol to make getaddrinfo happy

[Lutz Jaenicke]

On Wed, May 25, 2011 at 02:31:05PM +0200, Jan Engelhardt wrote:
> On Wednesday 2011-05-25 14:16, Lutz Jaenicke wrote:
> 
> >getaddrinfo() will fail for numeric port numbers if neither
> >the socket type (stream/datagram) nor the protocol is
> >provided.
> >Since matches on ports only make sense if the protocol is known
> >we "just" have to derive the protocol number from the information
> >already collected.
> 
> Not quite; ports are also possible with -m ipvs and -m conntrack, which 
> do not require -p tcp/udp.
>
> 
> If someone speicifies a numeric port to getaddrinfo, I just want the 
> thing encoded in struct sockaddr_XX.sXX_port - it does not need to know 
> the particular L4 proto, or the service's text-based name to do that - 
> which is most likely why glibc allows it.

Manual pages may not be the ultimate reference about the reasoning
on why something is implemented which way. Nevertheless let's see:

       Given node and service, which identify an Internet host and a  service,
       getaddrinfo()  returns  one  or more addrinfo structures, each of which
       contains an Internet address that can be specified in a call to bind(2)
       or  connect(2).

Taking this information "as is" actually means that using getaddrinfo()
the way it is done is at least stretching the functionality...

As I wrote in my introduction for the patch (which has not yet made it to the
mailing list), other C libraries behave like uClibc:
* http://gitorious.org/0xdroid/bionic/blobs/9ab75d4cc803e91b7f1b656ffbe2ad32c52a86f9/libc/netbsd/net/getaddrinfo.c                                                
  -> look for "ANDROID-SPECIFIC CHANGE TO MATCH GLIBC" :-)
* http://www.freebsd.org/cgi/query-pr.cgi?pr=51827
* Especially I like
  http://www.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libc/net/getaddrinfo.c?rev=1.72;content-type=text%2Fplain
  which in the text says 
	/*
	 * check for special cases.  (1) numeric servname is disallowed if
	 * socktype/protocol are left unspecified. (2) servname is disallowed
	 * for raw and other inet{,6} sockets.
	 */
  (and I took this as a reference until now) but later in get_portmatch():
  	case SOCK_DGRAM:
	case SOCK_STREAM:
	case ANY:
		allownumeric = 1;

> I would like to have an opinion on that from µclibc preferably..

Seems uClibc is outdated wrt adjusting the behavior with other C libraries...

Best regards,
	Lutz
-- 
Dr.-Ing. Lutz Jänicke
CTO
Innominate Security Technologies AG  /protecting industrial networks/
tel: +49.30.921028-200
fax: +49.30.921028-020
Rudower Chaussee 13
D-12489 Berlin, Germany
www.innominate.com

Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Volker Bibelhausen
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Provide family and protocol to make getaddrinfo happy

[Jan Engelhardt]

On Wednesday 2011-05-25 15:26, Lutz Jaenicke wrote:
>On Wed, May 25, 2011 at 02:31:05PM +0200, Jan Engelhardt wrote:
>> 
>> >getaddrinfo() will fail for numeric port numbers if neither
>> >the socket type (stream/datagram) nor the protocol is
>> >provided. [in uclibc]
>
>As I wrote in my introduction for the patch (which has not yet made it to the
>mailing list), other C libraries behave like uClibc:
>* http://gitorious.org/0xdroid/bionic/blobs/9ab75d4cc803e91b7f1b656ffbe2ad32c52a86f9/libc/netbsd/net/getaddrinfo.c                                                
>  -> look for "ANDROID-SPECIFIC CHANGE TO MATCH GLIBC" :-)
>* http://www.freebsd.org/cgi/query-pr.cgi?pr=51827
>* Especially I like
>  http://www.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libc/net/getaddrinfo.c?rev=1.72;content-type=text%2Fplain
>  which in the text says 
>	/*
>	 * check for special cases.  (1) numeric servname is disallowed if
>	 * socktype/protocol are left unspecified. (2) servname is disallowed
>	 * for raw and other inet{,6} sockets.
>	 */
>  (and I took this as a reference until now) but later in get_portmatch():
>  	case SOCK_DGRAM:
>	case SOCK_STREAM:
>	case ANY:
>		allownumeric = 1;

So in other words, four libraries allow it, and one - µclibc - has a bug that
should be fixed.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Provide family and protocol to make getaddrinfo happy

[Lutz Jaenicke]

On Wed, May 25, 2011 at 03:41:17PM +0200, Jan Engelhardt wrote:
> 
> On Wednesday 2011-05-25 15:26, Lutz Jaenicke wrote:
> >On Wed, May 25, 2011 at 02:31:05PM +0200, Jan Engelhardt wrote:
> >> 
> >> >getaddrinfo() will fail for numeric port numbers if neither
> >> >the socket type (stream/datagram) nor the protocol is
> >> >provided. [in uclibc]
> >
> >As I wrote in my introduction for the patch (which has not yet made it to the
> >mailing list), other C libraries behave like uClibc:
> >* http://gitorious.org/0xdroid/bionic/blobs/9ab75d4cc803e91b7f1b656ffbe2ad32c52a86f9/libc/netbsd/net/getaddrinfo.c                                                
> >  -> look for "ANDROID-SPECIFIC CHANGE TO MATCH GLIBC" :-)
> >* http://www.freebsd.org/cgi/query-pr.cgi?pr=51827
> >* Especially I like
> >  http://www.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libc/net/getaddrinfo.c?rev=1.72;content-type=text%2Fplain
> >  which in the text says 
> >	/*
> >	 * check for special cases.  (1) numeric servname is disallowed if
> >	 * socktype/protocol are left unspecified. (2) servname is disallowed
> >	 * for raw and other inet{,6} sockets.
> >	 */
> >  (and I took this as a reference until now) but later in get_portmatch():
> >  	case SOCK_DGRAM:
> >	case SOCK_STREAM:
> >	case ANY:
> >		allownumeric = 1;
> 
> So in other words, four libraries allow it, and one - µclibc - has a bug that
> should be fixed.

I am not sure it is actually a bug. RFC 3493 does not specify this way
or the other. It just seems that most other C libraries changed the
behavior from "not accepted" to "accepted" in 2009 such that these days
the consensus seems to be this way.

Best regards,
	Lutz
-- 
Dr.-Ing. Lutz Jänicke
CTO
Innominate Security Technologies AG  /protecting industrial networks/
tel: +49.30.921028-200
fax: +49.30.921028-020
Rudower Chaussee 13
D-12489 Berlin, Germany
www.innominate.com

Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Volker Bibelhausen
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html