Hi,
First of all, thanks for writing and maintaining this great piece of software :)
When I ported 4.06 to FreeBSD on Monday, there were three changes that needed to be made. One is the passing of -1, not -1000, to poll(2), which has already been discussed on this list (and which, by the way, is mandated by POSIX and the Single Unix Specification, as you can see at http://www.opengroup.org/onlinepubs/007908799/xsh/poll.html ) The other two can be seen in the patch below.
The first one is related to the fact that EAI_NODATA is not present on all systems - it was actually obsoleted by the KAME implementation of IPv6 about an year ago. On some systems it is defined and has its own value - then stunnel builds just fine. On other systems it is not defined at all, while on yet other systems it is aliased to EAI_NONAME, so 'case EAI_NONAME' and 'case EAI_NODATA' would produce an error message from the compiler. Thus, the #ifdef and #if in the first chunk of the patch. This could not be done portably with a single statement, something like '#if defined(EAI_NODATA) && EAI_NODATA != EAI_NONAME', since there are some compilers that would barf on the second part if EAI_NODATA was not actually defined.
The second chunk of the patch fixes getnameinfo() error reporting - just like with getaddrinfo(), getnameinfo() errors should be displayed using s_gai_strerror() instead of sockerror().
Keep up the good work!
G'luck, Peter
--- src/network.c.orig Thu Oct 14 18:03:49 2004 +++ src/network.c Wed Dec 29 14:16:06 2004 @@ -416,8 +416,12 @@ return "Temporary failure in name resolution (EAI_AGAIN)"; case EAI_FAIL: return "Non-recoverable failure in name resolution (EAI_FAIL)"; +#ifdef EAI_NODATA +#if EAI_NODATA != EAI_NONAME case EAI_NODATA: return "No address associated with nodename (EAI_NODATA)"; +#endif +#endif case EAI_FAMILY: return "ai_family not supported (EAI_FAMILY)"; case EAI_SOCKTYPE: @@ -562,10 +566,13 @@ /* getnameinfo() version */ char *s_ntop(char *text, SOCKADDR_UNION *addr) { char host[20], port[6]; + int err;
- if(getnameinfo(&addr->sa, addr_len(*addr), - host, 20, port, 6, NI_NUMERICHOST|NI_NUMERICSERV)) { - sockerror("getnameinfo"); + err = getnameinfo(&addr->sa, addr_len(*addr), + host, 20, port, 6, NI_NUMERICHOST|NI_NUMERICSERV); + if (err) { + s_log(LOG_ERR, "Error resolving the specified address: %s", + s_gai_strerror(err)); strcpy(text, "unresolvable IP"); return text; }
Peter Pentchev wrote:
The second chunk of the patch fixes getnameinfo() error reporting - just like with getaddrinfo(), getnameinfo() errors should be displayed using s_gai_strerror() instead of sockerror().
Manuals seem to claim just the opposite.
Linux Manual: RETURN VALUE On success 0 is returned, and node and service names, if requested, are filled with NUL-terminated strings, possibly truncated to fit the specified buffer lengths. On error a nonzero value is returned, and errno is set appropriately.
Microsoft: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winsock/win... Return Values Success returns zero. Any nonzero return value indicates failure. Use the WSAGetLastError function to retrieve error information.
Best regards, Mike
On Thu, Dec 30, 2004 at 10:47:28AM +0100, Michal Trojnara wrote:
Peter Pentchev wrote:
The second chunk of the patch fixes getnameinfo() error reporting - just like with getaddrinfo(), getnameinfo() errors should be displayed using s_gai_strerror() instead of sockerror().
Manuals seem to claim just the opposite.
Oops. I should have guessed that much when I saw that you had chosen that way; instead of checking why, I just wondered briefly and let it go.
Linux Manual: RETURN VALUE On success 0 is returned, and node and service names, if requested, are filled with NUL-terminated strings, possibly truncated to fit the specified buffer lengths. On error a nonzero value is returned, and errno is set appropriately.
That's funny. Which IPv6 implementation is that - USAGI?
Microsoft: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winsock/win... Return Values Success returns zero. Any nonzero return value indicates failure. Use the WSAGetLastError function to retrieve error information.
That's not so funny, but still interesting.
The problem is, FreeBSD uses the KAME implementation, and the getnameinfo(3) manpage claims POSIX 1003.1g conformance, as well as RFC 2553 conformance. You can see the FreeBSD manual page at http://www.FreeBSD.org/cgi/man.cgi?query=getnameinfo&sektion=3 and RFC 2553 at http://www.faqs.org/rfc/rfc2553.txt - both say that getnameinfo() should return the error directly, and the error should be one of the EAI_* constants, just like getaddrinfo().
What do you think about the following version of the patch, which adds a configure check for the KAME version of getnameinfo()?
G'luck, Peter
--- configure.ac.orig Sun Dec 26 01:30:48 2004 +++ configure.ac Thu Dec 30 13:18:26 2004 @@ -176,6 +176,30 @@ AC_MSG_NOTICE([**************************************** library functions]) AC_CHECK_FUNCS(snprintf vsnprintf openpty _getpty daemon waitpid wait4 sysconf getrlimit pthread_sigmask setgroups localtime_r chroot endhostent setsid getaddrinfo getnameinfo poll)
+AC_MSG_CHECKING([for KAME getnameinfo]) +AC_RUN_IFELSE( + [AC_LANG_PROGRAM( + [ + [#include <sys/types.h>] + [#include <sys/socket.h>] + [#include <stdio.h>] + [#include <stdlib.h>] + [#include <errno.h>] + [#include <netdb.h>] + ], + [ + [int err;] + [err = getnameinfo(NULL, -1, NULL, -1, NULL, -1, -1);] + [return (err != EAI_FAIL);] + ]) + ], + [ + AC_MSG_RESULT([yes]); + AC_DEFINE(HAVE_KAME_GETNAMEINFO) + ], + [AC_MSG_RESULT([no])] +) + AC_MSG_NOTICE([**************************************** optional features]) # Use RSA? AC_MSG_CHECKING([whether to disable RSA support]) --- src/network.c.orig Thu Oct 14 18:03:49 2004 +++ src/network.c Thu Dec 30 13:26:23 2004 @@ -563,9 +563,18 @@ char *s_ntop(char *text, SOCKADDR_UNION *addr) { char host[20], port[6];
+#ifdef HAVE_KAME_GETNAMEINFO + int err; + err = getnameinfo(&addr->sa, addr_len(*addr), + host, 20, port, 6, NI_NUMERICHOST|NI_NUMERICSERV); + if (err) { + s_log(LOG_ERR, "Error resolving the specified address: %s", + s_gai_strerror(err)); +#else if(getnameinfo(&addr->sa, addr_len(*addr), host, 20, port, 6, NI_NUMERICHOST|NI_NUMERICSERV)) { sockerror("getnameinfo"); +#endif strcpy(text, "unresolvable IP"); return text; }
Peter Pentchev wrote:
That's funny. Which IPv6 implementation is that - USAGI?
getnameinfo is defined in glibc library: "The getaddrinfo and getnameinfo functions and supporting code were written by Craig Metz; see the file LICENSES for details on their licensing."
What do you think about the following version of the patch, which adds a configure check for the KAME version of getnameinfo()?
I don't think getnameinfo() error is very probable with NI_NUMERICHOST|NI_NUMERICSERV. 8-) I guess we don't really need to report an error text other than "getnameinfo failed".
Best regards, Mike
On Thu, Dec 30, 2004 at 02:03:38PM +0100, Michal Trojnara wrote:
Peter Pentchev wrote:
That's funny. Which IPv6 implementation is that - USAGI?
getnameinfo is defined in glibc library: "The getaddrinfo and getnameinfo functions and supporting code were written by Craig Metz; see the file LICENSES for details on their licensing."
Ahh. Now all we need is a reason why there is no manpage mentioning getnameinfo() and getaddrinfo() in Debian testing :) That's why I thought it was part of some add-on kit, not plain vanilla glibc.
What do you think about the following version of the patch, which adds a configure check for the KAME version of getnameinfo()?
I don't think getnameinfo() error is very probable with NI_NUMERICHOST|NI_NUMERICSERV. 8-)
Actually, it is - for some reason I get an EAI_MEMORY error with the FreeBSD getnameinfo() implementation, when I define a service like this:
[ppp-stray] accept = f00f:4004:f00f:4004::f00f:1813 connect = 192.168.0.17:1813
I will look into this soon, but in the meantime, at least the FreeBSD port of stunnel will retain the s_gai_strerror() handling of getnameinfo() errors.
[about five minutes later]
*Oof*. Come to think of it, the reason is kinda obvious: s_ntop() only passes a 20-character host[] buffer to getnameinfo(), and a numeric IPv6 address may certainly grow a bit larger than that :) Shouldn't IPLEN in common.h be bumped up to at least 50? Maybe even a bit more - IPv6 addresses may be local to an interface - 'fe80::111:222%tun0' - so the total length should be 39 for the numeric address + 1 for the '%' + length of the interface name (which seems to be 15 for most BSD's and Linuxen) + 1 for the separator + 5 for the port name + 1 for the terminating null character = about 62. How about it?
I guess we don't really need to report an error text other than "getnameinfo failed".
Maybe, if we agree on increasing IPLEN :)
I'm attaching two patches:
stunnel-simple-iplen.patch - bumps IPLEN to 62 and makes s_ntop() use the correct size limit for host[] and the getnameinfo() invocation;
stunnel-kame-iplen.patch - same, but with the added autoconf detection of KAME getnameinfo() and use of s_gai_strerror() if necessary (and it might be necessary, at least on most BSD's, which have the KAME stack)
Of course, the EAI_NODATA chunk of the original patch is still necessary.
And now, hopefully I won't bother you anymore until the holidays are over :)
G'luck, Peter
Peter Pentchev wrote:
Ahh. Now all we need is a reason why there is no manpage mentioning getnameinfo() and getaddrinfo() in Debian testing :)
mtrojnar@mirt:~$ dpkg -S getnameinfo manpages-dev: /usr/share/man/man3/getnameinfo.3.gz mtrojnar@mirt:~$ dpkg -S getaddrinfo manpages-dev: /usr/share/man/man3/getaddrinfo.3.gz manpages-pl-dev: /usr/share/man/pl/man3/getaddrinfo.3.gz
I guess we don't really need to report an error text other than "getnameinfo failed".
Maybe, if we agree on increasing IPLEN :)
Agreed.
Best regards, Mike
-
Michal Trojnara -
Peter Pentchev