nnrpd -D and IPv6 (was Re: Joint is better than Panzerfaust)

[Jeffrey M. Vinocur]

The below patch was hiding in news.software.nntp (I almost missed
it, as there was a bunch of non-INN-related stuff at the top of
the article).  I ran this by Nathan (who did some of the IPv6
stuff in CURRENT), and he says if it does actually make something
work that was broken, might as well.

But I don't use nnrpd -D, and so have no way to test this, and
don't know the IPv6 code well enough to do it by inspection.  So
I figured I'd at least make sure it got full light of day and
hope somebody else would run with it.


In article <000OOOOOO0 at news.radio.cz> Barry wrote:
>
>[...]
>
>Methinks this discussion is best at home in alt.binaries.news-swerver-
>consumption, but I hesitate to attempt a followup there in case the
>mail2news gateway rejects this for policy reasons against potential
>abuse.  So, to try and be marginally on-topic in the other froup too,
>here's a quick and ugly (and not comprehensive) patch I've come up with --
>it can fail misleadingly for invalid IPv6 addresses, for example, and
>was intended purely to get daemon mode to work without taking the
>time to do it right, since I have no clue whatsoever what I'm doing:
>
>
>--- nnrpd.c-DIST	Fri Aug  2 17:37:52 2002
>+++ nnrpd.c	Wed Sep 11 08:43:48 2002
>@@ -842,11 +842,13 @@
>     socklen_t		clen;
> #ifdef HAVE_INET6
>     struct sockaddr_storage ssa, csa;
>-    struct sockaddr_in6	*ssa6 = (struct sockaddr_in6 *) &ssa;
>+    /* `ssa' is no longer used itself, but I'm too lazy to remove it  */
>+    struct sockaddr_in6	ssa6;
> #else
>     struct sockaddr_in	ssa, csa;
> #endif
>-    struct sockaddr_in	*ssa4 = (struct sockaddr_in *) &ssa;
>+    /* ditto about ssa */
>+    struct sockaddr_in	ssa4;
>     struct stat		Sb;
>     pid_t		pid = -1;
>     gid_t               NewsGID;
>@@ -986,23 +988,32 @@
>     if (DaemonMode) {
> 
> #ifdef HAVE_INET6
>-	memset(&ssa, '\0', sizeof(struct sockaddr_in6));
>-	ssa6->sin6_family = AF_INET6;
>-	ssa6->sin6_port   = htons(ListenPort);
>-	if (inet_pton(AF_INET6, ListenAddr, ssa6->sin6_addr.s6_addr) > 0) {
>-	    if ( (lfd = socket(AF_INET6, SOCK_STREAM, 0)) < 0) {
>+	/* memset ssa6 separately instead */
>+	memset(&ssa6, 0, sizeof ssa6);
>+	ssa6.sin6_family = AF_INET6;
>+	ssa6.sin6_port   = htons(ListenPort);
>+#ifdef HAVE_SOCKADDR_LEN
>+            ssa6.sin6_len = sizeof ssa6;
>+#endif
>+	if (inet_pton(AF_INET6, ListenAddr, &ssa6.sin6_addr) > 0) {
>+	    /* gratuitous changes to match inndstart */
>+	    if ( (lfd = socket(PF_INET6, SOCK_STREAM, 0)) < 0) {
> 		syslog(L_FATAL, "can't open socket (%m)");
> 		exit(1);
> 	    }
> 	}
> 	else {
> #endif
>-	    memset(&ssa, '\0', sizeof(struct sockaddr_in));
>-	    ssa4->sin_family = AF_INET;
>-	    ssa4->sin_port   = htons(ListenPort);
>-	    if (inet_aton(ListenAddr, &ssa4->sin_addr) <= 0 )
>-		ssa4->sin_addr.s_addr = htonl(INADDR_ANY);
>-	    if ( (lfd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
>+	    /* see comment above, here we whack ssa4 */
>+	    memset(&ssa4, 0, sizeof(struct sockaddr_in));
>+	    ssa4.sin_family = AF_INET;
>+	    ssa4.sin_port   = htons(ListenPort);
>+#ifdef HAVE_SOCKADDR_LEN
>+            ssa4.sin_len = sizeof ssa4;
>+#endif
>+	    if (inet_aton(ListenAddr, &ssa4.sin_addr) <= 0 )
>+		ssa4.sin_addr.s_addr = htonl(INADDR_ANY);
>+	    if ( (lfd = socket(PF_INET, SOCK_STREAM, 0)) < 0) {
> 		syslog(L_FATAL, "can't open socket (%m)");
> 		exit(1);
> 	    }
>@@ -1016,11 +1027,21 @@
> 	    exit(1);
> 	}
> 
>-	if (bind(lfd, (struct sockaddr *) &ssa, sizeof(ssa)) < 0) {
>+	/* Ugly.  Try to bind ssa6 whether or not ListenAddr really is IPv6 */
>+#ifdef HAVE_INET6
>+	if (bind(lfd, (struct sockaddr *) &ssa6, sizeof(ssa6)) < 0) {
>+	/* If this fails for *any* reason, not just if it's IPv4, then
>+           try that, possibly concealing the real failure...  */
>+#endif
>+	  if (bind(lfd, (struct sockaddr *) &ssa4, sizeof(ssa4)) < 0) {
> 	    fprintf(stderr, "%s: can't bind (%s)\n", argv[0], strerror(errno));
> 	    syslog(L_FATAL, "can't bind local address (%m)");
> 	    exit(1);
>+	  }
>+#ifdef HAVE_INET6
>+	/* But hey, if HAVE_INET6 was defined, things had been t0taL B0rkeD */
> 	}
>+#endif
> 
> 	/* If started as root, switch to news uid */
> 	if (getuid() == 0) {
>
>
>I mean, it just didn't work at all before, but now it does, though
>not perfectly.  What more could I ask for?  Okay, yes, that too.

-- 
Jeffrey M. Vinocur
jeff at litech.org