[Date Prev] [Date Index] [Date Next] [Thread Prev] [Thread Index] [Thread Next]

conserver's IPv6 support not fully baked...

Havard Eidnes he@uninett.no
Wed, 18 Jan 2017 10:20:56 GMT


Hi,

I recently had to deal with a conserver version upgrade, and
someone had decided to turn on the "IPv6" option in the package.

Attached below are two patches which do three things:

1) (unrelated to IPv6) print decoded errno on connection failure
2) use temp variables to walk address lists, avoid
   trying to do freeaddrinfo(NULL), as that leads to SEGV for the
   main server process
3) use correct socket address length when calling getnameinfo()
   in the IPv6 case, avoid early return with "permanent failure"
   because the socket address length is wrong

In my case I'm running this on NetBSD, and on that platform you
need to open two server sockets, one for IPv6 and one for IPv4 if
you are going to serve both.  The code as it stands on NetBSD
only opens an IPv6 socket if the IPv6 option is enabled, and
hence IPv4 is not served, causing backward compatibility problems
and failure to interoperate with conserver installations which
don't have IPv6 configured.

Plus ... it seems that it also can't connect to remote consoles
which are only reachable via IPv4 when the IPv6 option is
enabled, but admittedly I did not debug that more thoroughly.

It seems to be a bigger task to fix the Master() function in
conserver/master.c to do the "two server sockets" dance needed on
NetBSD.  Plus one needs to portably distinguish where two sockets
are needed and where they are not, so I've punted on that for
now.

Regards,

- Håvard
$NetBSD: patch-conserver_consent.c,v 1.1 2017/01/18 09:54:51 he Exp $

Print strerror() on failure.
Use scratch variables for walking address info list, so that
we don't end up trying to freeaddrinfo(NULL).

--- conserver/consent.c.orig	2015-06-02 17:17:45.000000000 +0000
+++ conserver/consent.c
@@ -919,13 +919,16 @@ ConsInit(CONSENT *pCE)
 				     rp->ai_addrlen)) == 0)
 			    goto success;
 		      fail:
+			error = errno;
 			close(cofile);
+		    } else {
+			error = errno;
 		    }
 		    rp = rp->ai_next;
 		}
 
-		Error("[%s]: Unable to connect to %s:%s", pCE->server,
-		      host, serv);
+		Error("[%s]: Unable to connect to %s:%s %s", pCE->server,
+		      host, serv, strerror(error));
 		ConsDown(pCE, FLAGTRUE, FLAGTRUE);
 		return;
 	      success:
@@ -1252,7 +1255,7 @@ AddrsMatch(char *addr1, char *addr2)
 {
 #if USE_IPV6
     int error, ret = 0;
-    struct addrinfo *ai1, *ai2, hints;
+    struct addrinfo *ai1, *aip1, *ai2, *aip2, hints;
 #else
     /* so, since we might use inet_addr, we're going to use
      * (in_addr_t)(-1) as a sign of an invalid ip address.
@@ -1290,17 +1293,17 @@ AddrsMatch(char *addr1, char *addr2)
 	goto done;
     }
 
-    for (; ai1 != NULL; ai1 = ai1->ai_next) {
-	for (; ai2 != NULL; ai2 = ai2->ai_next) {
-	    if (ai1->ai_addr->sa_family != ai2->ai_addr->sa_family)
+    for (aip1 = ai1; aip1 != NULL; aip1 = aip1->ai_next) {
+	for (aip2 = ai2; aip2 != NULL; aip2 = aip2->ai_next) {
+	    if (aip1->ai_addr->sa_family != aip2->ai_addr->sa_family)
 		continue;
 
 	    if (
 # if HAVE_MEMCMP
-		   memcmp(&ai1->ai_addr, &ai2->ai_addr,
+		   memcmp(&aip1->ai_addr, &aip2->ai_addr,
 			  sizeof(struct sockaddr_storage))
 # else
-		   bcmp(&ai1->ai_addr, &ai2->ai_addr,
+		   bcmp(&aip1->ai_addr, &aip2->ai_addr,
 			sizeof(struct sockaddr_storage))
 # endif
 		   == 0) {
@@ -1311,8 +1314,10 @@ AddrsMatch(char *addr1, char *addr2)
     }
 
   done:
-    freeaddrinfo(ai1);
-    freeaddrinfo(ai2);
+    if (ai1)
+	freeaddrinfo(ai1);
+    if (ai2)
+	freeaddrinfo(ai2);
     Msg("compare %s and %s returns %d", addr1, addr2, ret);
     return ret;
 #else
$NetBSD: patch-conserver_access.c,v 1.1 2017/01/18 09:54:51 he Exp $

Make sure to use correct sockaddr length when doing getnameinfo().

--- conserver/access.c.orig	2017-01-18 09:20:03.000000000 +0000
+++ conserver/access.c
@@ -150,6 +150,7 @@ AccType(INADDR_STYPE *addr, char **peern
     so = sizeof(*addr);
 
 #if USE_IPV6
+    so = sizeof(struct sockaddr_in6);
     error =
 	getnameinfo((struct sockaddr *)addr, so, ipaddr, sizeof(ipaddr),
 		    NULL, 0, NI_NUMERICHOST);