Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

[Daniel P. Berrange]

On Fri, Mar 17, 2017 at 06:44:56PM -0700, ashish mittal wrote:
> On Thu, Mar 16, 2017 at 5:29 PM, ashish mittal <address@hidden> wrote:
> > On Mon, Mar 13, 2017 at 2:57 AM, Daniel P. Berrange <address@hidden> wrote:
> >> On Tue, Mar 07, 2017 at 05:27:55PM -0800, ashish mittal wrote:
> >>> Thanks! There is one more input I need some help with!
> >>>
> >>> VxHS network library opens a fixed number of connection channels to a
> >>> given host, and all the vdisks (that connect to the same host) share
> >>> these connection channels.
> >>>
> >>> Therefore, we need to open secure channels to a specific target host
> >>> only once for the first vdisk that connects to that host. All the
> >>> other vdisks that connect to the same target host will share the same
> >>> set of secure communication channels.
> >>>
> >>> I hope the above scheme is acceptable?
> >>>
> >>> If yes, then we have a couple of options to implement this:
> >>>
> >>> (1) Accept the TLS credentials per vdisk using the previously
> >>> discussed --object tls-creds-x509 mechanism. In this case, if more
> >>> than one vdisk have the same host info, then we will use only the
> >>> first one's creds to set up the secure connection, and ignore the
> >>> others. vdisks that connect to different target hosts will use their
> >>> individual tls-creds-x509 to set up the secure channels. This is, of
> >>> course, not relevant for qemu-img/qemu-io as they can open only one
> >>> vdisk at a time.
> >>
> >> It looks like option 1 here is the way to go. Just report an error if
> >> there are multiple creds provided for the same host and they don't
> >> match.
> >>
> >
> > I have made changes to implement option 1 in the library (branch
> > per_channel_ssl).
> > Can you please help review it?
> > https://github.com/VeritasHyperScale/libqnio/compare/per_channel_ssl
> >
> > Here's the changelog:
> > (1) Changed code to not use instance UUID for setting up SSL context.
> > (2) User is supposed to pass the cacert, client_key and client_cert
> >     files to iio_open(). These will be used to set up a per-channel secure 
> > SSL
> >     connection to the server. All three values are needed to set up a
> >     secure connection.
> > (3) If the secure channel to a particular host is already open, other
> >     block device connections to the same host will have to provide
> >     TLS/SSL credentials that match the original one.
> > (4) Set default locations for trusted client CA certificates
> >      based on user specified cacert file.
> >
> > NB - I have given steps to test SSL communication (using the supplied
> > test client/server programs) in the commit log. I have not tested
> > using qemu binary yet. Will run more tests in the days to come.
> >
> > qemu vxhs patch changes should be pretty straightforward, given that
> > we have already discussed how to implement passing --object
> > tls-creds-x509 per block device.
> >
> > Thanks!
> >
> 
> Update -
> (1) Successfully tested SSL communication using qemu-io and the test server.
> (2) Minor changes to per_channel_ssl branch.
> (3) Created a pull request.
> 
> Please review per convenience. Thanks!

IIUC, on that branch the 'is_secure()' method is still looking for the
directory /var/lib/libvxhs/secure to exist on the host. If that is not
present, then it appears to be silently ignoring the SSL certs passed
in from QEMU.

IMHO it should enable TLS when 'cacert' passed to iio_open is not NULL,
not relying on a magic directory to exist.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|