[Libguestfs] [libnbd PATCH 2/2] uri: Reject nbd:unix:/path/to/socket as invalid URI

Eric Blake eblake at redhat.com
Wed Jun 26 02:10:00 UTC 2019


libxml2 parses it as valid per RFC 3986, as the nbd: scheme with no
authority and a relative path. This string has been used with qemu to
request a Unix socket, such that nbdkit --run produces it for $nbd
(compared to $unixsocket), but accepting it as a URI means that we
instead try to connect to a TCP socket with a default authority
(localhost, port 10809), and nbdkit ignores the path element
'unix:/path/to/socket' and you end up with a connection (or an
attempt) to a completely different server.

It's easiest to just reject all URIs that do not use the
scheme://authority form, at which point any path element is either
empty or begins with '/'.  This rejects 'nbd:unix:/path/to/socket'
which nbdkit produces, and also rejects 'nbd:/path/to/socket' (with no
authority but an absolute path) even though that form is less likely
to appear in the wild as qemu did not use it.

Reported-by: Martin Kletzander <mkletzan at redhat.com>
---
 lib/connect.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/lib/connect.c b/lib/connect.c
index 8b19e1e..e9d76ff 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -28,6 +28,7 @@
 #include <netinet/tcp.h>
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <assert.h>

 #ifdef HAVE_LIBXML2
 #include <libxml/uri.h>
@@ -275,6 +276,12 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
     goto cleanup;
   }

+  /* Insist on the scheme://[authority][/absname] form. */
+  if (strncmp (raw_uri + strlen (uri->scheme), "://", 3)) {
+    set_error (EINVAL, "URI must begin with '%s://'", uri->scheme);
+    goto cleanup;
+  }
+
   for (i = 0; i < nqueries; i++) {
     if (strcmp (queries[i].name, "socket") == 0)
       unixsocket = queries[i].value;
@@ -291,14 +298,11 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
    * nbd_unlocked_set_tls_* to match...
    */

-  /* Export name. Insist on the scheme://[authority][/absname] form. */
+  /* Export name. */
   if (uri->path) {
-    if (uri->path[0] == '/')
-      r = nbd_unlocked_set_export_name (h, &uri->path[1]);
-    else {
-      set_error (EINVAL, "URI must begin with '%s://'", uri->scheme);
-      goto cleanup;
-    }
+    /* Since we require scheme://authority above, any path is absolute */
+    assert (uri->path[0] == '/');
+    r = nbd_unlocked_set_export_name (h, &uri->path[1]);
   }
   else
     r = nbd_unlocked_set_export_name (h, "");
-- 
2.20.1




More information about the Libguestfs mailing list