[Libguestfs] [PATCH libnbd 9/9] FOR DISCUSSION ONLY: api: Add ‘allow’ parameter to nbd_connect_uri to control permitted URIs.

Richard W.M. Jones
2019-08-10T13:02:48Z

Add an extra parameter to nbd_connect_uri to control what URIs are
permitted, in case the caller wants to pass in user-controlled URIs
but have some control over who/what/how the connection happens.  For
example:

  nbd_connect_uri (nbd, "nbd://localhost",
LIBNBD_CONNECT_URI_REQUIRE_TLS)
  => error: URI must specify an encrypted connection: Permission denied

This obviously breaks the existing C API.
---
 TODO                                 |  1 -
 docs/libnbd.pod                      |  2 +-
 examples/batched-read-write.c        |  2 +-
 examples/strict-structured-reads.c   |  2 +-
 examples/threaded-reads-and-writes.c |  5 +--
 generator/generator                  | 50 +++++++++++++++++++++++++---
 lib/connect.c                        | 33 ++++++++++++++++--
 tests/aio-parallel-load.c            |  3 +-
 tests/connect-uri-tcp.c              |  2 +-
 tests/connect-uri-unix.c             |  3 +-
 10 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/TODO b/TODO
index 8e067c0..1dd64d7 100644
--- a/TODO
+++ b/TODO
@@ -31,7 +31,6 @@ Suggested API improvements:

   connecting:
   - nbd_connect_tcp: allow control over whether IPv4 or IPv6 is desired
-  - nbd_connect_uri: allow control over which features are enabled
   - nbd_connect_command: allow passing char **env

   TLS:
diff --git a/docs/libnbd.pod b/docs/libnbd.pod
index 01964de..7018621 100644
--- a/docs/libnbd.pod
+++ b/docs/libnbd.pod
@@ -364,7 +364,7 @@ when it is available.

 To connect to a URI via the high level API, use:

- nbd_connect_uri (nbd, "nbd://example.com/");
+ nbd_connect_uri (nbd, "nbd://example.com/", LIBNBD_CONNECT_URI_ALL);

 This feature is implemented by calling other libnbd APIs to set up the
 export name, TLS parameters, and finally connect over a Unix domain
diff --git a/examples/batched-read-write.c b/examples/batched-read-write.c
index d39a1e5..1b876da 100644
--- a/examples/batched-read-write.c
+++ b/examples/batched-read-write.c
@@ -143,7 +143,7 @@ main (int argc, char *argv[])
   /* Connect synchronously as this is simpler. */
   if (argc == 2) {
     if (strstr (argv[1], "://")) {
-      if (nbd_connect_uri (nbd, argv[1]) == -1) {
+      if (nbd_connect_uri (nbd, argv[1], LIBNBD_CONNECT_URI_ALL) == -1) {
         fprintf (stderr, "%s\n", nbd_get_error ());
         exit (EXIT_FAILURE);
       }
diff --git a/examples/strict-structured-reads.c
b/examples/strict-structured-reads.c
index b3880b7..fcf1186 100644
--- a/examples/strict-structured-reads.c
+++ b/examples/strict-structured-reads.c
@@ -197,7 +197,7 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
   if (strstr (argv[1], "://")) {
-    if (nbd_connect_uri (nbd, argv[1]) == -1) {
+    if (nbd_connect_uri (nbd, argv[1], LIBNBD_CONNECT_URI_ALL) == -1) {
       fprintf (stderr, "%s\n", nbd_get_error ());
       exit (EXIT_FAILURE);
     }
diff --git a/examples/threaded-reads-and-writes.c
b/examples/threaded-reads-and-writes.c
index 85d6e42..864e2f4 100644
--- a/examples/threaded-reads-and-writes.c
+++ b/examples/threaded-reads-and-writes.c
@@ -89,7 +89,7 @@ main (int argc, char *argv[])
   /* Connect first to check if the server supports writes and multi-conn. */
   if (argc == 2) {
     if (strstr (argv[1], "://")) {
-      if (nbd_connect_uri (nbd, argv[1]) == -1) {
+      if (nbd_connect_uri (nbd, argv[1], LIBNBD_CONNECT_URI_ALL) == -1) {
         fprintf (stderr, "%s\n", nbd_get_error ());
         exit (EXIT_FAILURE);
       }
@@ -211,7 +211,8 @@ start_thread (void *arg)

   if (status->argc == 2) {
     if (strstr (status->argv[1], "://")) {
-      if (nbd_connect_uri (nbd, status->argv[1]) == -1) {
+      if (nbd_connect_uri (nbd, status->argv[1],
+                           LIBNBD_CONNECT_URI_ALL) == -1) {
         fprintf (stderr, "%s\n", nbd_get_error ());
         exit (EXIT_FAILURE);
       }
diff --git a/generator/generator b/generator/generator
index fe24738..5f5523a 100755
--- a/generator/generator
+++ b/generator/generator
@@ -939,7 +939,17 @@ let cmd_flags = {
     "REQ_ONE", 1 lsl 3;
   ]
 }
-let all_flags = [ cmd_flags ]
+let connect_uri_allow_flags = {
+  flag_prefix = "CONNECT_URI";
+  all_flags_bitmask = true;
+  flags = [
+    "ALLOW_TCP",   1 lsl 0;
+    "ALLOW_UNIX",  1 lsl 1;
+    "ALLOW_TLS",   1 lsl 2;
+    "REQUIRE_TLS", 1 lsl 3;
+  ]
+}
+let all_flags = [ cmd_flags; connect_uri_allow_flags ]

 (* Calls.
  *
@@ -1225,7 +1235,8 @@ C<\"qemu:dirty-bitmap:...\"> for qemu-nbd

   "connect_uri", {
     default_call with
-    args = [ String "uri" ]; ret = RErr;
+    args = [ String "uri"; Flags ("allow",
connect_uri_allow_flags) ];
+    ret = RErr;
     permitted_states = [ Created ];
     shortdesc = "connect to NBD URI";
     longdesc = "\
@@ -1238,7 +1249,37 @@ the connection has been made.
 This call will fail if libnbd was not compiled with libxml2; you can
 test whether this is the case with C<nbd_supports_uri>.  Support for
 URIs that require TLS will fail if libnbd was not compiled with
-gnutls; you can test whether this is the case with
C<nbd_supports_tls>.";
+gnutls; you can test whether this is the case with C<nbd_supports_tls>.
+
+The C<allow> parameter lets you choose which NBD URI features
+are enabled, in case for example you don't want to allow
+remote connections.  Currently defined flags are:
+
+=over 4
+
+=item C<LIBNBD_CONNECT_URI_ALLOW_TCP>
+
+Allow TCP sockets.
+
+=item C<LIBNBD_CONNECT_URI_ALLOW_UNIX>
+
+Allow Unix domain sockets.
+
+=item C<LIBNBD_CONNECT_URI_ALLOW_TLS>
+
+Allow TLS encryption.
+
+=item C<LIBNBD_CONNECT_URI_REQUIRE_TLS>
+
+Require TLS encryption.
+
+=item C<LIBNBD_CONNECT_URI_ALL>
+
+Enables all features which are defined at the time that
+the program is compiled.  Later features added to libnbd
+will not be allowed unless you recompile your program.
+
+=back";
   };

   "connect_unix", {
@@ -1737,7 +1778,8 @@ on the connection.";

   "aio_connect_uri", {
     default_call with
-    args = [ String "uri" ]; ret = RErr;
+    args = [ String "uri"; Flags ("allow",
connect_uri_allow_flags) ];
+    ret = RErr;
     permitted_states = [ Created ];
     shortdesc = "connect to an NBD URI";
     longdesc = "\
diff --git a/lib/connect.c b/lib/connect.c
index f98bcdb..a4c594e 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -73,9 +73,10 @@ wait_until_connected (struct nbd_handle *h)

 /* Connect to an NBD URI. */
 int
-nbd_unlocked_connect_uri (struct nbd_handle *h, const char *uri)
+nbd_unlocked_connect_uri (struct nbd_handle *h,
+                          const char *uri, uint32_t allow)
 {
-  if (nbd_unlocked_aio_connect_uri (h, uri) == -1)
+  if (nbd_unlocked_aio_connect_uri (h, uri, allow) == -1)
     return -1;

   return wait_until_connected (h);
@@ -228,7 +229,8 @@ error:
 #endif

 int
-nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
+nbd_unlocked_aio_connect_uri (struct nbd_handle *h,
+                              const char *raw_uri, uint32_t allow)
 {
 #ifdef HAVE_LIBXML2
   xmlURIPtr uri = NULL;
@@ -276,6 +278,31 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
     goto cleanup;
   }

+  /* If the user specified the REQUIRE_TLS flag, we assume they must
+   * also mean to ALLOW_TLS.
+   */
+  if ((allow & LIBNBD_CONNECT_URI_REQUIRE_TLS) != 0)
+    allow |= LIBNBD_CONNECT_URI_ALLOW_TLS;
+
+  /* Check allow flags. */
+  if (tcp && !(allow & LIBNBD_CONNECT_URI_ALLOW_TCP)) {
+    set_error (EPERM, "TCP URIs are not allowed");
+    goto cleanup;
+  }
+  if (!tcp && !(allow & LIBNBD_CONNECT_URI_ALLOW_UNIX)) {
+    set_error (EPERM, "Unix domain socket URIs are not allowed");
+    goto cleanup;
+  }
+  if (tls && !(allow & LIBNBD_CONNECT_URI_ALLOW_TLS)) {
+    set_error (EPERM, "TLS encrypted URIs are not allowed");
+    goto cleanup;
+  }
+  if (!tls && (allow & LIBNBD_CONNECT_URI_REQUIRE_TLS)) {
+    set_error (EPERM, "URI must specify an encrypted connection "
+               "(use nbds: or nbds+unix:)");
+    goto cleanup;
+  }
+
   /* Insist on the scheme://[authority][/absname][?queries] form. */
   if (strncmp (raw_uri + strlen (uri->scheme), "://", 3)) {
     set_error (EINVAL, "URI must begin with '%s://'",
uri->scheme);
diff --git a/tests/aio-parallel-load.c b/tests/aio-parallel-load.c
index 614c22b..16c2aa2 100644
--- a/tests/aio-parallel-load.c
+++ b/tests/aio-parallel-load.c
@@ -220,7 +220,8 @@ start_thread (void *arg)

   /* Connect to nbdkit. */
   if (strstr (connection, "://")) {
-    if (nbd_connect_uri (nbd, connection) == -1) {
+    if (nbd_connect_uri (nbd, connection,
+                         LIBNBD_CONNECT_URI_ALLOW_UNIX) == -1) {
       fprintf (stderr, "%s\n", nbd_get_error ());
       exit (EXIT_FAILURE);
     }
diff --git a/tests/connect-uri-tcp.c b/tests/connect-uri-tcp.c
index 0c5c1df..b33d94c 100644
--- a/tests/connect-uri-tcp.c
+++ b/tests/connect-uri-tcp.c
@@ -80,7 +80,7 @@ main (int argc, char *argv[])

   snprintf (uri, sizeof uri, "nbd://localhost:%d", port);

-  if (nbd_connect_uri (nbd, uri) == -1) {
+  if (nbd_connect_uri (nbd, uri, LIBNBD_CONNECT_URI_ALLOW_TCP) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
diff --git a/tests/connect-uri-unix.c b/tests/connect-uri-unix.c
index 1e27d26..601fdf0 100644
--- a/tests/connect-uri-unix.c
+++ b/tests/connect-uri-unix.c
@@ -71,7 +71,8 @@ main (int argc, char *argv[])
     exit (77);
   }

-  if (nbd_connect_uri (nbd, "nbd+unix:///?socket=" SOCKET) == -1) {
+  if (nbd_connect_uri (nbd, "nbd+unix:///?socket=" SOCKET,
+                       LIBNBD_CONNECT_URI_ALLOW_UNIX) == -1) {
     fprintf (stderr, "%s\n", nbd_get_error ());
     exit (EXIT_FAILURE);
   }
--
2.22.0