From 81762f81aaa34650685475495d04939b7bae064c Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 11 Jun 2019 10:22:33 -0500 Subject: [PATCH] nbd: Add TLS client support Well, really add parameters to pass on to libnbd which does all the heavy lifting :) This also adds testsuite coverage that we support TLS over Unix sockets. Signed-off-by: Eric Blake --- TODO | 13 +---- plugins/nbd/nbd.c | 77 +++++++++++++++++++++++++- plugins/nbd/nbdkit-nbd-plugin.pod | 69 ++++++++++++++++++----- tests/Makefile.am | 6 +- tests/{test-tls-psk.sh => test-nbd-tls-psk.sh} | 56 +++++++++---------- tests/{test-tls.sh => test-nbd-tls.sh} | 48 +++++++++------- tests/test-tls-psk.sh | 2 +- tests/test-tls.sh | 4 +- 8 files changed, 194 insertions(+), 81 deletions(-) copy tests/{test-tls-psk.sh => test-nbd-tls-psk.sh} (64%) copy tests/{test-tls.sh => test-nbd-tls.sh} (69%) diff --git a/TODO b/TODO index b9ddb1ee..332400b3 100644 --- a/TODO +++ b/TODO @@ -90,13 +90,6 @@ qemu-nbd for these use cases. https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02971.html is a partial solution but it needs cleaning up. -nbdkit-nbd-plugin could use enhancements: - -* Enable client-side TLS (right now, the nbd plugin allows us to - support an encrypted client connecting to a plain server; but we - would need TLS to support a plain client connecting to an encrypted - server). - nbdkit-floppy-plugin: * Add boot sector support. In theory this is easy (eg. using @@ -159,11 +152,7 @@ Filters allow certain types of composition, but others would not be possible, for example RAIDing over multiple nbd sources. Because the plugin API limits us to loading a single plugin to the server, the best way to do this (and the most robust) is to compose multiple -nbdkit processes. - -The nbd plugin (plugins/nbd) already contains an NBD client, so we -could factor this client out and make it available to other plugins to -use. +nbdkit processes. Perhaps libnbd will prove useful for this purpose. Build-related ------------- diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 0f54805c..fa14ea1b 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -100,6 +100,13 @@ static unsigned long retry; static bool shared; static struct handle *shared_handle; +/* Control TLS settings */ +static int tls = -1; +static char *tls_certificates; +static int tls_verify = -1; +static const char *tls_username; +static char *tls_psk; + static struct handle *nbdplug_open_handle (int readonly); static void nbdplug_close_handle (struct handle *h); @@ -109,12 +116,15 @@ nbdplug_unload (void) if (shared) nbdplug_close_handle (shared_handle); free (sockname); + free (tls_certificates); + free (tls_psk); } /* Called for each key=value passed on the command line. This plugin * accepts socket=, hostname=/port=, or * [uri=] (exactly one connection required), and optional - * parameters export=, retry= and shared=. + * parameters export=, retry=, shared= and various + * tls settings. */ static int nbdplug_config (const char *key, const char *value) @@ -151,6 +161,37 @@ nbdplug_config (const char *key, const char *value) return -1; shared = r; } + else if (strcmp (key, "tls") == 0) { + if (strcasecmp (value, "require") == 0 || + strcasecmp (value, "required") == 0 || + strcasecmp (value, "force") == 0) + tls = 2; + else { + tls = nbdkit_parse_bool (value); + if (tls == -1) + exit (EXIT_FAILURE); + } + } + else if (strcmp (key, "tls-certificates") == 0) { + free (tls_certificates); + tls_certificates = nbdkit_absolute_path (value); + if (!tls_certificates) + return -1; + } + else if (strcmp (key, "tls-verify") == 0) { + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + tls_verify = r; + } + else if (strcmp (key, "tls-username") == 0) + tls_username = value; + else if (strcmp (key, "tls-psk") == 0) { + free (tls_psk); + tls_psk = nbdkit_absolute_path (value); + if (!tls_psk) + return -1; + } else { nbdkit_error ("unknown parameter '%s'", key); return -1; @@ -208,6 +249,23 @@ nbdplug_config_complete (void) if (!export) export = ""; + if (tls == -1) + tls = tls_certificates || tls_verify >= 0 || tls_username || tls_psk; + if (tls > 0) { + struct nbd_handle *nbd = nbd_create (); + + if (!nbd) { + nbdkit_error ("unable to query libnbd details: %s", nbd_get_error ()); + return -1; + } + if (!nbd_supports_tls (nbd)) { + nbdkit_error ("libnbd was compiled without tls support"); + nbd_close (nbd); + return -1; + } + nbd_close (nbd); + } + if (shared && (shared_handle = nbdplug_open_handle (false)) == NULL) return -1; return 0; @@ -222,6 +280,11 @@ nbdplug_config_complete (void) "retry= Retry connection up to N seconds (default 0).\n" \ "shared= True to share one server connection among all clients,\n" \ " rather than a connection per client (default false).\n" \ + "tls= How to use TLS; one of 'off', 'on', or 'require'.\n" \ + "tls-certificates= Directory containing files for X.509 certificates.\n" \ + "tls-verify= True (default for X.509) to validate server.\n" \ + "tls-username= Override username presented in X.509 TLS.\n" \ + "tls-psk= File containing Pre-Shared Key for TLS.\n" \ static void nbdplug_dump_plugin (void) @@ -233,6 +296,7 @@ nbdplug_dump_plugin (void) exit (EXIT_FAILURE); } printf ("libnbd_version=%s\n", nbd_get_version (nbd)); + printf ("libnbd_tls=%d\n", nbd_supports_tls (nbd)); printf ("libnbd_uri=%d\n", nbd_supports_uri (nbd)); nbd_close (nbd); } @@ -427,6 +491,17 @@ nbdplug_open_handle (int readonly) goto err; if (nbd_add_meta_context (h->nbd, "base:allocation") == -1) goto err; + if (nbd_set_tls (h->nbd, tls) == -1) + goto err; + if (tls_certificates && + nbd_set_tls_certificates (h->nbd, tls_certificates) == -1) + goto err; + if (tls_verify >= 0 && nbd_set_tls_verify_peer (h->nbd, tls_verify) == -1) + goto err; + if (tls_username && nbd_set_tls_username (h->nbd, tls_username) == -1) + goto err; + if (tls_psk && nbd_set_tls_psk_file (h->nbd, tls_psk) == -1) + goto err; if (uri) r = nbd_connect_uri (h->nbd, uri); else if (sockname) diff --git a/plugins/nbd/nbdkit-nbd-plugin.pod b/plugins/nbd/nbdkit-nbd-plugin.pod index 98f45a35..171a9775 100644 --- a/plugins/nbd/nbdkit-nbd-plugin.pod +++ b/plugins/nbd/nbdkit-nbd-plugin.pod @@ -5,7 +5,8 @@ nbdkit-nbd-plugin - nbdkit nbd plugin =head1 SYNOPSIS nbdkit nbd { socket=SOCKNAME | hostname=HOST [port=PORT] | [uri=]URI } - [export=NAME] [retry=N] [shared=BOOL] + [export=NAME] [retry=N] [shared=BOOL] [tls=MODE] [tls-certificates=DIR] + [tls-verify=BOOL] [tls-username=NAME] [tls-psk=FILE] =head1 DESCRIPTION @@ -20,10 +21,9 @@ original server lacks it). Use of this plugin along with nbdkit filters (adding I<--filter> to the nbdkit command line) makes it possible to apply any nbdkit filter to any other NBD server. -For now, this is limited to connecting to another NBD server over an -unencrypted connection; if the data is sensitive, it is better to -stick to a Unix socket rather than transmitting plaintext over TCP. It -is feasible that future additions will support encryption. +Remember that when using this plugin as a bridge between an encrypted +and a non-encrypted endpoint, it is best to preserve encryption over +TCP and use plaintext only on a Unix socket. =head1 PARAMETERS @@ -91,6 +91,46 @@ shell glob. The B parameter is only available when the plugin was compiled against libnbd with URI support; C will contain C if this is the case. +=item BMODE + +Selects which TLS mode to use with the server. If no other tls option +is present, this defaults to C, where the client does not attempt +encryption (and may be rejected by a server that requires it). If +omitted but another tls option is present, this defaults to C, +where the client opportunistically attempts a TLS handshake, but will +continue running unencrypted if the server does not support +encryption. If set to C or if the C parameter is used +with a scheme that requires encryption (such as C), then +this requires an encrypted connection to the server. + +The B parameter is only available when the plugin was compiled +against libnbd with TLS support; C will +contain C if this is the case. Note the difference +between C<--tls=...> controlling what nbdkit serves, and C +controlling what the nbd plugin connects to as a client. + +=item BDIR + +This specifies the directory containing X.509 client certificates to +present to the server. + +=item BBOOL + +This defaults to true; setting it to false disables server name +verification, which opens you to potential Man-in-the-Middle (MITM) +attacks, but allows for a simpler setup for distributing certificates. + +=item BNAME + +If provided, this overrides the user name to present to the server +alongside the certificate. + +=item BFILE + +If provided, this is the filename containing the Pre-Shared Keys (PSK) +to present to the server. While this is easier to set up than X.509, +it requires that the PSK file be transmitted over a secure channel. + =back =head1 EXAMPLES @@ -104,9 +144,9 @@ that the old server exits. nbdkit --exit-with-parent --tls=require nbd socket=$sock & exec /path/to/oldserver --socket=$sock ) - ┌────────────┐ ┌────────┐ ┌────────────┐ - │ new client │ ────────▶│ nbdkit │ ────────▶│ old server │ - └────────────┘ TCP └────────┘ Unix └────────────┘ + ┌────────────┐ TLS ┌────────┐ plaintext ┌────────────┐ + │ new client │ ────────▶│ nbdkit │ ───────────▶│ old server │ + └────────────┘ TCP └────────┘ Unix └────────────┘ Combine nbdkit's partition filter with qemu-nbd's ability to visit qcow2 files (nbdkit does not have a native qcow2 plugin), performing @@ -121,16 +161,17 @@ utilize a 5-second retry to give qemu-nbd time to create the socket: exec qemu-nbd -k $sock -f qcow2 /path/to/image.qcow2 ) Conversely, expose the contents of export I from a new style -server with unencrypted data to a client that can only consume +server with encrypted data to a client that can only consume unencrypted old style. Use I<--run> to clean up nbdkit at the time the -client exits. +client exits. In general, note that it is best to keep the plaintext +connection limited to a Unix socket on the local machine. - nbdkit -U - -o nbd hostname=example.com export=foo \ + nbdkit -U - -o --tls=off nbd hostname=example.com export=foo tls=require \ --run '/path/to/oldclient --socket=$unixsocket' - ┌────────────┐ ┌────────┐ ┌────────────┐ - │ old client │ ────────▶│ nbdkit │ ────────▶│ new server │ - └────────────┘ Unix └────────┘ TCP └────────────┘ + ┌────────────┐ plaintext ┌────────┐ TLS ┌────────────┐ + │ old client │ ───────────▶│ nbdkit │ ────────▶│ new server │ + └────────────┘ Unix └────────┘ TCP └────────────┘ Learn which features are provided by libnbd by inspecting the C lines: diff --git a/tests/Makefile.am b/tests/Makefile.am index aaf74502..26ca2a12 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -81,6 +81,8 @@ EXTRA_DIST = \ test-memory-largest.sh \ test-memory-largest-for-qemu.sh \ test-nbd-extents.sh \ + test-nbd-tls.sh \ + test-nbd-tls-psk.sh \ test-nozero.sh \ test-null-extents.sh \ test_ocaml_plugin.ml \ @@ -534,7 +536,9 @@ TESTS += \ # nbd plugin test. LIBGUESTFS_TESTS += test-nbd TESTS += \ - test-nbd-extents.sh + test-nbd-extents.sh \ + test-nbd-tls.sh \ + test-nbd-tls-psk.sh test_nbd_SOURCES = test-nbd.c test.h test_nbd_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS) diff --git a/tests/test-tls-psk.sh b/tests/test-nbd-tls-psk.sh similarity index 64% copy from tests/test-tls-psk.sh copy to tests/test-nbd-tls-psk.sh index c5f4f974..d0bbc468 100755 --- a/tests/test-tls-psk.sh +++ b/tests/test-nbd-tls-psk.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # nbdkit -# Copyright (C) 2018 Red Hat Inc. +# Copyright (C) 2019 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -36,25 +36,18 @@ set -x requires qemu-img --version -if ! qemu-img --help | grep -- --object; then - echo "$0: 'qemu-img' command does not have the --object option" - exit 77 -fi - -# Does the qemu-img binary support PSK? -if LANG=C qemu-img info --object tls-creds-psk,id=id disk |& - grep -sq "invalid object type: tls-creds-psk" -then - echo "$0: 'qemu-img' command does not support TLS-PSK" - exit 77 -fi - # Does the nbdkit binary support TLS? if ! nbdkit --dump-config | grep -sq tls=yes; then echo "$0: nbdkit built without TLS support" exit 77 fi +# Does the nbd plugin support TLS? +if ! nbdkit --dump-plugin nbd | grep -sq libnbd_tls=1; then + echo "$0: nbd plugin built without TLS support" + exit 77 +fi + # Did we create the PSK keys file? # Probably 'certtool' is missing. if [ ! -s keys.psk ]; then @@ -62,22 +55,27 @@ if [ ! -s keys.psk ]; then exit 77 fi -# Unfortunately qemu cannot do TLS over a Unix domain socket (nbdkit -# probably can, although it is not tested). Find an unused port to -# listen on. -pick_unused_port +sock1=`mktemp -u` +sock2=`mktemp -u` +pid1="test-nbd-tls-psk.pid1" +pid2="test-nbd-tls-psk.pid2" + +files="$sock1 $sock2 $pid1 $pid2 nbd-tls-psk.out" +rm -f $files +cleanup_fn rm -f $files + +# Run encrypted server +start_nbdkit -P "$pid1" -U "$sock1" \ + --tls=require --tls-psk=keys.psk example1 -cleanup_fn rm -f tls-psk.pid tls-psk.out -start_nbdkit -P tls-psk.pid -p $port -n \ - --tls=require --tls-psk=keys.psk example1 +# Run nbd plugin as intermediary +LIBNBD_DEBUG=1 start_nbdkit -P "$pid2" -U "$sock2" --tls=off \ + nbd tls=require tls-psk=keys.psk tls-username=qemu socket="$sock1" -# Run qemu-img against the server. -LANG=C \ -qemu-img info \ - --object "tls-creds-psk,id=tls0,endpoint=client,dir=$PWD" \ - --image-opts "file.driver=nbd,file.host=localhost,file.port=$port,file.tls-creds=tls0" > tls-psk.out +# Run unencrypted client +LANG=C qemu-img info -f raw "nbd+unix:///?socket=$sock2" > nbd-tls-psk.out -cat tls-psk.out +cat nbd-tls-psk.out -grep -sq "^file format: raw" tls-psk.out -grep -sq "^virtual size: 100M" tls-psk.out +grep -sq "^file format: raw" nbd-tls-psk.out +grep -sq "^virtual size: 100M" nbd-tls-psk.out diff --git a/tests/test-tls.sh b/tests/test-nbd-tls.sh similarity index 69% copy from tests/test-tls.sh copy to tests/test-nbd-tls.sh index 4c3d010e..af824d23 100755 --- a/tests/test-tls.sh +++ b/tests/test-nbd-tls.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # nbdkit -# Copyright (C) 2017 Red Hat Inc. +# Copyright (C) 2019 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -36,17 +36,18 @@ set -x requires qemu-img --version -if ! qemu-img --help | grep -- --object; then - echo "$0: 'qemu-img' command does not have the --object option" - exit 77 -fi - # Does the nbdkit binary support TLS? if ! nbdkit --dump-config | grep -sq tls=yes; then echo "$0: nbdkit built without TLS support" exit 77 fi +# Does the nbd plugin support TLS? +if ! nbdkit --dump-plugin nbd | grep -sq libnbd_tls=1; then + echo "$0: nbd plugin built without TLS support" + exit 77 +fi + # Did we create the PKI files? # Probably 'certtool' is missing. pkidir="$PWD/pki" @@ -55,22 +56,27 @@ if [ ! -f "$pkidir/ca-cert.pem" ]; then exit 77 fi -# Unfortunately qemu cannot do TLS over a Unix domain socket (nbdkit -# probably can, although it is not tested). Find an unused port to -# listen on. -pick_unused_port +sock1=`mktemp -u` +sock2=`mktemp -u` +pid1="test-nbd-tls.pid1" +pid2="test-nbd-tls.pid2" + +files="$sock1 $sock2 $pid1 $pid2 nbd-tls.out" +rm -f $files +cleanup_fn rm -f $files + +# Run encrypted server +start_nbdkit -P "$pid1" -U "$sock1" \ + --tls=require --tls-certificates="$pkidir" example1 -cleanup_fn rm -f tls.pid tls.out -start_nbdkit -P tls.pid -p $port -n --tls=require \ - --tls-certificates="$pkidir" example1 +# Run nbd plugin as intermediary +LIBNBD_DEBUG=1 start_nbdkit -P "$pid2" -U "$sock2" --tls=off \ + nbd tls=require tls-certificates="$pkidir" socket="$sock1" -# Run qemu-img against the server. -LANG=C \ -qemu-img info \ - --object "tls-creds-x509,id=tls0,endpoint=client,dir=$pkidir" \ - --image-opts "file.driver=nbd,file.host=localhost,file.port=$port,file.tls-creds=tls0" > tls.out +# Run unencrypted client +LANG=C qemu-img info -f raw "nbd+unix:///?socket=$sock2" > nbd-tls.out -cat tls.out +cat nbd-tls.out -grep -sq "^file format: raw" tls.out -grep -sq "^virtual size: 100M" tls.out +grep -sq "^file format: raw" nbd-tls.out +grep -sq "^virtual size: 100M" nbd-tls.out diff --git a/tests/test-tls-psk.sh b/tests/test-tls-psk.sh index c5f4f974..393f5893 100755 --- a/tests/test-tls-psk.sh +++ b/tests/test-tls-psk.sh @@ -63,7 +63,7 @@ if [ ! -s keys.psk ]; then fi # Unfortunately qemu cannot do TLS over a Unix domain socket (nbdkit -# probably can, although it is not tested). Find an unused port to +# can, but that is tested in tests-nbd-tls-psk.sh). Find an unused port to # listen on. pick_unused_port diff --git a/tests/test-tls.sh b/tests/test-tls.sh index 4c3d010e..70d40aea 100755 --- a/tests/test-tls.sh +++ b/tests/test-tls.sh @@ -55,8 +55,8 @@ if [ ! -f "$pkidir/ca-cert.pem" ]; then exit 77 fi -# Unfortunately qemu cannot do TLS over a Unix domain socket (nbdkit -# probably can, although it is not tested). Find an unused port to +# Unfortunately qemu 4.0 cannot do TLS over a Unix domain socket (nbdkit +# can, but that is tested in tests-nbd-tls.sh). Find an unused port to # listen on. pick_unused_port -- 2.11.4.GIT