Whee! Thanks!

I'd had this on my TODO list for, well, quite a while, but never had
gotten around to actually work on this.

On Tue, Mar 16, 2010 at 06:36:22PM +0700, Neutron Soutmun wrote:
> Package: nbd
> Version: 2.9.14-3
> Severity: normal
> Tags: patch

I should probably point out that there's a git tree for the upstream
code: it's on github ("/yoe/nbd"), and on sourceforge
(nbd.git.sourceforge.net).

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> I have provide the initial patch for IPv6 support in
> nbd-{server, client} which the nbd-server able to
> serve in both IPv4 and IPv6 requests.
> 
> Patch is attached.
> 
> The client can connect to the server that listen on
> IPv6 or IPv4 like this on command line
> 
> # nbd-client ::1...@11111 /dev/nbd0
> or
> # nbd-client 127.0....@11111 /dev/nbd0
> 
> Substitute the port separator from ":" to "@".

Indeed, that would be required for proper IPv6 support.

However, I'd like to have some way of backwards compatibility. Since
IPv6 addresses have at least two colons in them, we could check whether
there are two or more, and if not, assume that the colon is still the
separator.

That shouldn't cause too much of a problem.

> Please test and also advice me if you have seen
> something wrong in the code.
> 
> Regards,
> Neutron Soutmun
> 
> - -- System Information:
> Debian Release: squeeze/sid
>   APT prefers unstable
>   APT policy: (500, 'unstable')
> Architecture: amd64 (x86_64)
> 
> Kernel: Linux 2.6.32-2-amd64 (SMP w/2 CPU cores)
> Locale: LANG=th_TH.UTF-8, LC_CTYPE=th_TH.UTF-8 (charmap=UTF-8)
> Shell: /bin/sh linked to /bin/bash
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> 
> iEYEARECAAYFAkufbTEACgkQ1k7Ar9TO/TfBjwCfUfGLSRwg3Mlo0TvcubZQzQbY
> 7lYAniCEFhJAIdChCnli0NC4as5VNzTW
> =FF47
> -----END PGP SIGNATURE-----

> diff -u nbd-2.9.14/nbd-server.c nbd-2.9.14/nbd-server.c
> --- nbd-2.9.14/nbd-server.c
> +++ nbd-2.9.14/nbd-server.c
> @@ -49,6 +49,10 @@
>   * 11/02/2004 - Doxygenified the source, modularized it a bit. Needs a 
>   *   lot more work, but this is a start. Wouter Verhelst
>   *   <[email protected]>
> + * 16/03/2010 - Add IPv6 support.
> + *   Kitt Tientanopajai <[email protected]>
> + *   Neutron Soutmun <[email protected]>
> + *   Suriya Soutmun <[email protected]>
>   */
>  
>  /* Includes LFS defines, which defines behaviours of some of the following
> @@ -70,8 +74,8 @@
>  #include <signal.h>          /* sigaction */
>  #include <errno.h>
>  #include <netinet/tcp.h>
> -#include <netinet/in.h>              /* sockaddr_in, htons, in_addr */
> -#include <netdb.h>           /* hostent, gethostby*, getservby* */
> +#include <netinet/in.h>
> +#include <netdb.h>
>  #include <syslog.h>
>  #include <unistd.h>
>  #include <stdio.h>
> @@ -178,6 +182,7 @@
>       char* authname;      /**< filename of the authorization file */
>       int flags;           /**< flags associated with this exported file */
>       int socket;          /**< The socket of this server. */
> +     int socket_family;   /**< family of the socket */
>       VIRT_STYLE virtstyle;/**< The style of virtualization, if any */
>       uint8_t cidrlen;     /**< The length of the mask when we use
>                                 CIDR-style virtualization */
> @@ -338,7 +343,7 @@
>   */
>  void usage() {
>       printf("This is nbd-server version " VERSION "\n");
> -     printf("Usage: [ip:]port file_to_export [size][kKmM] [-l 
> authorize_file] [-r] [-m] [-c] [-C configuration file] [-p PID file name] [-o 
> section name]\n"
> +     printf("Usage: [...@]port file_to_export [size][kKmM] [-l 
> authorize_file] [-r] [-m] [-c] [-C configuration file] [-p PID file name] [-o 
> section name]\n"
>              "\t-r|--read-only\t\tread only\n"
>              "\t-m|--multi-file\t\tmultiple file\n"
>              "\t-c|--copy-on-write\tcopy on write\n"
> @@ -417,7 +422,7 @@
>                       /* non-option argument */
>                       switch(nonspecial++) {
>                       case 0:
> -                             addr_port=g_strsplit(optarg, ":", 2);
> +                             addr_port=g_strsplit(optarg, "@", 2);
>                               if(addr_port[1]) {
>                                       serve->port=strtol(addr_port[1], NULL, 
> 0);
>                                       
> serve->listenaddr=g_strdup(addr_port[0]);
> @@ -526,6 +531,46 @@
>  }
>  
>  /**
> + * duplicate server 
> + * @param s the old server we want to duplicate
> + **/
> +SERVER* dup_serve(SERVER *s) {
> +     SERVER *serve = NULL;
> +
> +     serve=g_new0(SERVER, 1);
> +     if (serve == NULL)
> +             return NULL;
> +
> +     memset(serve,0,sizeof(SERVER));

g_new0 already does this (that's the 0 in the function name)

> +     if (s->exportname)
> +             serve->exportname = g_strdup(s->exportname);
> +
> +     serve->expected_size = s->expected_size;
> +
> +     if (s->listenaddr)
> +             serve->listenaddr = g_strdup(s->listenaddr);
> +
> +     serve->port = s->port;  
> +
> +     if (s->authname)
> +             serve->authname = strdup(s->authname);
> +
> +     serve->flags = s->flags;
> +     serve->socket = serve->socket;
> +     serve->socket_family = serve->socket_family;
> +     serve->cidrlen = s->cidrlen;
> +
> +     if (s->prerun)
> +             serve->prerun = g_strdup(s->prerun);
> +
> +     if (s->postrun)
> +             serve->postrun = g_strdup(s->postrun);
> +
> +     return serve;
> +}
> +
> +/**
>   * Parse the config file.
>   *
>   * @param f the name of the config file
> @@ -539,6 +584,7 @@
>       const char* DEFAULT_ERROR = "Could not parse %s in group %s: %s";
>       const char* MISSING_REQUIRED_ERROR = "Could not find required value %s 
> in group %s: %s";
>       SERVER s;
> +     SERVER *ns = NULL;
>       gchar *virtstyle=NULL;
>       PARAM lp[] = {
>               { "exportname", TRUE,   PARAM_STRING,   NULL, 0 },
> @@ -690,7 +736,17 @@
>               if(i>0) {
>                       if(!s.listenaddr) {
>                               s.listenaddr = g_strdup("0.0.0.0");
> +                             s.socket_family = AF_UNSPEC;
> +
> +                       ns = dup_serve (&s);  
> +                             if (ns) {
> +                                     ns->socket_family = AF_INET6;
> +                                     g_array_append_val(retval, *ns);
> +                                     free(ns);
> +                                     ns = NULL;
> +                             }       
>                       }

What is the reason for that? Doesn't AF_UNSPEC already mean we're
listening for v6 addresses, too? Or am I missing something?

> +
>                       g_array_append_val(retval, s);
>               }
>  #ifndef WITH_SDP
> @@ -1346,15 +1402,34 @@
>  void set_peername(int net, CLIENT *client) {
>       struct sockaddr_storage addrin;
>       struct sockaddr_storage netaddr;
> +     struct sockaddr_in  *netaddr4 = NULL;
> +     struct sockaddr_in6 *netaddr6 = NULL;
>       size_t addrinlen = sizeof( addrin );
> -     char *peername;
> -     char *netname;
> -     char *tmp;
> +     struct addrinfo hints;
> +     struct addrinfo *ai = NULL;
> +     char peername[NI_MAXHOST];
> +     char netname[NI_MAXHOST];
> +     char *tmp = NULL;
>       int i;
> +     int e;
> +     int shift;
>  
> -     if (getpeername(net, (struct sockaddr *) &addrin, (socklen_t 
> *)&addrinlen) < 0)
> +     if (getpeername(net, (struct sockaddr *)&addrin, (socklen_t 
> *)&addrinlen) < 0)

Please don't do gratuitous whitespace changes; they convolute the patch
and make it harder to see what's going on.

>               err("getsockname failed: %m");
> -     peername = g_strdup(inet_ntoa(addrin.sin_addr));
> +
> +     getnameinfo((struct sockaddr *)&addrin, (socklen_t)addrinlen,
> +             peername, sizeof (peername), NULL, 0, NI_NUMERICHOST);
> +
> +     memset(&hints, '\0', sizeof (hints));
> +     hints.ai_flags = AI_ADDRCONFIG;
> +     e = getaddrinfo(peername, NULL, &hints, &ai);
> +
> +     if(e != 0) {
> +             fprintf(stderr, "getaddrinfo failed: %s", gai_strerror(e));
> +             freeaddrinfo(ai);
> +             return;
> +     }
> +
>       switch(client->server->virtstyle) {
>               case VIRT_NONE:
>                       client->exportname=g_strdup(client->server->exportname);
> @@ -1370,18 +1445,42 @@
>                       break;
>               case VIRT_CIDR:
>                       memcpy(&netaddr, &addrin, addrinlen);
> -                     netaddr.sin_addr.s_addr>>=32-(client->server->cidrlen);
> -                     netaddr.sin_addr.s_addr<<=32-(client->server->cidrlen);
> -                     netname = inet_ntoa(netaddr.sin_addr);
> -                     tmp=g_strdup_printf("%s/%s", netname, peername);
> -                     
> client->exportname=g_strdup_printf(client->server->exportname, tmp);
> +                     if(ai->ai_family == AF_INET) {
> +                             netaddr4 = (struct sockaddr_in *)&netaddr;
> +                             
> (netaddr4->sin_addr).s_addr>>=32-(client->server->cidrlen);
> +                             
> (netaddr4->sin_addr).s_addr<<=32-(client->server->cidrlen);
> +
> +                             getnameinfo((struct sockaddr *) netaddr4, 
> (socklen_t) addrinlen,
> +                                                     netname, sizeof 
> (netname), NULL, 0, NI_NUMERICHOST);
> +                             tmp=g_strdup_printf("%s/%s", netname, peername);
> +                     }else if(ai->ai_family == AF_INET6) {
> +                             netaddr6 = (struct sockaddr_in6 *)&netaddr;
> +
> +                             shift = 128-(client->server->cidrlen);
> +                             i = 3;
> +                             while(shift >= 32) {
> +                                     ((netaddr6->sin6_addr).s6_addr32[i])=0;
> +                                     shift-=32;
> +                                     i--;
> +                             }
> +                             (netaddr6->sin6_addr).s6_addr32[i]>>=shift;
> +                             (netaddr6->sin6_addr).s6_addr32[i]<<=shift;
> +
> +                             getnameinfo((struct sockaddr *)netaddr6, 
> (socklen_t)addrinlen,
> +                                         netname, sizeof(netname), NULL, 0, 
> NI_NUMERICHOST);
> +                             tmp=g_strdup_printf("%s/%s", netname, peername);
> +                     }
> +
> +                     if(tmp != NULL)
> +                       
> client->exportname=g_strdup_printf(client->server->exportname, tmp);
> +
>                       break;
>       }
>  
> +     freeaddrinfo(ai);
>       msg4(LOG_INFO, "connect from %s, assigned file is %s", 
>            peername, client->exportname);
>       client->clientname=g_strdup(peername);
> -     g_free(peername);
>  }
>  
>  /**
> @@ -1493,23 +1592,53 @@
>   **/
>  void setup_serve(SERVER *serve) {
>       struct sockaddr_storage addrin;
> +     struct addrinfo hints;
> +     struct addrinfo *ai = NULL;
>       struct sigaction sa;
>       int addrinlen = sizeof(addrin);
>       int sock_flags;
> -     int af;
>  #ifndef sun
>       int yes=1;
>  #else
>       char yes='1';
>  #endif /* sun */
> +     gchar *port = NULL;
> +     int e;
> +
> +     memset(&hints,'\0',sizeof(hints));
> +     hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG | AI_NUMERICSERV;
> +     hints.ai_socktype = SOCK_STREAM;
> +
> +     port = g_strdup_printf ("%d", serve->port);
> +     if (port == NULL)
> +             return;
> +
> +     if(g_strcmp0(serve->listenaddr, "0.0.0.0") == 0) {

This would mean that if someone were to use 'nbd-server 0.0.0.0:1234' to
explicitly bind to an IPv4 address, this would not work.

I think the disabling of IPv6 for those who want it, should still be
possible.

> +             hints.ai_family = serve->socket_family;
> +             e = getaddrinfo(NULL,port,&hints,&ai); 
> +     } else {
> +             e = getaddrinfo(serve->listenaddr,port,&hints,&ai); 
> +     }
> +
> +     g_free(port);
> +
> +     if(e!=0) {
> +             freeaddrinfo(ai);
> +             return;
> +     }

This should probably use the err() function (which logs an error and
exits) instead.

> +
> +     if(serve->socket_family == AF_UNSPEC)
> +             serve->socket_family = ai->ai_family;
>  
> -     af = AF_INET;
>  #ifdef WITH_SDP
> -     if ((serve->flags) && F_SDP) {
> -             af = AF_INET_SDP;
> +     if((serve->flags) && F_SDP) {
> +             if (ai->ai_family == AF_INET)
> +                     ai->ai_family = AF_INET_SDP;
> +             else (ai->ai_family == AF_INET6)
> +                     ai->ai_family = AF_INET6_SDP;
>       }
>  #endif
> -     if ((serve->socket = socket(af, SOCK_STREAM, IPPROTO_TCP)) < 0)
> +     if ((serve->socket = socket(ai->ai_family, ai->ai_socktype, 
> ai->ai_protocol)) < 0)
>               err("socket: %m");
>  
>       /* lose the pesky "Address already in use" error message */
> @@ -1529,20 +1658,15 @@
>       }
>  
>       DEBUG("Waiting for connections... bind, ");
> -     addrin.sin_family = AF_INET;
> -#ifdef WITH_SDP
> -     if(serve->flags & F_SDP) {
> -             addrin.sin_family = AF_INET_SDP;
> -     }
> -#endif
> -     addrin.sin_port = htons(serve->port);
> -     if(!inet_aton(serve->listenaddr, &(addrin.sin_addr)))
> -             err("could not parse listen address");
> -     if (bind(serve->socket, (struct sockaddr *) &addrin, addrinlen) < 0)
> +     e = bind(serve->socket, ai->ai_addr, ai->ai_addrlen);
> +     if (e != 0 && errno != EADDRINUSE)
>               err("bind: %m");
>       DEBUG("listen, ");
>       if (listen(serve->socket, 1) < 0)
>               err("listen: %m");
> +
> +     freeaddrinfo (ai);
> +
>       sa.sa_handler = sigchld_handler;
>       sigemptyset(&sa.sa_mask);
>       sa.sa_flags = SA_RESTART;
[... packaging bits snipped ...]
> --- nbd-2.9.14.orig/nbd-client.c
> +++ nbd-2.9.14/nbd-client.c
> @@ -11,6 +11,10 @@
>   * Version 2.1 - Check for disconnection before INIT_PASSWD is received
>   *   to make errormsg a bit more helpful in case the server can't
>   *   open the exported file.
> + * 16/03/2010 - Add IPv6 support.
> + *   Kitt Tientanopajai <[email protected]>
> + *   Neutron Soutmun <[email protected]>
> + *   Suriya Soutmun <[email protected]>
>   */
>  
>  #include "config.h"
> @@ -21,8 +25,8 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <netinet/tcp.h>
> -#include <netinet/in.h>              /* sockaddr_in, htons, in_addr */
> -#include <netdb.h>           /* hostent, gethostby*, getservby* */
> +#include <netinet/in.h>
> +#include <netdb.h>
>  #include <stdio.h>
>  #include <fcntl.h>
>  #include <syslog.h>
> @@ -67,33 +71,54 @@
>  
>  int opennet(char *name, int port, int sdp) {
>       int sock;
> -     struct sockaddr_in xaddrin;
> -     int xaddrinlen = sizeof(xaddrin);
> -     struct hostent *hostn;
> -     int af;
> -
> -     hostn = gethostbyname(name);
> -     if (!hostn)
> -             err("Gethostname failed: %h\n");
> +     char portstr[6];
> +     struct addrinfo hints;
> +     struct addrinfo *ai = NULL;
> +     struct addrinfo *rp = NULL;
> +     int e;
> +
> +     snprintf(portstr, sizeof(portstr), "%d", port);
> +
> +     memset(&hints,'\0',sizeof(hints));
> +     hints.ai_family = AF_UNSPEC;
> +     hints.ai_socktype = SOCK_STREAM;
> +     hints.ai_flags = AI_ADDRCONFIG | AI_NUMERICSERV;
> +     hints.ai_protocol = IPPROTO_TCP;
> +
> +     e = getaddrinfo(name, portstr, &hints, &ai);
> +
> +     if(e != 0) {
> +             freeaddrinfo(ai);
> +             return -1;
> +     }

Again, use err() here.

> -     af = AF_INET;
>       if(sdp) {
>  #ifdef WITH_SDP
> -             af = AF_INET_SDP;
> +     if (ai->ai_family == AF_INET)
> +             ai->ai_family = AF_INET_SDP;
> +     else (ai->ai_family == AF_INET6)
> +             ai->ai_family = AF_INET6_SDP;
>  #else
>               err("Can't do SDP: I was not compiled with SDP support!");
>  #endif
>       }
> -     if ((sock = socket(af, SOCK_STREAM, IPPROTO_TCP)) < 0)
> -             err("Socket failed: %m");
>  
> -     xaddrin.sin_family = af;
> -     xaddrin.sin_port = htons(port);
> -     xaddrin.sin_addr.s_addr = *((int *) hostn->h_addr);
> -     if ((connect(sock, (struct sockaddr *) &xaddrin, xaddrinlen) < 0))
> -             err("Connect: %m");
> +     for(rp = ai; rp != NULL; rp = rp->ai_next) {
> +             sock = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
> +
> +             if(sock == -1)
> +                     continue;       /* error */
> +
> +             if(connect(sock, rp->ai_addr, rp->ai_addrlen) != -1)
> +                     break;          /* success */
> +     }
> +
> +     if (rp == NULL)
> +             err("Socket failed: %m");
>  
>       setmysockopt(sock);
> +
> +     freeaddrinfo(ai);
>       return sock;
>  }

The rest of the patch looks fine, on first glance.

If you could update it with those comments (I'm not sure I understand
all the intricacies yet), I'll push the patch upstream and do an upload
ASAP.

Thanks,

-- 
The biometric identification system at the gates of the CIA headquarters
works because there's a guard with a large gun making sure no one is
trying to fool the system.
  http://www.schneier.com/blog/archives/2009/01/biometrics.html



-- 
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]

Reply via email to