[Libguestfs] [PATCH libnbd] api: Allow NBD URIs to be restricted.
Richard W.M. Jones
rjones at redhat.com
Sun Oct 20 11:06:24 UTC 2019
New APIs are added which let you enable or disable features of NBD
URIs, mainly for security reasons.
tls-psk-file is *disabled* by default for obvious security reasons.
All other features are enabled by default.
---
generator/generator | 127 +++++++++++++++++++++++++++++++++++++++++++-
lib/handle.c | 26 +++++++++
lib/internal.h | 5 ++
lib/uri.c | 31 +++++++++--
tests/connect-uri.c | 2 +
5 files changed, 186 insertions(+), 5 deletions(-)
diff --git a/generator/generator b/generator/generator
index 6cfe1fd..c2ff0db 100755
--- a/generator/generator
+++ b/generator/generator
@@ -991,7 +991,15 @@ let handshake_flags = {
"NO_ZEROES", 1 lsl 1;
]
}
-let all_flags = [ cmd_flags; handshake_flags ]
+let allow_transport_flags = {
+ flag_prefix = "ALLOW_TRANSPORT";
+ flags = [
+ "TCP", 1 lsl 0;
+ "UNIX", 1 lsl 1;
+ "VSOCK", 1 lsl 2;
+ ]
+}
+let all_flags = [ cmd_flags; handshake_flags; allow_transport_flags ]
(* Calls.
*
@@ -1445,6 +1453,75 @@ C<\"qemu:dirty-bitmap:...\"> for qemu-nbd
see_also = ["L<nbd_block_status(3)>"];
};
+ "set_uri_allow_transports", {
+ default_call with
+ args = [ Flags ("mask", allow_transport_flags) ]; ret = RErr;
+ permitted_states = [ Created ];
+ shortdesc = "set the allowed transports in NBD URIs";
+ longdesc = "\
+Set which transports are allowed to appear in NBD URIs. The
+default is to allow any transports.
+
+The C<mask> parameter may contain any of the following flags
+ORed together:
+
+=over 4
+
+=item C<LIBNBD_ALLOW_TRANSPORT_TCP>
+
+=item C<LIBNBD_ALLOW_TRANSPORT_UNIX>
+
+=item C<LIBNBD_ALLOW_TRANSPORT_VSOCK>
+
+=back";
+ see_also = ["L<nbd_connect_uri(3)>"; "L<nbd_set_uri_allow_tls(3)>"];
+ };
+
+ "set_uri_allow_tls", {
+ default_call with
+ args = [ Enum ("tls", tls_enum) ]; ret = RErr;
+ permitted_states = [ Created ];
+ shortdesc = "set the allowed TLS settings in NBD URIs";
+ longdesc = "\
+Set which TLS settings are allowed to appear in NBD URIs. The
+default is to allow either non-TLS or TLS URIs.
+
+The C<tls> parameter can be:
+
+=over 4
+
+=item C<LIBNBD_TLS_DISABLE>
+
+TLS URIs are not permitted, ie. a URI such as C<nbds://...>
+will be rejected.
+
+=item C<LIBNBD_TLS_ALLOW>
+
+This is the default. TLS may be used or not, depending on
+whether the URI uses C<nbds> or C<nbd>.
+
+=item C<LIBNBD_TLS_REQUIRE>
+
+TLS URIs are required. All URIs must use C<nbs>.
+
+=back";
+ see_also = ["L<nbd_connect_uri(3)>"; "L<nbd_set_uri_allow_transports(3)>"];
+ };
+
+ "set_uri_allow_local_file", {
+ default_call with
+ args = [ Bool "allow" ]; ret = RErr;
+ permitted_states = [ Created ];
+ shortdesc = "set the allowed transports in NBD URIs";
+ longdesc = "\
+Allow NBD URIs to reference local files. This is I<disabled>
+by default.
+
+Currently this setting only controls whether the C<tls-psk-file>
+parameter in NBD URIs is allowed.";
+ see_also = ["L<nbd_connect_uri(3)>"];
+ };
+
"connect_uri", {
default_call with
args = [ String "uri" ]; ret = RErr;
@@ -1539,7 +1616,50 @@ be present for the other transports.
=item B<tls-psk-file=>F<PSKFILE>
-Set the PSK file. See L<nbd_set_tls_psk_file(3)>.
+Set the PSK file. See L<nbd_set_tls_psk_file(3)>. Note
+this is not allowed by default - see next section.
+
+=back
+
+=head2 Disable URI features
+
+For security reasons you might want to disable certain URI
+features. Pre-filtering URIs is error-prone and should not
+be attempted. Instead use the libnbd APIs below to control
+what can appear in URIs. Note you must call these functions
+on the same handle before calling C<nbd_connect_uri> or
+L<nbd_aio_connect_uri(3)>.
+
+=over 4
+
+=item TCP, Unix domain socket or C<AF_VSOCK> transports
+
+Default: all allowed
+
+To select which transports are allowed call
+L<nbd_set_uri_allow_transports(3)>.
+
+=item TLS
+
+Default: both non-TLS and TLS connections allowed
+
+To force TLS off or on in URIs call
+L<nbd_set_uri_allow_tls(3)>.
+
+=item Connect to Unix domain socket in the local filesystem
+
+Default: allowed
+
+To prevent this you must disable the C<+unix> transport
+using L<nbd_set_uri_allow_transports(3)>.
+
+=item Read from local files
+
+Default: denied
+
+To allow URIs to contain references to local files
+(eg. for parameters like C<tls-psk-file>) call
+L<nbd_set_uri_allow_local_file(3)>.
=back
@@ -2900,6 +3020,9 @@ let first_version = [
"aio_connect_socket", (1, 2);
"connect_vsock", (1, 2);
"aio_connect_vsock", (1, 2);
+ "set_uri_allow_transports", (1, 2);
+ "set_uri_allow_tls", (1, 2);
+ "set_uri_allow_local_file", (1, 2);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/lib/handle.c b/lib/handle.c
index 1d4a527..cdbca86 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -64,6 +64,11 @@ nbd_create (void)
h->unique = 1;
h->tls_verify_peer = true;
h->request_sr = true;
+
+ h->uri_allow_transports = (uint32_t) -1;
+ h->uri_allow_tls = LIBNBD_TLS_ALLOW;
+ h->uri_allow_local_file = false;
+
h->gflags = (LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE |
LIBNBD_HANDSHAKE_FLAG_NO_ZEROES);
@@ -360,3 +365,24 @@ nbd_unlocked_get_protocol (struct nbd_handle *h)
return h->protocol;
}
+
+int
+nbd_unlocked_set_uri_allow_transports (struct nbd_handle *h, uint32_t mask)
+{
+ h->uri_allow_transports = mask;
+ return 0;
+}
+
+int
+nbd_unlocked_set_uri_allow_tls (struct nbd_handle *h, int tls)
+{
+ h->uri_allow_tls = tls;
+ return 0;
+}
+
+int
+nbd_unlocked_set_uri_allow_local_file (struct nbd_handle *h, bool allow)
+{
+ h->uri_allow_local_file = allow;
+ return 0;
+}
diff --git a/lib/internal.h b/lib/internal.h
index 435cd36..50c0a9b 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -76,6 +76,11 @@ struct nbd_handle {
bool request_sr;
char **request_meta_contexts;
+ /* Allowed in URIs, see lib/uri.c. */
+ uint32_t uri_allow_transports;
+ int uri_allow_tls;
+ bool uri_allow_local_file;
+
/* Global flags from the server. */
uint16_t gflags;
diff --git a/lib/uri.c b/lib/uri.c
index b3dfe7d..704641c 100644
--- a/lib/uri.c
+++ b/lib/uri.c
@@ -216,6 +216,24 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
goto cleanup;
}
+ /* Check the transport is allowed. */
+ if ((transport == tcp &&
+ (h->uri_allow_transports & LIBNBD_ALLOW_TRANSPORT_TCP) == 0) ||
+ (transport == unix_sock &&
+ (h->uri_allow_transports & LIBNBD_ALLOW_TRANSPORT_UNIX) == 0) ||
+ (transport == vsock &&
+ (h->uri_allow_transports & LIBNBD_ALLOW_TRANSPORT_VSOCK) == 0)) {
+ set_error (EPERM, "URI transport %s is not permitted", uri->scheme);
+ goto cleanup;
+ }
+
+ /* Check TLS is allowed. */
+ if ((tls && h->uri_allow_tls == LIBNBD_TLS_DISABLE) ||
+ (!tls && h->uri_allow_tls == LIBNBD_TLS_REQUIRE)) {
+ set_error (EPERM, "URI TLS setting %s is not permitted", uri->scheme);
+ goto cleanup;
+ }
+
/* Parse the socket parameter. */
for (i = 0; i < nqueries; i++) {
if (strcmp (queries[i].name, "socket") == 0)
@@ -239,9 +257,16 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
/* Look for some tls-* parameters. XXX More to come. */
for (i = 0; i < nqueries; i++) {
- if (strcmp (queries[i].name, "tls-psk-file") == 0 &&
- nbd_unlocked_set_tls_psk_file (h, queries[i].value) == -1)
- goto cleanup;
+ if (strcmp (queries[i].name, "tls-psk-file") == 0) {
+ if (! h->uri_allow_local_file) {
+ set_error (EPERM,
+ "local file access (tls-psk-file) is not allowed, "
+ "call nbd_set_uri_allow_local_file to enable this");
+ goto cleanup;
+ }
+ if (nbd_unlocked_set_tls_psk_file (h, queries[i].value) == -1)
+ goto cleanup;
+ }
}
/* Username. */
diff --git a/tests/connect-uri.c b/tests/connect-uri.c
index 9eb34d1..6e7d168 100644
--- a/tests/connect-uri.c
+++ b/tests/connect-uri.c
@@ -73,6 +73,8 @@ main (int argc, char *argv[])
exit (77);
}
+ nbd_set_uri_allow_local_file (nbd, true);
+
if (nbd_connect_uri (nbd, URI) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
--
2.23.0
More information about the Libguestfs
mailing list