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

[Ilya Dryomov]

On Tue, Nov 15, 2022 at 1:25 PM Or Ozeri <oro@il.ibm.com> wrote:
>
> Starting from ceph Reef, RBD has built-in support for layered encryption,
> where each ancestor image (in a cloned image setting) can be possibly
> encrypted using a unique passphrase.
>
> A new function, rbd_encryption_load2, was added to librbd API.
> This new function supports an array of passphrases (via "spec" structs).
>
> This commit extends the qemu rbd driver API to use this new librbd API,
> in order to support this new layered encryption feature.
>
> Signed-off-by: Or Ozeri <oro@il.ibm.com>
> ---
> v3: further nit fixes suggested by @idryomov
> v2: nit fixes suggested by @idryomov
> ---
>  block/rbd.c          | 119 ++++++++++++++++++++++++++++++++++++++++++-
>  qapi/block-core.json |  35 +++++++++++--
>  2 files changed, 150 insertions(+), 4 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index f826410f40..ce017c29b5 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -71,6 +71,16 @@ static const char rbd_luks2_header_verification[
>      'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
>  };
>
> +static const char rbd_layered_luks_header_verification[
> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +    'R', 'B', 'D', 'L', 0xBA, 0xBE, 0, 1
> +};
> +
> +static const char rbd_layered_luks2_header_verification[
> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +    'R', 'B', 'D', 'L', 0xBA, 0xBE, 0, 2
> +};
> +
>  typedef enum {
>      RBD_AIO_READ,
>      RBD_AIO_WRITE,
> @@ -470,6 +480,9 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>      size_t passphrase_len;
>      rbd_encryption_luks1_format_options_t luks_opts;
>      rbd_encryption_luks2_format_options_t luks2_opts;
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
> +    rbd_encryption_luks_format_options_t luks_any_opts;
> +#endif
>      rbd_encryption_format_t format;
>      rbd_encryption_options_t opts;
>      size_t opts_size;
> @@ -505,6 +518,23 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>              luks2_opts.passphrase_size = passphrase_len;
>              break;
>          }
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
> +        case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ANY: {
> +            memset(&luks_any_opts, 0, sizeof(luks_any_opts));
> +            format = RBD_ENCRYPTION_FORMAT_LUKS;
> +            opts = &luks_any_opts;
> +            opts_size = sizeof(luks_any_opts);
> +            r = qemu_rbd_convert_luks_options(
> +                    
> qapi_RbdEncryptionOptionsLUKSAny_base(&encrypt->u.luks_any),
> +                    &passphrase, &passphrase_len, errp);
> +            if (r < 0) {
> +                return r;
> +            }
> +            luks_any_opts.passphrase = passphrase;
> +            luks_any_opts.passphrase_size = passphrase_len;
> +            break;
> +        }
> +#endif
>          default: {
>              r = -ENOTSUP;
>              error_setg_errno(
> @@ -522,6 +552,74 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>
>      return 0;
>  }
> +
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
> +static int qemu_rbd_encryption_load2(rbd_image_t image,
> +                                     RbdEncryptionOptions *encrypt,
> +                                     Error **errp)
> +{
> +    int r = 0;
> +    int encrypt_count = 1;
> +    int i;
> +    RbdEncryptionOptions *curr_encrypt;
> +    rbd_encryption_spec_t *specs;
> +    rbd_encryption_luks_format_options_t* luks_any_opts;
> +
> +    /* count encryption options */
> +    for (curr_encrypt = encrypt; curr_encrypt->has_parent;
> +         curr_encrypt = curr_encrypt->parent) {
> +        ++encrypt_count;
> +    }
> +
> +    specs = g_new0(rbd_encryption_spec_t, encrypt_count);
> +
> +    curr_encrypt = encrypt;
> +    for (i = 0; i < encrypt_count; ++i) {
> +        if (curr_encrypt->format != RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ANY) {
> +            r = -ENOTSUP;
> +            error_setg_errno(
> +                    errp, -r, "unknown image encryption format: %u",
> +                    curr_encrypt->format);
> +            goto exit;
> +        }
> +
> +        specs[i].format = RBD_ENCRYPTION_FORMAT_LUKS;
> +        specs[i].opts_size = sizeof(rbd_encryption_luks_format_options_t);
> +
> +        luks_any_opts = g_new0(rbd_encryption_luks_format_options_t, 1);
> +        specs[i].opts = luks_any_opts;
> +
> +        r = qemu_rbd_convert_luks_options(
> +                qapi_RbdEncryptionOptionsLUKSAny_base(
> +                        &curr_encrypt->u.luks_any),
> +                (char**)&luks_any_opts->passphrase,

Nit: I would change qemu_rbd_convert_luks_options() to take
const char **passphrase and eliminate this cast.  It's a trivial
fixup so it can be folded into this patch with no explanation.

> +                &luks_any_opts->passphrase_size,
> +                errp);
> +        if (r < 0) {
> +            goto exit;
> +        }
> +
> +        curr_encrypt = curr_encrypt->parent;
> +    }
> +
> +    r = rbd_encryption_load2(image, specs, encrypt_count);
> +    if (r < 0) {
> +        error_setg_errno(errp, -r, "layered encryption load fail");
> +        goto exit;
> +    }
> +
> +exit:
> +    for (i = 0; i < encrypt_count; ++i) {
> +        luks_any_opts = specs[i].opts;
> +        if (luks_any_opts) {
> +            g_free((char*)luks_any_opts->passphrase);

Nit: when resorting to a cast, cast to the actual expected type.
In case of free(), that's void *.

free() should have been specified to take const void * but that
ship has sailed.  Too bad GLib didn't fix this for g_free()...

Thanks,

                Ilya