Re: [Qemu-block] [PATCH v2 1/6] qemu-nbd: add support for authorization of TLS clients

[Daniel P . Berrangé]

On Wed, Jun 20, 2018 at 08:58:40AM -0500, Eric Blake wrote:
> On 06/20/2018 07:14 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.
> > 
> > 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
> 
> Is the space in O= intentional?

Yes it really does have a space, though perhaps for sake of
illustration I'll remove the space.

> 
> > 
> > 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 \
> 
> you need shell quoting to preserve the space. Also, the indentation breaks
> the intent that these long lines are single arguments, not separate
> arguments.

Yeah I'll definitely remove the space, because it just makes this
more confusing than needed.

I know the \ isn't really supposed to separate args, but I don't
see a nicer way to format it for a commit message while respecting
line length. I figured people would understand what I meant, but
the fact that I've indented the second line to vertically align.

> 
> >             --tls-creds tls0 \
> >             --tls-authz authz0
> >        ....other qemu-nbd args...
> 
> Is it also worth a sample command line using JSON syntax?

Probably overkill for the commit message IMHO.

> 
> > 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >   include/block/nbd.h |  2 +-
> >   nbd/server.c        | 10 +++++-----
> >   qemu-nbd.c          | 13 ++++++++++++-
> >   qemu-nbd.texi       |  4 ++++
> >   4 files changed, 22 insertions(+), 7 deletions(-)
> > 
> 
> Reviewed-by: Eric Blake <address@hidden>
> 
> -- 
> 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 :|