From b25e12daff2c3e5ba933f85e8ba278f5bcba8f4d Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 27 Feb 2019 16:20:33 +0000 Subject: qemu-nbd: add support for authorization of TLS clients Currently any client which can complete the TLS handshake is able to use the NBD server. The server admin can turn on the 'verify-peer' option for the x509 creds to require the client to provide a x509 certificate. This means the client will have to acquire a certificate from the CA before they are permitted to use the NBD server. This is still a fairly low bar to cross. This adds a '--tls-authz OBJECT-ID' option to the qemu-nbd command which takes the ID of a previously added 'QAuthZ' object instance. This will be used to validate the client's x509 distinguished name. Clients failing the authorization check will not be permitted to use the NBD server. For example to setup authorization that only allows connection from a client whose x509 certificate distinguished name is CN=laptop.example.com,O=Example Org,L=London,ST=London,C=GB escape the commas in the name and use: qemu-nbd --object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\ endpoint=server,verify-peer=yes \ --object 'authz-simple,id=auth0,identity=CN=laptop.example.com,,\ O=Example Org,,L=London,,ST=London,,C=GB' \ --tls-creds tls0 \ --tls-authz authz0 \ ....other qemu-nbd args... NB: a real shell command line would not have leading whitespace after the line continuation, it is just included here for clarity. Reviewed-by: Juan Quintela Signed-off-by: Daniel P. Berrange Message-Id: <20190227162035.18543-2-berrange@redhat.com> Reviewed-by: Eric Blake [eblake: split long line in --help text, tweak 233 to show that whitespace after ,, in identity= portion is actually okay] Signed-off-by: Eric Blake --- include/block/nbd.h | 2 +- nbd/server.c | 10 +++++----- qemu-nbd.c | 19 ++++++++++++++++++- qemu-nbd.texi | 11 +++++++++-- tests/qemu-iotests/233 | 32 +++++++++++++++++++++++++++++--- tests/qemu-iotests/233.out | 11 +++++++++++ 6 files changed, 73 insertions(+), 12 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index c6ef1ef..b742ec4 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -326,7 +326,7 @@ void nbd_export_close_all(void); void nbd_client_new(QIOChannelSocket *sioc, QCryptoTLSCreds *tlscreds, - const char *tlsaclname, + const char *tlsauthz, void (*close_fn)(NBDClient *, bool)); void nbd_client_get(NBDClient *client); void nbd_client_put(NBDClient *client); diff --git a/nbd/server.c b/nbd/server.c index 0910d09..8ddfd3e 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -111,7 +111,7 @@ struct NBDClient { NBDExport *exp; QCryptoTLSCreds *tlscreds; - char *tlsaclname; + char *tlsauthz; QIOChannelSocket *sioc; /* The underlying data channel */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ @@ -686,7 +686,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client, tioc = qio_channel_tls_new_server(ioc, client->tlscreds, - client->tlsaclname, + client->tlsauthz, errp); if (!tioc) { return NULL; @@ -1348,7 +1348,7 @@ void nbd_client_put(NBDClient *client) if (client->tlscreds) { object_unref(OBJECT(client->tlscreds)); } - g_free(client->tlsaclname); + g_free(client->tlsauthz); if (client->exp) { QTAILQ_REMOVE(&client->exp->clients, client, next); nbd_export_put(client->exp); @@ -2425,7 +2425,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque) */ void nbd_client_new(QIOChannelSocket *sioc, QCryptoTLSCreds *tlscreds, - const char *tlsaclname, + const char *tlsauthz, void (*close_fn)(NBDClient *, bool)) { NBDClient *client; @@ -2437,7 +2437,7 @@ void nbd_client_new(QIOChannelSocket *sioc, if (tlscreds) { object_ref(OBJECT(client->tlscreds)); } - client->tlsaclname = g_strdup(tlsaclname); + client->tlsauthz = g_strdup(tlsauthz); client->sioc = sioc; object_ref(OBJECT(client->sioc)); client->ioc = QIO_CHANNEL(sioc); diff --git a/qemu-nbd.c b/qemu-nbd.c index 00c07fd..941ba72 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -58,6 +58,7 @@ #define QEMU_NBD_OPT_TLSCREDS 261 #define QEMU_NBD_OPT_IMAGE_OPTS 262 #define QEMU_NBD_OPT_FORK 263 +#define QEMU_NBD_OPT_TLSAUTHZ 264 #define MBR_SIZE 512 @@ -71,6 +72,7 @@ static int shared = 1; static int nb_fds; static QIONetListener *server; static QCryptoTLSCreds *tlscreds; +static const char *tlsauthz; static void usage(const char *name) { @@ -103,6 +105,8 @@ static void usage(const char *name) " --object type,id=ID,... define an object such as 'secret' for providing\n" " passwords and/or encryption keys\n" " --tls-creds=ID use id of an earlier --object to provide TLS\n" +" --tls-authz=ID use id of an earlier --object to provide\n" +" authorization\n" " -T, --trace [[enable=]][,events=][,file=]\n" " specify tracing options\n" " --fork fork off the server process and exit the parent\n" @@ -452,7 +456,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nb_fds++; nbd_update_server_watch(); - nbd_client_new(cioc, tlscreds, NULL, nbd_client_closed); + nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed); } static void nbd_update_server_watch(void) @@ -643,6 +647,7 @@ int main(int argc, char **argv) { "export-name", required_argument, NULL, 'x' }, { "description", required_argument, NULL, 'D' }, { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS }, + { "tls-authz", required_argument, NULL, QEMU_NBD_OPT_TLSAUTHZ }, { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS }, { "trace", required_argument, NULL, 'T' }, { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK }, @@ -862,6 +867,9 @@ int main(int argc, char **argv) g_free(trace_file); trace_file = trace_opt_parse(optarg); break; + case QEMU_NBD_OPT_TLSAUTHZ: + tlsauthz = optarg; + break; case QEMU_NBD_OPT_FORK: fork_process = true; break; @@ -934,12 +942,21 @@ int main(int argc, char **argv) error_report("TLS is not supported with a host device"); exit(EXIT_FAILURE); } + if (tlsauthz && list) { + error_report("TLS authorization is incompatible with export list"); + exit(EXIT_FAILURE); + } tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err); if (local_err) { error_report("Failed to get TLS creds %s", error_get_pretty(local_err)); exit(EXIT_FAILURE); } + } else { + if (tlsauthz) { + error_report("--tls-authz is not permitted without --tls-creds"); + exit(EXIT_FAILURE); + } } if (list) { diff --git a/qemu-nbd.texi b/qemu-nbd.texi index d0c5182..de342c7 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -117,6 +117,10 @@ option; or provide the credentials needed for connecting as a client in list mode. @item --fork Fork off the server process and exit the parent once the server is running. +@item --tls-authz=ID +Specify the ID of a qauthz object previously created with the +--object option. This will be used to authorize connecting users +against their x509 distinguished name. @item -v, --verbose Display extra debugging information. @item -h, --help @@ -142,13 +146,16 @@ qemu-nbd -f qcow2 file.qcow2 @end example Start a long-running server listening with encryption on port 10810, -and require clients to have a correct X.509 certificate to connect to +and whitelist clients with a specific X.509 certificate to connect to a 1 megabyte subset of a raw file, using the export name 'subset': @example qemu-nbd \ --object tls-creds-x509,id=tls0,endpoint=server,dir=/path/to/qemutls \ - --tls-creds tls0 -t -x subset -p 10810 \ + --object 'authz-simple,id=auth0,identity=CN=laptop.example.com,,\ + O=Example Org,,L=London,,ST=London,,C=GB' \ + --tls-creds tls0 --tls-authz auth0 \ + -t -x subset -p 10810 \ --image-opts driver=raw,offset=1M,size=1M,file.driver=file,file.filename=file.raw @end example diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233 index adb742f..5e5fe1e 100755 --- a/tests/qemu-iotests/233 +++ b/tests/qemu-iotests/233 @@ -61,6 +61,7 @@ tls_x509_create_root_ca "ca2" tls_x509_create_server "ca1" "server1" tls_x509_create_client "ca1" "client1" tls_x509_create_client "ca2" "client2" +tls_x509_create_client "ca1" "client3" echo echo "== preparing image ==" @@ -93,11 +94,15 @@ $QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port echo echo "== check TLS works ==" -obj=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0 -$QEMU_IMG info --image-opts --object $obj \ +obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0 +obj2=tls-creds-x509,dir=${tls_dir}/client3,endpoint=client,id=tls0 +$QEMU_IMG info --image-opts --object $obj1 \ driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \ 2>&1 | sed "s/$nbd_tcp_port/PORT/g" -$QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj \ +$QEMU_IMG info --image-opts --object $obj2 \ + driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \ + 2>&1 | sed "s/$nbd_tcp_port/PORT/g" +$QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj1 \ --tls-creds=tls0 echo @@ -120,6 +125,27 @@ $QEMU_IO -c 'r -P 0x11 1m 1m' -c 'w -P 0x22 1m 1m' --image-opts \ $QEMU_IO -f $IMGFMT -r -U -c 'r -P 0x22 1m 1m' "$TEST_IMG" | _filter_qemu_io echo +echo "== check TLS with authorization ==" + +nbd_server_stop + +nbd_server_start_tcp_socket \ + --object tls-creds-x509,dir=${tls_dir}/server1,endpoint=server,id=tls0,verify-peer=yes \ + --object "authz-simple,id=authz0,identity=CN=localhost,, \ + O=Cthulu Dark Lord Enterprises client1,,L=R'lyeh,,C=South Pacific" \ + --tls-authz authz0 \ + --tls-creds tls0 \ + -f $IMGFMT "$TEST_IMG" 2>> "$TEST_DIR/server.log" + +$QEMU_IMG info --image-opts \ + --object tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0 \ + driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 + +$QEMU_IMG info --image-opts \ + --object tls-creds-x509,dir=${tls_dir}/client3,endpoint=client,id=tls0 \ + driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 + +echo echo "== final server log ==" cat "$TEST_DIR/server.log" rm -f "$TEST_DIR/server.log" diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out index 6d45f3b..5acbc13 100644 --- a/tests/qemu-iotests/233.out +++ b/tests/qemu-iotests/233.out @@ -6,6 +6,7 @@ Generating a self signed certificate... Generating a signed certificate... Generating a signed certificate... Generating a signed certificate... +Generating a signed certificate... == preparing image == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 @@ -29,6 +30,10 @@ image: nbd://127.0.0.1:PORT file format: nbd virtual size: 64M (67108864 bytes) disk size: unavailable +image: nbd://127.0.0.1:PORT +file format: nbd +virtual size: 64M (67108864 bytes) +disk size: unavailable exports available: 1 export: '' size: 67108864 @@ -51,7 +56,13 @@ wrote 1048576/1048576 bytes at offset 1048576 read 1048576/1048576 bytes at offset 1048576 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +== check TLS with authorization == +qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=10809,tls-creds=tls0': Failed to read option reply: Cannot read from TLS channel: Software caused connection abort +qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=10809,tls-creds=tls0': Failed to read option reply: Cannot read from TLS channel: Software caused connection abort + == final server log == qemu-nbd: option negotiation failed: Verify failed: No certificate was found. qemu-nbd: option negotiation failed: Verify failed: No certificate was found. +qemu-nbd: option negotiation failed: TLS x509 authz check for CN=localhost,O=Cthulhu Dark Lord Enterprises client1,L=R'lyeh,C=South Pacific is denied +qemu-nbd: option negotiation failed: TLS x509 authz check for CN=localhost,O=Cthulhu Dark Lord Enterprises client3,L=R'lyeh,C=South Pacific is denied *** done -- cgit v1.1 From 000194556b65970a19ca437cd96b804a3f069f11 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 27 Feb 2019 16:20:34 +0000 Subject: nbd: allow authorization with nbd-server-start QMP command As with the previous patch to qemu-nbd, the nbd-server-start QMP command also needs to be able to specify authorization when enabling TLS encryption. First the client must create a QAuthZ object instance using the 'object-add' command: { 'execute': 'object-add', 'arguments': { 'qom-type': 'authz-list', 'id': 'authz0', 'parameters': { 'policy': 'deny', 'rules': [ { 'match': '*CN=fred', 'policy': 'allow' } ] } } } They can then reference this in the new 'tls-authz' parameter when executing the 'nbd-server-start' command: { 'execute': 'nbd-server-start', 'arguments': { 'addr': { 'type': 'inet', 'host': '127.0.0.1', 'port': '9000' }, 'tls-creds': 'tls0', 'tls-authz': 'authz0' } } Reviewed-by: Eric Blake Reviewed-by: Juan Quintela Signed-off-by: Daniel P. Berrange Message-Id: <20190227162035.18543-3-berrange@redhat.com> Signed-off-by: Eric Blake --- blockdev-nbd.c | 11 ++++++++--- hmp.c | 2 +- include/block/nbd.h | 2 +- qapi/block.json | 8 +++++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index d73ac1b..66eebab 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -23,6 +23,7 @@ typedef struct NBDServerData { QIONetListener *listener; QCryptoTLSCreds *tlscreds; + char *tlsauthz; } NBDServerData; static NBDServerData *nbd_server; @@ -36,7 +37,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, gpointer opaque) { qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); - nbd_client_new(cioc, nbd_server->tlscreds, NULL, + nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz, nbd_blockdev_client_closed); } @@ -52,6 +53,7 @@ static void nbd_server_free(NBDServerData *server) if (server->tlscreds) { object_unref(OBJECT(server->tlscreds)); } + g_free(server->tlsauthz); g_free(server); } @@ -87,7 +89,7 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) void nbd_server_start(SocketAddress *addr, const char *tls_creds, - Error **errp) + const char *tls_authz, Error **errp) { if (nbd_server) { error_setg(errp, "NBD server already running"); @@ -117,6 +119,8 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds, } } + nbd_server->tlsauthz = g_strdup(tls_authz); + qio_net_listener_set_client_func(nbd_server->listener, nbd_accept, NULL, @@ -131,11 +135,12 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds, void qmp_nbd_server_start(SocketAddressLegacy *addr, bool has_tls_creds, const char *tls_creds, + bool has_tls_authz, const char *tls_authz, Error **errp) { SocketAddress *addr_flat = socket_address_flatten(addr); - nbd_server_start(addr_flat, tls_creds, errp); + nbd_server_start(addr_flat, tls_creds, tls_authz, errp); qapi_free_SocketAddress(addr_flat); } diff --git a/hmp.c b/hmp.c index c2ad3f8..4a702d5 100644 --- a/hmp.c +++ b/hmp.c @@ -2373,7 +2373,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) goto exit; } - nbd_server_start(addr, NULL, &local_err); + nbd_server_start(addr, NULL, NULL, &local_err); qapi_free_SocketAddress(addr); if (local_err != NULL) { goto exit; diff --git a/include/block/nbd.h b/include/block/nbd.h index b742ec4..6d05983 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -332,7 +332,7 @@ void nbd_client_get(NBDClient *client); void nbd_client_put(NBDClient *client); void nbd_server_start(SocketAddress *addr, const char *tls_creds, - Error **errp); + const char *tls_authz, Error **errp); /* nbd_read * Reads @size bytes from @ioc. Returns 0 on success. diff --git a/qapi/block.json b/qapi/block.json index 5a79d63..7962308 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -225,6 +225,11 @@ # # @addr: Address on which to listen. # @tls-creds: (optional) ID of the TLS credentials object. Since 2.6 +# @tls-authz: ID of the QAuthZ authorization object used to validate +# the client's x509 distinguished name. This object is +# is only resolved at time of use, so can be deleted and +# recreated on the fly while the NBD server is active. +# If missing, it will default to denying access (since 4.0). # # Returns: error if the server is already running. # @@ -232,7 +237,8 @@ ## { 'command': 'nbd-server-start', 'data': { 'addr': 'SocketAddressLegacy', - '*tls-creds': 'str'} } + '*tls-creds': 'str', + '*tls-authz': 'str'} } ## # @nbd-server-add: -- cgit v1.1 From ab7c5d940b223586caafc034551a5389944e0d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 27 Feb 2019 16:20:35 +0000 Subject: nbd: fix outdated qapi docs syntax for tls-creds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Message-Id: <20190227162035.18543-4-berrange@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Eric Blake --- qapi/block.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/block.json b/qapi/block.json index 7962308..145c268 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -224,7 +224,7 @@ # QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME". # # @addr: Address on which to listen. -# @tls-creds: (optional) ID of the TLS credentials object. Since 2.6 +# @tls-creds: ID of the TLS credentials object (since 2.6). # @tls-authz: ID of the QAuthZ authorization object used to validate # the client's x509 distinguished name. This object is # is only resolved at time of use, so can be deleted and -- cgit v1.1 From 054be3605459d4342e9ee5a82ae0fcffeeb09e4d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 5 Mar 2019 12:29:08 -0600 Subject: iotests: Wait for qemu to end in 223 When iotest 223 was first written, it didn't matter if we waited for the qemu process to clean up. But with the introduction of a later qemu-nbd process trying to reuse the same file, there is a race where even though the asynchronous qemu process has responded to "quit", it has not yet had time to unlock the file and exit, resulting in: -[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false}, -{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true}, -{ "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] +qemu-nbd: Failed to blk_new_open 'tests/qemu-iotests/scratch/t.qcow2': Failed to get shared "write" lock +Is another process using the image [tests/qemu-iotests/scratch/t.qcow2]? +qemu-img: Could not open 'driver=nbd,server.type=unix,server.path=tests/qemu-iotests/scratch/qemu-nbd.sock,x-dirty-bitmap=qemu:dirty-bitmap:b': Failed to connect socket tests/qemu-iotests/scratch/qemu-nbd.sock: Connection refused +./common.nbd: line 33: kill: (11122) - No such process Fixes: ddd09448 Reported-by: Alberto Garcia Signed-off-by: Eric Blake Message-Id: <20190305182908.13557-1-eblake@redhat.com> Tested-by: Alberto Garcia Reviewed-by: Kevin Wolf --- tests/qemu-iotests/223 | 1 + tests/qemu-iotests/223.out | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223 index f120a01..c0a4f9c 100755 --- a/tests/qemu-iotests/223 +++ b/tests/qemu-iotests/223 @@ -179,6 +179,7 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove", _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return" _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "error" # Again _send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return" +wait=yes _cleanup_qemu echo echo "=== Use qemu-nbd as server ===" diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 6476b77..95c40a1 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -89,6 +89,7 @@ read 2097152/2097152 bytes at offset 2097152 {"return": {}} {"error": {"class": "GenericError", "desc": "NBD server not running"}} {"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} === Use qemu-nbd as server === -- cgit v1.1