From patchwork Mon Mar 27 03:07:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ashish Mittal X-Patchwork-Id: 743596 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vrzVL59zBz9s03 for ; Mon, 27 Mar 2017 14:07:54 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="KJ0cb0ro"; dkim-atps=neutral Received: from localhost ([::1]:43914 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1csL0a-00060M-6f for incoming@patchwork.ozlabs.org; Sun, 26 Mar 2017 23:07:52 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38801) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1csL0I-0005zi-Eq for qemu-devel@nongnu.org; Sun, 26 Mar 2017 23:07:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1csL0G-000510-RG for qemu-devel@nongnu.org; Sun, 26 Mar 2017 23:07:34 -0400 Received: from mail-it0-x244.google.com ([2607:f8b0:4001:c0b::244]:34488) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1csL0G-00050w-Ku for qemu-devel@nongnu.org; Sun, 26 Mar 2017 23:07:32 -0400 Received: by mail-it0-x244.google.com with SMTP id e75so1785152itd.1 for ; Sun, 26 Mar 2017 20:07:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=KzuPcseWcKPN8u9DMrotzxxDLFTS7VyRZeaOb+o/1VU=; b=KJ0cb0ro+7yu2WIhAmtyGXnU2/CnjJSpnUsS9kjGMqViDvM91ib+WEmL+jcVL9uML4 2cf84z/ae3/feWh4f5/dY6HpUw+ZNnOkfHmxXPZbk/gop1itMHUUdlF+Jrvh0wr2qTmz fEZAsFWD0BEef3wZq0jKQi7gT3UbWqhaj6TR8K63C/iZ8hmMtxB0TFysTpL5fy+bXLjn 5+biFg9FPyrvCsuNkFj7nN7iYqpfqMIqec7mKmHI/XWrM9ezyr0CIBpgMyTx9G//3VlQ Y6T7qUHeF0i80foRYBWYK36/B5wFMCFGizqOu9PJiMejAMmxgoE81mYv9Pb/J897jt2a KB5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=KzuPcseWcKPN8u9DMrotzxxDLFTS7VyRZeaOb+o/1VU=; b=Y7X5sPj6x8PXxjysXXS+mbzfZn3gJ5exc3cE4TUb9ppmLhl6blbQIbVj4pg9zjCA7Q OsL/FNDLLxx/lmCGR1O1lmfz35UkD7apHiGFTzwypUs1/zBdHkHlRKqbNlBg/DgkoNzk 8tNXWCxXfDZSM9sgxK8QtwjAN5Dc2asbWDMPAkDV8LbVnTbWXDXVS5co2XzTGtQ9i1ZU Wn6/x4kSjFNinX1srMwKPPNRPpolHr8vKpiQG6Bz9medAkUk7lqgqb0GISFAPLrm/XDb cW/x1nBO0cOGlbNejmWwog+s4iG5TE3C73HX+J98prKv79yYeGseWs/E/D6igY64CV4a JdPA== X-Gm-Message-State: AFeK/H1PY7lp1NnMwxr7HBqBIJ4SXqOUFWRILHNMogzn4ObdDH4XGMoN8hKGp/0bFr3t2dylpdioYbbntpDprQ== X-Received: by 10.36.84.199 with SMTP id t190mr7976215ita.114.1490584051904; Sun, 26 Mar 2017 20:07:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.171.65 with HTTP; Sun, 26 Mar 2017 20:07:31 -0700 (PDT) In-Reply-To: References: <20170227092207.GA18219@redhat.com> <20170301091851.GD17125@redhat.com> <20170306092305.GA9866@redhat.com> <20170313095723.GC4799@redhat.com> <20170320125528.GM3792@redhat.com> From: ashish mittal Date: Sun, 26 Mar 2017 20:07:31 -0700 Message-ID: To: "Daniel P. Berrange" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4001:c0b::244 Subject: Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs" X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Peter Maydell , "Venkatesha M.G." , Ashish Mittal , Fam Zheng , Nitin Jerath , Stefan Hajnoczi , Jeff Cody , qemu-devel , Rakesh Ranjan , Markus Armbruster , Suraj Singh , Ketan Nilangekar , Abhijit Dey , John Ferlan , Paolo Bonzini , Buddhi Madhav Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On Wed, Mar 22, 2017 at 5:03 PM, ashish mittal wrote: > On Mon, Mar 20, 2017 at 5:55 AM, Daniel P. Berrange wrote: >> On Fri, Mar 17, 2017 at 06:44:56PM -0700, ashish mittal wrote: >>> On Thu, Mar 16, 2017 at 5:29 PM, ashish mittal wrote: >>> > On Mon, Mar 13, 2017 at 2:57 AM, Daniel P. Berrange 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. >> > > I have made changes per above to the library. Please see commits - > > https://github.com/VeritasHyperScale/libqnio/commit/6c3261e9c9bb1350f4433a1ae4fcd98f7692cf39 > https://github.com/VeritasHyperScale/libqnio/commit/502c74278457e6ac86a4ee4ad9102e56ff3be5d4 > > Commit log: Enable secure mode based on the SSL/TLS args passed in iio_open() > > (1) Do not use /var/lib/libvxhs/secure to enable secure SSL mode on > the client side. > (2) Instead, enable SSL mode if the user passes TLS/SSL creds for > the block device on the qemu CLI. > > Will be posting v10 qemu vxhs patch soon. v10 will work with the > latest library changes, and will support passing tls-creds-x509 creds > for every vxhs block device. > I have posted v10 patches today. I've had to make one change in addition to what has been discussed. Basically I cannot use qcrypto_tls_creds_get_path() because its definition is under a #ifdef CONFIG_GNUTLS. Therefore my builds were failing whenever "gnutls" was not configured. I've replaced it as below - static int vxhs_open(BlockDriverState *bs, QDict *options, Regards, Ashish > >> 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/ :| diff --git a/block/vxhs.c b/block/vxhs.c index 0aa7849..44c0ecd 100644 --- a/block/vxhs.c +++ b/block/vxhs.c @@ -19,7 +19,6 @@ #include "qemu/uuid.h" #include "crypto/secret.h" #include "crypto/tlscredsx509.h" -#include "crypto/tlscredspriv.h" #define VXHS_OPT_FILENAME "filename" #define VXHS_OPT_VDISK_ID "vdisk-id" @@ -288,17 +287,17 @@ static void vxhs_get_tls_creds(const char *id, char **cacert, /* * Get the cacert, client_cert and client_key file names. */ - if (qcrypto_tls_creds_get_path(creds, QCRYPTO_TLS_CREDS_X509_CA_CERT, - true, cacert, errp) < 0 || - qcrypto_tls_creds_get_path(&creds_x509->parent_obj, - QCRYPTO_TLS_CREDS_X509_CLIENT_CERT, - false, cert, errp) < 0 || - qcrypto_tls_creds_get_path(&creds_x509->parent_obj, - QCRYPTO_TLS_CREDS_X509_CLIENT_KEY, - false, key, errp) < 0) { - error_setg(errp, - "Error retrieving information from TLS object"); + if (!creds->dir) { + error_setg(errp, "TLS object missing 'dir' property value"); + return; } + + *cacert = g_strdup_printf("%s/%s", creds->dir, + QCRYPTO_TLS_CREDS_X509_CA_CERT); + *cert = g_strdup_printf("%s/%s", creds->dir, + QCRYPTO_TLS_CREDS_X509_CLIENT_CERT); + *key = g_strdup_printf("%s/%s", creds->dir, + QCRYPTO_TLS_CREDS_X509_CLIENT_KEY); }