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

[Daniel P . Berrangé]

On Wed, Jun 20, 2018 at 09:05:32AM -0500, Eric Blake wrote:
> On 06/20/2018 07:14 AM, Daniel P. Berrangé wrote:
> > From: "Daniel P. Berrange" <address@hidden>
> 
> 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)

Yeah its "only" been sitting in my tree since late 2016 :-)

> 
> > 
> > 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 <address@hidden>
> 
> 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

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|