[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 1/3] qemu-nbd: add support for authorization
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v6 1/3] qemu-nbd: add support for authorization of TLS clients |
Date: |
Wed, 27 Feb 2019 10:43:40 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 |
On 2/27/19 10:20 AM, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <address@hidden>
>
> 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.
>
> +++ b/qemu-nbd.c
> @@ -103,6 +105,7 @@ 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
> authorization\n"
Usage line exceeds 80 columns; I don't mind splitting the line.
> @@ -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' \
A long line may be necessary here, unless the whitespace in the
identity= parameter inserted by the line continuation is harmless. Long
lines in man pages are annoying, but even worse is an example that
copies-and-pastes incorrectly. I may just s/^ *O/O/.
>
> +== 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
A rather uninformative message for the client to figure out why it
failed, but (as with all things security-related), giving too many
details may in itself be an improper information leak. At any rate, I
don't know that you could make it work any better, so it is not a
problem with this patch. It may be the sign of a server bug for closing
the socket too soon (before the client has a chance to read an actual
error message), where we could tweak things to provoke a nicer error
than 'Software caused connection abort', but that would be a separate patch.
Reviewed-by: Eric Blake <address@hidden>
I can make the minor changes as part of staging through my NBD tree
without needing a v7.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org