date:20210620

RE: [PATCH] block/rbd: Add support for rbd image encryption

2021-06-20 Thread Or Ozeri

Thanks for your review!Regarding the last point:I currently don't have any plans for adding new luks2-only options, but they do exist. See in cryptsetup, for example the sector size which is configurable in luks2 only.In the librbd API we followed the same spirit and split luks1 and luks2 option structs.Same goes for passphrase (or "key-secret", as it called in qemu). In cryptsetup this is sort-of optional (there are other ways to authenticate othar than a passphrase, like a keyfile or a token).-Ilya Dryomov  wrote: -To: Or Ozeri From: Ilya Dryomov Date: 06/19/2021 10:44PMCc: qemu-de...@nongnu.org, qemu-block@nongnu.org, kw...@redhat.com, Mykola Golub , Danny Harnik , berra...@redhat.comSubject: [EXTERNAL] Re: [PATCH] block/rbd: Add support for rbd image encryptionOn Thu, Jun 17, 2021 at 6:05 PM Or Ozeri  wrote:>> Starting from ceph Pacific, RBD has built-in support for image-level encryption.> Currently supported formats are LUKS version 1 and 2.>> There are 2 new relevant librbd APIs for controlling encryption, both expect an> open image context:>> rbd_encryption_format: formats an image (i.e. writes the LUKS header)> rbd_encryption_load: loads encryptor/decryptor to the image IO stack>> This commit extends the qemu rbd driver API to support the above.>> Signed-off-by: Or Ozeri > --->  block/raw-format.c   |   7 +>  block/rbd.c          | 371 ++->  qapi/block-core.json | 110 ->  3 files changed, 482 insertions(+), 6 deletions(-)>> diff --git a/block/raw-format.c b/block/raw-format.c> index 7717578ed6..f6e70e2356 100644> --- a/block/raw-format.c> +++ b/block/raw-format.c> @@ -369,6 +369,12 @@ static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)>      return bdrv_get_info(bs->file->bs, bdi);>  }>> +static ImageInfoSpecific *raw_get_specific_info(BlockDriverState *bs,> +                                                Error **errp)> +{> +    return bdrv_get_specific_info(bs->file->bs, errp);> +}> +>  static void raw_refresh_limits(BlockDriverState *bs, Error **errp)>  {>      if (bs->probed) {> @@ -603,6 +609,7 @@ BlockDriver bdrv_raw = {>      .has_variable_length  = true,>      .bdrv_measure         = _measure,>      .bdrv_get_info        = _get_info,> +    .bdrv_get_specific_info = _get_specific_info,Hi Or,This feels a bit contentious to me.AFAIU ImageInfoSpecific is for format-specfic information.  "raw"is a format and passing the request down the stack this way resultsin a somewhat confusing output such as    $ qemu-img info rbd:foo/bar    image: json:{"driver": "raw", "file": {"pool": "foo", "image":"bar", "driver": "rbd", "namespace": ""}}    file format: raw    ...    Format specific information:       I think this should be broken out into its own patch and get separateacks.>      .bdrv_refresh_limits  = _refresh_limits,>      .bdrv_probe_blocksizes = _probe_blocksizes,>      .bdrv_probe_geometry  = _probe_geometry,> diff --git a/block/rbd.c b/block/rbd.c> index f098a89c7b..183b17cd84 100644> --- a/block/rbd.c> +++ b/block/rbd.c> @@ -73,6 +73,18 @@>  #define LIBRBD_USE_IOVEC 0>  #endif>> +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8> +> +static const char rbd_luks_header_verification[> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {> +    'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1> +};> +> +static const char rbd_luks2_header_verification[> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {> +    'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2> +};> +>  typedef enum {>      RBD_AIO_READ,>      RBD_AIO_WRITE,> @@ -341,6 +353,206 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)>      }>  }>> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION> +static int qemu_rbd_convert_luks_options(> +        RbdEncryptionOptionsLUKSBase *luks_opts,> +        char **passphrase,> +        Error **errp)> +{> +    int r = 0;> +> +    if (!luks_opts->has_key_secret) {> +        r = -EINVAL;> +        error_setg_errno(errp, -r, "missing encrypt.key-secret");> +        return r;> +    }Why is key-secret optional?> +> +    *passphrase = qcrypto_secret_lookup_as_utf8(luks_opts->key_secret, errp);> +    if (!*passphrase) {> +        return -EIO;> +    }> +> +    return 0;> +}> +> +static int qemu_rbd_convert_luks_create_options(> +        RbdEncryptionCreateOptionsLUKSBase *luks_opts,> +        rbd_encryption_algorithm_t *alg,> +        char **passphrase,> +        Error **errp)> +{> +    int r = 0;> +> +    r = qemu_rbd_convert_luks_options(> +            qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),> +            passphrase, errp);> +    if (r < 0) {> +        return r;> +    }> +> +    if (luks_opts->has_cipher_alg) {> +        switch (luks_opts->cipher_alg) {> +            case QCRYPTO_CIPHER_ALG_AES_128: {> +                *alg = RBD_ENCRYPTION_ALGORITHM_AES128;> +                break;> +

Re: [PATCH] block/rbd: Add support for rbd image encryption

2021-06-20 Thread Ilya Dryomov

On Sat, Jun 19, 2021 at 9:44 PM Ilya Dryomov  wrote:
>
> On Thu, Jun 17, 2021 at 6:05 PM Or Ozeri  wrote:
> >
> > Starting from ceph Pacific, RBD has built-in support for image-level 
> > encryption.
> > Currently supported formats are LUKS version 1 and 2.
> >
> > There are 2 new relevant librbd APIs for controlling encryption, both 
> > expect an
> > open image context:
> >
> > rbd_encryption_format: formats an image (i.e. writes the LUKS header)
> > rbd_encryption_load: loads encryptor/decryptor to the image IO stack
> >
> > This commit extends the qemu rbd driver API to support the above.
> >
> > Signed-off-by: Or Ozeri 
> > ---
> >  block/raw-format.c   |   7 +
> >  block/rbd.c  | 371 ++-
> >  qapi/block-core.json | 110 -
> >  3 files changed, 482 insertions(+), 6 deletions(-)
> >
> > diff --git a/block/raw-format.c b/block/raw-format.c
> > index 7717578ed6..f6e70e2356 100644
> > --- a/block/raw-format.c
> > +++ b/block/raw-format.c
> > @@ -369,6 +369,12 @@ static int raw_get_info(BlockDriverState *bs, 
> > BlockDriverInfo *bdi)
> >  return bdrv_get_info(bs->file->bs, bdi);
> >  }
> >
> > +static ImageInfoSpecific *raw_get_specific_info(BlockDriverState *bs,
> > +Error **errp)
> > +{
> > +return bdrv_get_specific_info(bs->file->bs, errp);
> > +}
> > +
> >  static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
> >  {
> >  if (bs->probed) {
> > @@ -603,6 +609,7 @@ BlockDriver bdrv_raw = {
> >  .has_variable_length  = true,
> >  .bdrv_measure = _measure,
> >  .bdrv_get_info= _get_info,
> > +.bdrv_get_specific_info = _get_specific_info,
>
> Hi Or,
>
> This feels a bit contentious to me.
>
> AFAIU ImageInfoSpecific is for format-specfic information.  "raw"
> is a format and passing the request down the stack this way results
> in a somewhat confusing output such as
>
> $ qemu-img info rbd:foo/bar
> image: json:{"driver": "raw", "file": {"pool": "foo", "image":
> "bar", "driver": "rbd", "namespace": ""}}
> file format: raw
> ...
> Format specific information:
>
>
> I think this should be broken out into its own patch and get separate
> acks.
>
> >  .bdrv_refresh_limits  = _refresh_limits,
> >  .bdrv_probe_blocksizes = _probe_blocksizes,
> >  .bdrv_probe_geometry  = _probe_geometry,
> > diff --git a/block/rbd.c b/block/rbd.c
> > index f098a89c7b..183b17cd84 100644
> > --- a/block/rbd.c
> > +++ b/block/rbd.c
> > @@ -73,6 +73,18 @@
> >  #define LIBRBD_USE_IOVEC 0
> >  #endif
> >
> > +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8
> > +
> > +static const char rbd_luks_header_verification[
> > +RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> > +'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1
> > +};
> > +
> > +static const char rbd_luks2_header_verification[
> > +RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> > +'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
> > +};
> > +
> >  typedef enum {
> >  RBD_AIO_READ,
> >  RBD_AIO_WRITE,
> > @@ -341,6 +353,206 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t 
> > offs)
> >  }
> >  }
> >
> > +#ifdef LIBRBD_SUPPORTS_ENCRYPTION
> > +static int qemu_rbd_convert_luks_options(
> > +RbdEncryptionOptionsLUKSBase *luks_opts,
> > +char **passphrase,
> > +Error **errp)
> > +{
> > +int r = 0;
> > +
> > +if (!luks_opts->has_key_secret) {
> > +r = -EINVAL;
> > +error_setg_errno(errp, -r, "missing encrypt.key-secret");
> > +return r;
> > +}
>
> Why is key-secret optional?
>
> > +
> > +*passphrase = qcrypto_secret_lookup_as_utf8(luks_opts->key_secret, 
> > errp);
> > +if (!*passphrase) {
> > +return -EIO;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int qemu_rbd_convert_luks_create_options(
> > +RbdEncryptionCreateOptionsLUKSBase *luks_opts,
> > +rbd_encryption_algorithm_t *alg,
> > +char **passphrase,
> > +Error **errp)
> > +{
> > +int r = 0;
> > +
> > +r = qemu_rbd_convert_luks_options(
> > +qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),
> > +passphrase, errp);
> > +if (r < 0) {
> > +return r;
> > +}
> > +
> > +if (luks_opts->has_cipher_alg) {
> > +switch (luks_opts->cipher_alg) {
> > +case QCRYPTO_CIPHER_ALG_AES_128: {
> > +*alg = RBD_ENCRYPTION_ALGORITHM_AES128;
> > +break;
> > +}
> > +case QCRYPTO_CIPHER_ALG_AES_256: {
> > +*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
> > +break;
> > +}
> > +default: {
> > +r = -ENOTSUP;
> > +error_setg_errno(errp, -r, "unknown encryption algorithm: 
> > %u",
> > + luks_opts->cipher_alg);
> > +return r;
> > +}
> > +

RE: [PATCH] block/rbd: Add support for rbd image encryption

Re: [PATCH] block/rbd: Add support for rbd image encryption

2 matches

Site Navigation

Mail list logo

Footer information