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