Re: [Qemu-devel] [PATCH v2 2/6] nbd: allow authorization with nbd-server-start QMP command

[Eric Blake <eblake@redhat.com>]

On 06/20/2018 07:14 AM, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>

I thought you preferred the UTF-8 accent in your Author lines these 
days?  Or is this because this patch has been sitting around in your 
local repo prior to the point where you switched your git config author 
spelling? (Also applies to S-o-b in the series)

> 
> 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:
> 

> They can then reference this in the new 'tls-authz' parameter when
> executing the 'nbd-server-start' command:
> 

> @@ -132,11 +137,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);

Relies on QMP generated code setting tls_authz = NULL if has_tls_authz 
is false (but no different than the fact that we already relied on it 
for tls_creds).  Someday it would be nice to get rid of the has_FOO for 
optional strings, but that's not your problem.

> +++ b/qapi/block.json
> @@ -197,6 +197,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 3.0
>   #
>   # Returns: error if the server is already running.
>   #
> @@ -204,7 +209,8 @@
>   ##
>   { 'command': 'nbd-server-start',
>     'data': { 'addr': 'SocketAddressLegacy',
> -            '*tls-creds': 'str'} }
> +            '*tls-creds': 'str',
> +            '*tls-authz': 'str'} }

Reviewed-by: Eric Blake <eblake@redhat.com>

Although patch 1 and 2 touch NBD, I'm happy for Dan to be the one that 
merges it as part of the larger series.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org