On Sun, Nov 20, 2022 at 11:28 AM Or Ozeri <o...@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 <o...@il.ibm.com>
> ---
> block/rbd.c | 161 ++++++++++++++++++++++++++++++++++++++++++-
> qapi/block-core.json | 17 ++++-
> 2 files changed, 175 insertions(+), 3 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 7feae45e82..157922e23a 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,
> @@ -537,6 +547,136 @@ 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_luks1_format_options_t* luks_opts;
> + rbd_encryption_luks2_format_options_t* luks2_opts;
> + rbd_encryption_luks_format_options_t* luks_any_opts;
Hi Or,
Stick to the pointer alignment style used in this file:
rbd_encryption_luks1_format_options_t *luks_opts;
rbd_encryption_luks2_format_options_t *luks2_opts;
rbd_encryption_luks_format_options_t *luks_any_opts;
> +
> + /* count encryption options */
> + for (curr_encrypt = encrypt; curr_encrypt->has_parent;
I think this needs to be rebased on top of 54fde4ff0621 ("qapi block:
Elide redundant has_FOO in generated C"). has_parent is probably not
a thing anymore.
> + 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) {
> + switch (curr_encrypt->format) {
> + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: {
> + specs[i].format = RBD_ENCRYPTION_FORMAT_LUKS1;
> + specs[i].opts_size =
> + sizeof(rbd_encryption_luks1_format_options_t);
> +
> + luks_opts = g_new0(rbd_encryption_luks1_format_options_t, 1);
> + specs[i].opts = luks_opts;
I would move opts_size assignment here and avoid repeating the type (and
similar for LUKS2 and LUKS cases):
specs[i].opts_size = sizeof(*luks_opts);
> +
> + r = qemu_rbd_convert_luks_options(
> + qapi_RbdEncryptionOptionsLUKS_base(
> + &curr_encrypt->u.luks),
> + &luks_opts->passphrase,
> + &luks_opts->passphrase_size,
> + errp);
> + break;
> + }
> +
No need to leave a blank line between case statements.
> + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: {
> + specs[i].format = RBD_ENCRYPTION_FORMAT_LUKS2;
> + specs[i].opts_size =
> + sizeof(rbd_encryption_luks2_format_options_t);
> +
> + luks2_opts = g_new0(rbd_encryption_luks2_format_options_t,
> 1);
> + specs[i].opts = luks2_opts;
> +
> + r = qemu_rbd_convert_luks_options(
> + qapi_RbdEncryptionOptionsLUKS2_base(
> + &curr_encrypt->u.luks2),
> + &luks2_opts->passphrase,
> + &luks2_opts->passphrase_size,
> + errp);
> + break;
> + }
> +
> + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ANY: {
> + 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),
> + &luks_any_opts->passphrase,
> + &luks_any_opts->passphrase_size,
> + errp);
> + break;
> + }
> +
> + default: {
> + r = -ENOTSUP;
> + error_setg_errno(
> + errp, -r, "unknown image encryption format: %u",
> + curr_encrypt->format);
> + }
> + }
> +
> + 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) {
> + if (!specs[i].opts) {
> + break;
> + }
> +
> + switch (specs[i].format) {
> + case RBD_ENCRYPTION_FORMAT_LUKS1: {
> + luks_opts = specs[i].opts;
> + g_free((void*)luks_opts->passphrase);
Pointer alignment style:
g_free((void *)luks_opts->passphrase);
> + break;
> + }
> +
No need to leave a blank line between case statements.
Thanks,
Ilya
