On 8/10/19 8:02 AM, Richard W.M. Jones wrote:
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.
Alternative: we could leave nbd_connect_uri() alone, and make it forward
to a new API nbd_connect_uri_flags(, default_flags).
+++ 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);
As written later in this patch, this change in the docs and example code
implies the use of the REQUIRE_TLS flag. Is that intentional that
passing all flags forbids the use of non-encrypted connections?
+++ 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;
+ ]
The REQUIRE_TLS flag is worth having, but putting it in the bitmask for
all flags makes for some odd semantics.
+}
+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.
This probably needs to call more attention to the fact that all flags
means encryption will be required.
@@ -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;
+ }
+
Are there any other flags we might want to support, such as permitting
or forbidding an authority section that specifies a username?
/* 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) {
Is this too strict? 'make check' may not use TCP, but I don't see that
as something we can't support for other uses of this binary. Similar
question to other changes under tests/.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org