Libguestfs - Apr 2019 - [nbdkit PATCH 0/2] Let nbd plugin connect to TCP socket

Accepting only Unix sockets can be a bit limiting; let's be more flexible.

Eric Blake (2):
  nbd: Refactor Unix socket connection
  nbd: Support TCP socket

 plugins/nbd/nbdkit-nbd-plugin.pod |  36 ++++--
 plugins/nbd/nbd.c                 | 175 ++++++++++++++++++++++--------
 TODO                              |   3 -
 3 files changed, 161 insertions(+), 53 deletions(-)

-- 
2.20.1

Prepare to add TCP socket support by extracting Unix socket connection
into its own function. While at it, take advantage of nbdkit's
guarantee that .config values have a lifetime longer than the plugin
(no need to malloc 'export'). No semantic change.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 plugins/nbd/nbd.c | 68 ++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index fd1855e..a826363 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -52,14 +52,16 @@
 #include "byte-swapping.h"
 #include "cleanup.h"

-static char *sockname = NULL;
-static char *export = NULL;
+/* Connect to server via absolute name of Unix socket */
+static char *sockname;
+
+/* Name of export on remote server, default '', ignored for oldstyle */
+static const char *export;

 static void
 nbd_unload (void)
 {
   free (sockname);
-  free (export);
 }

 /* Called for each key=value passed on the command line.  This plugin
@@ -75,14 +77,8 @@ nbd_config (const char *key, const char *value)
     if (!sockname)
       return -1;
   }
-  else if (strcmp (key, "export") == 0) {
-    free (export);
-    export = strdup (value);
-    if (!export) {
-      nbdkit_error ("memory failure: %m");
-      return -1;
-    }
-  }
+  else if (strcmp (key, "export") == 0)
+    export = value;
   else {
     nbdkit_error ("unknown parameter '%s'", key);
     return -1;
@@ -107,11 +103,7 @@ nbd_config_complete (void)
     return -1;
   }
   if (!export)
-    export = strdup ("");
-  if (!export) {
-    nbdkit_error ("memory failure: %m");
-    return -1;
-  }
+    export = "";
   return 0;
 }

@@ -846,33 +838,46 @@ nbd_newstyle_haggle (struct handle *h)
   }
 }

-/* Create the per-connection handle. */
-static void *
-nbd_open (int readonly)
+/* Connect to a Unix socket */
+static int
+nbd_connect_unix(struct handle *h)
 {
-  struct handle *h;
   struct sockaddr_un sock = { .sun_family = AF_UNIX };
-  struct old_handshake old;
-  uint64_t version;

-  h = calloc (1, sizeof *h);
-  if (h == NULL) {
-    nbdkit_error ("malloc: %m");
-    return NULL;
-  }
+  nbdkit_debug ("connecting to Unix socket name=%s", sockname);
   h->fd = socket (AF_UNIX, SOCK_STREAM, 0);
   if (h->fd < 0) {
     nbdkit_error ("socket: %m");
-    free (h);
-    return NULL;
+    return -1;
   }
+
   /* We already validated length during nbd_config_complete */
   assert (strlen (sockname) <= sizeof sock.sun_path);
   memcpy (sock.sun_path, sockname, strlen (sockname));
   if (connect (h->fd, (const struct sockaddr *) &sock, sizeof sock) <
0) {
     nbdkit_error ("connect: %m");
+    return -1;
+  }
+  return 0;
+}
+
+/* Create the per-connection handle. */
+static void *
+nbd_open (int readonly)
+{
+  struct handle *h;
+  struct old_handshake old;
+  uint64_t version;
+
+  h = calloc (1, sizeof *h);
+  if (h == NULL) {
+    nbdkit_error ("malloc: %m");
+    return NULL;
+  }
+  h->fd = -1;
+
+  if (nbd_connect_unix (h) == -1)
     goto err;
-  }

   /* old and new handshake share same meaning of first 16 bytes */
   if (read_full (h->fd, &old, offsetof (struct old_handshake,
exportsize))) {
@@ -972,7 +977,8 @@ nbd_open (int readonly)
   return h;

  err:
-  close (h->fd);
+  if (h->fd >= 0)
+    close (h->fd);
   free (h);
   return NULL;
 }
-- 
2.20.1

I've documented a desire to do this for a while, time to actually
follow through and support connecting as a client to a TCP server.

Note that it is desirable to support the plugin connecting to an
encrypted server over TCP, then exposing the raw data over a local
Unix socket; that aspect requires yet more work, left for another
day. But even allowing an old-style client to connect to an
unencrypted TCP server is still useful.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 plugins/nbd/nbdkit-nbd-plugin.pod |  36 ++++++++--
 plugins/nbd/nbd.c                 | 113 ++++++++++++++++++++++++++----
 TODO                              |   3 -
 3 files changed, 127 insertions(+), 25 deletions(-)

diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod
b/plugins/nbd/nbdkit-nbd-plugin.pod
index 5add56f..21f0dcf 100644
--- a/plugins/nbd/nbdkit-nbd-plugin.pod
+++ b/plugins/nbd/nbdkit-nbd-plugin.pod
@@ -4,7 +4,7 @@ nbdkit-nbd-plugin - nbdkit nbd plugin

 =head1 SYNOPSIS

- nbdkit nbd socket=SOCKNAME [export=NAME]
+ nbdkit nbd { socket=SOCKNAME | hostname=HOST [port=PORT] } [export=NAME]

 =head1 DESCRIPTION

@@ -19,9 +19,10 @@ original server lacks it).  Use of this plugin along with
nbdkit
 filters (adding I<--filter> to the nbdkit command line) makes it
 possible to apply any nbdkit filter to any other NBD server.

-For now, this is limited to connecting to another NBD server over a
-named Unix socket without TLS, although it is feasible that future
-additions will support network sockets and encryption.
+For now, this is limited to connecting to another NBD server over an
+unencrypted connection; if the data is sensitive, it is better to
+stick to a Unix socket rather than transmitting plaintext over TCP. It
+is feasible that future additions will support encryption.

 =head1 PARAMETERS

@@ -29,10 +30,19 @@ additions will support network sockets and encryption.

 =item B<socket=>SOCKNAME

-Connect to the NBD server located at the Unix socket C<SOCKNAME>.  The
-server can speak either new or old style protocol.
+Connect to the NBD server located at the Unix socket C<SOCKNAME>.

-This parameter is required.
+=item B<hostname=>HOST
+
+Connect to the NBD server at the given remote C<HOST> using a TCP socket.
+
+=item B<port=>PORT
+
+When B<hostname> is supplied, use B<PORT> instead of the default
port
+10809.
+
+Either B<socket> or B<hostname> must be provided. The server can
speak
+either new or old style protocol.

 =item B<export=>NAME

@@ -66,6 +76,18 @@ the same task as the deprecated C<qemu-nbd -P 1 -f qcow2
    nbdkit --exit-with-parent --filter=partition nbd socket=$sock partition=1
&
    exec qemu-nbd -k $sock -f qcow2 /path/to/image.qcow2 )

+Conversely, expose the contents of export I<foo> from a new style
+server with unencrypted data to a client that can only consume
+unencrypted old style. Use I<--run> to clean up nbdkit at the time the
+client exits.
+
+ nbdkit -U - -o nbd hostname=example.com export=foo \
+  --run '/path/to/oldclient --socket=$unixsocket'
+
+ ┌────────────┐          ┌────────┐          ┌────────────┐
+ │ old client │ ────────▶│ nbdkit │ ────────▶│ new server │
+ └────────────┘   Unix   └────────┘   TCP    └────────────┘
+
 =head1 SEE ALSO

 L<nbdkit(1)>,
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index a826363..ce6859d 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -35,11 +35,15 @@
 #include <stdlib.h>
 #include <stddef.h>
 #include <stdbool.h>
+#include <stdio.h>
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
+#include <netdb.h>
+#include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <assert.h>
@@ -55,6 +59,13 @@
 /* Connect to server via absolute name of Unix socket */
 static char *sockname;

+/* Connect to server via TCP socket */
+static const char *hostname;
+static const char *port;
+
+/* Human-readable server description */
+static char *servname;
+
 /* Name of export on remote server, default '', ignored for oldstyle */
 static const char *export;

@@ -62,10 +73,12 @@ static void
 nbd_unload (void)
 {
   free (sockname);
+  free (servname);
 }

 /* Called for each key=value passed on the command line.  This plugin
- * accepts socket=<sockname> (required for now) and export=<name>
(optional).
+ * accepts socket=<sockname> or
hostname=<hostname>/port=<port>
+ * (exactly one connection required) and export=<name> (optional).
  */
 static int
 nbd_config (const char *key, const char *value)
@@ -77,6 +90,10 @@ nbd_config (const char *key, const char *value)
     if (!sockname)
       return -1;
   }
+  else if (strcmp (key, "hostname") == 0)
+    hostname = value;
+  else if (strcmp (key, "port") == 0)
+    port = value;
   else if (strcmp (key, "export") == 0)
     export = value;
   else {
@@ -87,29 +104,52 @@ nbd_config (const char *key, const char *value)
   return 0;
 }

-/* Check the user did pass a socket=<SOCKNAME> parameter. */
+/* Check the user passed exactly one socket description. */
 static int
 nbd_config_complete (void)
 {
-  struct sockaddr_un sock;
+  int r;

-  if (sockname == NULL) {
-    nbdkit_error ("you must supply the socket=<SOCKNAME> parameter
"
-                  "after the plugin name on the command line");
-    return -1;
+  if (sockname) {
+    struct sockaddr_un sock;
+
+    if (hostname || port) {
+      nbdkit_error ("cannot mix Unix socket and TCP hostname/port
parameters");
+      return -1;
+    }
+    if (strlen (sockname) > sizeof sock.sun_path) {
+      nbdkit_error ("socket file name too large");
+      return -1;
+    }
+    servname = strdup (sockname);
   }
-  if (strlen (sockname) > sizeof sock.sun_path) {
-    nbdkit_error ("socket file name too large");
-    return -1;
+  else {
+    if (!hostname) {
+      nbdkit_error ("must supply socket= or hostname= of external NBD
server");
+      return -1;
+    }
+    if (!port)
+      port = "10809";
+    if (strchr (hostname, ':'))
+      r = asprintf (&servname, "[%s]:%s", hostname, port);
+    else
+      r = asprintf (&servname, "%s:%s", hostname, port);
+    if (r < 0) {
+      nbdkit_error ("asprintf: %m");
+      return -1;
+    }
   }
+
   if (!export)
     export = "";
   return 0;
 }

 #define nbd_config_help \
-  "socket=<SOCKNAME>   (required) The Unix socket to connect
to.\n" \
-  "export=<NAME>                  Export name to connect to (default
\"\").\n" \
+  "socket=<SOCKNAME>      The Unix socket to connect to.\n" \
+  "hostname=<HOST>        The hostname for the TCP socket to connect
to.\n" \
+  "port=<PORT>            TCP port or service name to use (default
10809).\n" \
+  "export=<NAME>          Export name to connect to (default
\"\").\n" \

 #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL

@@ -199,7 +239,7 @@ nbd_mark_dead (struct handle *h)
   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&h->trans_lock);
   if (!h->dead) {
     nbdkit_debug ("permanent failure while talking to server %s: %m",
-                  sockname);
+                  servname);
     h->dead = true;
   }
   else if (!err)
@@ -861,6 +901,45 @@ nbd_connect_unix(struct handle *h)
   return 0;
 }

+/* Connect to a TCP socket */
+static int
+nbd_connect_tcp(struct handle *h)
+{
+  struct addrinfo hints = { .ai_family = AF_UNSPEC,
+                            .ai_socktype = SOCK_STREAM, };
+  struct addrinfo *result, *rp;
+  int r;
+  const int optval = 1;
+
+  nbdkit_debug ("connecting to TCP socket host=%s port=%s", hostname,
port);
+  r = getaddrinfo (hostname, port, &hints, &result);
+  if (r != 0) {
+    nbdkit_error ("getaddrinfo: %s", gai_strerror(r));
+    return -1;
+  }
+
+  for (rp = result; rp; rp = rp->ai_next) {
+    h->fd = socket(rp->ai_family, rp->ai_socktype,
rp->ai_protocol);
+    if (h->fd == -1)
+      continue;
+    if (connect (h->fd, rp->ai_addr, rp->ai_addrlen) != -1)
+      break;
+    close (h->fd);
+  }
+  freeaddrinfo (result);
+  if (rp == NULL) {
+    nbdkit_error ("connect: %m");
+    return -1;
+  }
+
+  if (setsockopt (h->fd, IPPROTO_TCP, TCP_NODELAY, &optval,
+                  sizeof(int)) == -1) {
+    nbdkit_error ("cannot set TCP_NODELAY option: %m");
+    return -1;
+  }
+  return 0;
+}
+
 /* Create the per-connection handle. */
 static void *
 nbd_open (int readonly)
@@ -876,7 +955,11 @@ nbd_open (int readonly)
   }
   h->fd = -1;

-  if (nbd_connect_unix (h) == -1)
+  if (sockname) {
+    if (nbd_connect_unix (h) == -1)
+      goto err;
+  }
+  else if (nbd_connect_tcp (h) == -1)
     goto err;

   /* old and new handshake share same meaning of first 16 bytes */
@@ -885,7 +968,7 @@ nbd_open (int readonly)
     goto err;
   }
   if (strncmp(old.nbdmagic, "NBDMAGIC", sizeof old.nbdmagic)) {
-    nbdkit_error ("wrong magic, %s is not an NBD server", sockname);
+    nbdkit_error ("wrong magic, %s is not an NBD server", servname);
     goto err;
   }
   version = be64toh (old.version);
diff --git a/TODO b/TODO
index 53d3358..b9ddb1e 100644
--- a/TODO
+++ b/TODO
@@ -97,9 +97,6 @@ nbdkit-nbd-plugin could use enhancements:
   would need TLS to support a plain client connecting to an encrypted
   server).

-* Support for connecting to a server over IP rather than just Unix
-  sockets.
-
 nbdkit-floppy-plugin:

 * Add boot sector support.  In theory this is easy (eg. using
-- 
2.20.1

ACK series, thanks.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html