QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Maxim Levitsky <mlevitsk@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
	qemu-block@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 08/13] qcrypto: add the plumbing for encryption management
Date: Thu, 22 Aug 2019 14:47:03 +0300
Message-ID: <1cedbbf007f4447f9424d09848c6486b0341b43d.camel@redhat.com> (raw)
In-Reply-To: <20190822111608.GN3267@redhat.com>

On Thu, 2019-08-22 at 12:16 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 14, 2019 at 11:22:14PM +0300, Maxim Levitsky wrote:
> > This adds qcrypto_block_manage_encryption, which
> >  is thin wrapper around manage_encryption of the crypto driver
> >  which is also added
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  crypto/block.c         | 29 +++++++++++++++++++++++++++++
> >  crypto/blockpriv.h     |  9 +++++++++
> >  include/crypto/block.h | 27 +++++++++++++++++++++++++++
> >  3 files changed, 65 insertions(+)
> > 
> > diff --git a/crypto/block.c b/crypto/block.c
> > index ee96759f7d..5916e49aba 100644
> > --- a/crypto/block.c
> > +++ b/crypto/block.c
> > @@ -20,6 +20,7 @@
> >  
> >  #include "qemu/osdep.h"
> >  #include "qapi/error.h"
> > +
> >  #include "blockpriv.h"
> >  #include "block-qcow.h"
> >  #include "block-luks.h"
> > @@ -282,6 +283,34 @@ void qcrypto_block_free(QCryptoBlock *block)
> >  }
> >  
> >  
> > +int qcrypto_block_setup_encryption(QCryptoBlock *block,
> > +                                   QCryptoBlockReadFunc readfunc,
> > +                                   QCryptoBlockWriteFunc writefunc,
> > +                                   void *opaque,
> > +                                   enum BlkSetupEncryptionAction action,
> > +                                   QCryptoEncryptionSetupOptions *options,
> > +                                   bool force,
> > +                                   Error **errp)
> > +{
> > +    if (!block->driver->setup_encryption) {
> > +        error_setg(errp,
> > +                "Crypto format %s doesn't support management of encryption keys",
> > +                QCryptoBlockFormat_str(block->format));
> > +        return -1;
> > +    }
> > +
> > +    return block->driver->setup_encryption(block,
> > +                                           readfunc,
> > +                                           writefunc,
> > +                                           opaque,
> > +                                           action,
> > +                                           options,
> > +                                           force,
> > +                                           errp);
> > +}
> > +
> > +
> > +
> >  typedef int (*QCryptoCipherEncDecFunc)(QCryptoCipher *cipher,
> >                                          const void *in,
> >                                          void *out,
> > diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
> > index 71c59cb542..804965dca3 100644
> > --- a/crypto/blockpriv.h
> > +++ b/crypto/blockpriv.h
> > @@ -81,6 +81,15 @@ struct QCryptoBlockDriver {
> >  
> >      bool (*has_format)(const uint8_t *buf,
> >                         size_t buflen);
> > +
> > +    int (*setup_encryption)(QCryptoBlock *block,
> > +                            QCryptoBlockReadFunc readfunc,
> > +                            QCryptoBlockWriteFunc writefunc,
> > +                            void *opaque,
> > +                            enum BlkSetupEncryptionAction action,
> > +                            QCryptoEncryptionSetupOptions *options,
> > +                            bool force,
> > +                            Error **errp);
> >  };
> >  
> >  
> > diff --git a/include/crypto/block.h b/include/crypto/block.h
> > index fe12899831..60d46e3efc 100644
> > --- a/include/crypto/block.h
> > +++ b/include/crypto/block.h
> > @@ -23,6 +23,7 @@
> >  
> >  #include "crypto/cipher.h"
> >  #include "crypto/ivgen.h"
> > +#include "block/block.h"
> >  
> >  typedef struct QCryptoBlock QCryptoBlock;
> >  
> > @@ -268,4 +269,30 @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);
> >   */
> >  void qcrypto_block_free(QCryptoBlock *block);
> >  
> > +
> > +/**
> > + * qcrypto_block_setup_encryption:
> > + * @block: the block encryption object
> > + *
> > + * @readfunc: callback for reading data from the volume header
> > + * @writefunc: callback for writing data to the volume header
> > + * @opaque: data to pass to @readfunc and @writefunc
> > + * @action: tell the driver the setup action (add/erase currently)
> > + * @options: driver specific options, that specify
> > + *           what encryption settings to manage
> > + * @force: hint for the driver to allow unsafe operation
> > + * @errp: error pointer
> > + *
> > + * Adds/Erases a new encryption key using @options
> 
> I'd prefer to see separate APIs for add + erase instead
> of overloading. It'll lead to a clearer API from callers
> POV to see exactly which parameters are for each action.


The downside of this is some duplication of code in the middle layers.
I don't mind doing that if you are OK with that.

Also note that if we go with amend options, we will have to use the single
API.


Best regards,
	Maxim Levitsky




  reply index

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-14 20:22 [Qemu-devel] [PATCH 00/13] RFC: luks/encrypted qcow2 key management Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 01/13] block-crypto: misc refactoring Maxim Levitsky
2019-08-20 16:38   ` Max Reitz
2019-08-22  0:05     ` Maxim Levitsky
2019-08-22 14:34       ` Max Reitz
2019-08-22 15:04         ` Maxim Levitsky
2019-08-21 15:39   ` Daniel P. Berrangé
2019-08-22  0:08     ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 02/13] qcrypto-luks: " Maxim Levitsky
2019-08-15 21:40   ` [Qemu-devel] [Qemu-block] " John Snow
2019-08-19 14:21     ` Maxim Levitsky
2019-08-22 10:29     ` Daniel P. Berrangé
2019-08-22 11:04       ` Maxim Levitsky
2019-08-22 11:10         ` Daniel P. Berrangé
2019-08-22 11:13           ` Maxim Levitsky
2019-08-20 17:36   ` [Qemu-devel] " Max Reitz
2019-08-21 23:59     ` Maxim Levitsky
2019-08-22 14:32       ` Max Reitz
2019-08-25 10:46         ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 03/13] qcrypto-luks: refactoring: extract load/store/check/parse header functions Maxim Levitsky
2019-08-20 18:01   ` Max Reitz
2019-08-21 22:43     ` Maxim Levitsky
2019-08-22 10:32       ` Daniel P. Berrangé
2019-08-22 10:57         ` Maxim Levitsky
2019-08-22 10:34       ` Daniel P. Berrangé
2019-08-25 14:11         ` Maxim Levitsky
2019-08-22 10:38   ` Daniel P. Berrangé
2019-08-25 14:09     ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 04/13] qcrypto-luks: refactoring: simplify the math used for keyslot locations Maxim Levitsky
2019-08-22 10:47   ` Daniel P. Berrangé
2019-08-25 14:30     ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 05/13] qcrypto-luks: clear the masterkey and password before freeing them always Maxim Levitsky
2019-08-20 18:12   ` Max Reitz
2019-08-21 22:40     ` Maxim Levitsky
2019-08-22 10:49     ` Daniel P. Berrangé
2019-08-22 10:56       ` Maxim Levitsky
2019-08-25 15:31         ` Maxim Levitsky
2019-08-25 17:15           ` Maxim Levitsky
2019-08-27  8:55           ` Daniel P. Berrangé
2019-08-21 23:01   ` [Qemu-devel] [Qemu-block] " Nir Soffer
2019-08-21 23:11     ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 06/13] qcrypto-luks: implement more rigorous header checking Maxim Levitsky
2019-08-22 11:04   ` Daniel P. Berrangé
2019-08-25 15:40     ` Maxim Levitsky
2019-08-25 16:08       ` Maxim Levitsky
2019-08-26 13:31         ` Eric Blake
2019-08-26 13:39           ` Maxim Levitsky
2019-08-27  8:56         ` Daniel P. Berrangé
2019-08-14 20:22 ` [Qemu-devel] [PATCH 07/13] block: add manage-encryption command (qmp and blockdev) Maxim Levitsky
2019-08-20 18:27   ` Max Reitz
2019-08-21 22:32     ` Maxim Levitsky
2019-08-22 11:14     ` Daniel P. Berrangé
2019-08-21 11:47   ` Markus Armbruster
2019-08-21 22:24     ` Maxim Levitsky
2019-08-22 14:07       ` Markus Armbruster
2019-08-25 16:42         ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 08/13] qcrypto: add the plumbing for encryption management Maxim Levitsky
2019-08-22 11:16   ` Daniel P. Berrangé
2019-08-22 11:47     ` Maxim Levitsky [this message]
2019-08-22 11:49       ` Daniel P. Berrangé
2019-08-14 20:22 ` [Qemu-devel] [PATCH 09/13] qcrypto-luks: implement the encryption key management Maxim Levitsky
2019-08-22 11:27   ` Daniel P. Berrangé
2019-08-25 17:01     ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 10/13] block/crypto: " Maxim Levitsky
2019-08-22 11:29   ` Daniel P. Berrangé
2019-08-22 11:36     ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 11/13] block/qcow2: implement the encryption key managment Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 12/13] qemu-img: implement key management Maxim Levitsky
2019-08-20 18:29   ` Max Reitz
2019-08-21 22:33     ` Maxim Levitsky
2019-08-22 11:32     ` Daniel P. Berrangé
2019-08-22 14:42       ` Max Reitz
2019-08-25 17:04         ` Maxim Levitsky
2019-08-14 20:22 ` [Qemu-devel] [PATCH 13/13] iotests : add tests for encryption " Maxim Levitsky
2019-08-14 21:08 ` [Qemu-devel] [PATCH 00/13] RFC: luks/encrypted qcow2 " Eric Blake
2019-08-15  8:49   ` Maxim Levitsky
2019-08-15  9:10   ` Kevin Wolf
2019-08-15 14:18     ` Markus Armbruster
2019-08-15 14:44       ` Maxim Levitsky
2019-08-15 15:00         ` Eric Blake
2019-08-19 12:35           ` Maxim Levitsky
2019-08-21 11:31             ` Markus Armbruster
2019-08-21 13:22               ` Maxim Levitsky
2019-08-20 17:59 ` Max Reitz
2019-08-21 22:00   ` Maxim Levitsky
2019-08-22 11:35 ` Daniel P. Berrangé
2019-08-25 17:10   ` Maxim Levitsky

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1cedbbf007f4447f9424d09848c6486b0341b43d.camel@redhat.com \
    --to=mlevitsk@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=fam@euphon.net \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox