diff mbox series

[RESEND,v3,06/10] block: Support detached LUKS header creation using blockdev-create

Message ID 20ab47b728492cedb7ea671239f0397a141c3f5a.1703482349.git.yong.huang@smartx.com (mailing list archive)
State New, archived
Headers show
Series Support generic Luks encryption | expand

Commit Message

Yong Huang Dec. 25, 2023, 5:45 a.m. UTC 
The LUKS disk with detached header consists of a separate LUKS
header and payload. This LUKS disk type should be formatted
as follows:

1. add the secret to lock/unlock the cipher stored in the
   detached LUKS header
$ virsh qemu-monitor-command vm '{"execute":"object-add",
> "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'

2. create a header img with 0 size
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job0", "options":{"driver":"file",
> "filename":"/path/to/detached_luks_header.img", "size":0 }}}'

3. add protocol blockdev node for header
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments": {"driver":"file", "filename":
> "/path/to/detached_luks_header.img", "node-name":
> "detached-luks-header-storage"}}'

4. create a payload img with 0 size
$ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> "arguments":{"job-id":"job1", "options":{"driver":"file",
> "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'

5. add protocol blockdev node for payload
$ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> "arguments": {"driver":"file", "filename":
> "/path/to/detached_luks_payload_raw.img", "node-name":
> "luks-payload-raw-storage"}}'

6. do the formatting with 128M size
$ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
---
 block/crypto.c      | 109 ++++++++++++++++++++++++++++++++++++++++----
 crypto/block-luks.c |   6 ++-
 crypto/block.c      |   1 +
 3 files changed, 106 insertions(+), 10 deletions(-)

Comments

Daniel P. Berrangé Jan. 4, 2024, 2:47 p.m. UTC | #1 
On Mon, Dec 25, 2023 at 01:45:08PM +0800, Hyman Huang wrote:
> The LUKS disk with detached header consists of a separate LUKS
> header and payload. This LUKS disk type should be formatted
> as follows:
> 
> 1. add the secret to lock/unlock the cipher stored in the
>    detached LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > "arguments":{"qom-type": "secret", "id": "sec0", "data": "foo"}}'
> 
> 2. create a header img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job0", "options":{"driver":"file",
> > "filename":"/path/to/detached_luks_header.img", "size":0 }}}'
> 
> 3. add protocol blockdev node for header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments": {"driver":"file", "filename":
> > "/path/to/detached_luks_header.img", "node-name":
> > "detached-luks-header-storage"}}'
> 
> 4. create a payload img with 0 size
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job1", "options":{"driver":"file",
> > "filename":"/path/to/detached_luks_payload_raw.img", "size":0}}}'
> 
> 5. add protocol blockdev node for payload
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments": {"driver":"file", "filename":
> > "/path/to/detached_luks_payload_raw.img", "node-name":
> > "luks-payload-raw-storage"}}'
> 
> 6. do the formatting with 128M size
> $ virsh qemu-monitor-command c81_node1 '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job2", "options":{"driver":"luks", "header":
> > "detached-luks-header-storage", "file":"luks-payload-raw-storage",
> > "size":134217728, "preallocation":"full", "key-secret":"sec0" }}}'
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  block/crypto.c      | 109 ++++++++++++++++++++++++++++++++++++++++----
>  crypto/block-luks.c |   6 ++-
>  crypto/block.c      |   1 +
>  3 files changed, 106 insertions(+), 10 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 78fbe79c95..76cc8bda49 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -160,6 +160,48 @@ error:
>      return ret;
>  }
>  
> +static int coroutine_fn GRAPH_UNLOCKED
> +block_crypto_co_format_luks_payload(BlockdevCreateOptionsLUKS *luks_opts,
> +                                    Error **errp)
> +{
> +    BlockDriverState *bs = NULL;
> +    BlockBackend *blk = NULL;
> +    Error *local_error = NULL;
> +    int ret;
> +
> +    if (luks_opts->size > INT64_MAX) {
> +        return -EFBIG;
> +    }
> +
> +    bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
> +    if (bs == NULL) {
> +        return -EIO;
> +    }
> +
> +    blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
> +                             BLK_PERM_ALL, errp);
> +    if (!blk) {
> +        ret = -EPERM;
> +        goto fail;
> +    }
> +
> +    ret = blk_truncate(blk, luks_opts->size, true,
> +                       luks_opts->preallocation, 0, &local_error);
> +    if (ret < 0) {
> +        if (ret == -EFBIG) {
> +            /* Replace the error message with a better one */
> +            error_free(local_error);
> +            error_setg(errp, "The requested file size is too large");
> +        }
> +        goto fail;
> +    }
> +
> +    ret = 0;
> +
> +fail:
> +    bdrv_co_unref(bs);
> +    return ret;
> +}
>  
>  static QemuOptsList block_crypto_runtime_opts_luks = {
>      .name = "crypto",
> @@ -651,6 +693,7 @@ static int coroutine_fn GRAPH_UNLOCKED
>  block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>  {
>      BlockdevCreateOptionsLUKS *luks_opts;
> +    BlockDriverState *hdr_bs = NULL;
>      BlockDriverState *bs = NULL;
>      QCryptoBlockCreateOptions create_opts;
>      PreallocMode preallocation = PREALLOC_MODE_OFF;
> @@ -659,8 +702,22 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>      assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
>      luks_opts = &create_options->u.luks;
>  
> -    if (luks_opts->file == NULL) {
> -        error_setg(errp, "Formatting LUKS disk requires parameter 'file'");
> +    if (luks_opts->header == NULL && luks_opts->file == NULL) {
> +        error_setg(errp, "Either the parameter 'header' or 'file' should "

s/should/must/

> +                   "be specified");
> +        return -EINVAL;
> +    }
> +
> +    if (luks_opts->detached_header && luks_opts->header == NULL) {
> +        error_setg(errp, "Formatting a detached LUKS disk requries "

typo   s/requries/requires/

> +                   "'header' to be specified");
> +        return -EINVAL;
> +    }
> +
> +    if ((luks_opts->preallocation != PREALLOC_MODE_OFF) &&
> +        (luks_opts->file == NULL)) {
> +        error_setg(errp, "Parameter 'preallocation' requries 'file' to be "

typo   s/requries/requires/

> +                   "specified for formatting LUKS disk");
>          return -EINVAL;
>      }
>  
> @@ -673,7 +730,40 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>          preallocation = luks_opts->preallocation;
>      }
>  
> -    if (luks_opts->file) {
> +    if (luks_opts->header) {
> +        hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
> +        if (hdr_bs == NULL) {
> +            return -EIO;
> +        }
> +
> +        /*
> +         * If blockdev reference of header is specified,
> +         * detached_header default to true
> +         */
> +        create_opts.u.luks.detached_header = true;
> +
> +        /* Format the LUKS header node */
> +        ret = block_crypto_co_create_generic(hdr_bs, 0, &create_opts,
> +                                             PREALLOC_MODE_OFF, errp);
> +        if (ret < 0) {
> +            goto hdr_bs_failed;
> +        }
> +
> +        /* Format the LUKS payload node */
> +        if (luks_opts->file) {
> +            ret = block_crypto_co_format_luks_payload(luks_opts, errp);
> +            if (ret < 0) {
> +                goto hdr_bs_failed;
> +            }
> +        }
> +
> +        ret = 0;
> +
> +hdr_bs_failed:

I'd suggest we just make the existing 'fail:' label
cope with unref'ing  hdr_bs if it is non-NULL,
rather than having multiple goto label choices.

> +        bdrv_co_unref(hdr_bs);
> +        return ret;
> +    } else if (luks_opts->file) {
> +        /* None detached LUKS header path */
>          bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
>          if (bs == NULL) {
>              return -EIO;
> @@ -682,14 +772,15 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>          ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
>                                               preallocation, errp);
>          if (ret < 0) {
> -            goto fail;
> +            goto bs_failed;
>          }

> -    }
>  
> -    ret = 0;
> -fail:
> -    bdrv_co_unref(bs);
> -    return ret;
> +        ret = 0;
> +
> +bs_failed:
> +        bdrv_co_unref(bs);
> +        return ret;
> +    }
>  }
>  
>  static int coroutine_fn GRAPH_UNLOCKED
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 48443ffcae..474c7aee2e 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -1561,8 +1561,12 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>      block->payload_offset =
>          qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
>  
> +    block->detached_header_size =
> +        (header_sectors + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS *
> +         split_key_sectors) * block->sector_size;

Storing this in 'detached_header_size' struct field looks redundant,
as the onlY place that reads it is the next line. Remove the struct
field and use a local variable.

> +
>      /* Reserve header space to match payload offset */
> -    initfunc(block, block->payload_offset, opaque, &local_err);
> +    initfunc(block, block->detached_header_size, opaque, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          goto error;
> diff --git a/crypto/block.c b/crypto/block.c
> index 7bb4b74a37..ea493f056e 100644
> --- a/crypto/block.c
> +++ b/crypto/block.c
> @@ -102,6 +102,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
>      }
>  
>      block->driver = qcrypto_block_drivers[options->format];
> +    block->detached_header = options->u.luks.detached_header;

You cannot access a 'luks' field here, as this can be used for
non-LUKS formats. In an earlier patch I suggested using a enum
flag to indicate detached header when opening a device. that
will work for creating it too.

>  
>      if (block->driver->create(block, options, optprefix, initfunc,
>                                writefunc, opaque, errp) < 0) {
> -- 
> 2.39.1
> 

With regards,
Daniel
Markus Armbruster Jan. 11, 2024, 2:05 p.m. UTC | #2 
Fails to compile for me:

../block/crypto.c: In function ‘block_crypto_co_create_luks’:
../block/crypto.c:784:1: error: control reaches end of non-void function [-Werror=return-type]
  784 | }
      | ^
Yong Huang Jan. 11, 2024, 3:52 p.m. UTC | #3 
On Thu, Jan 11, 2024 at 10:05 PM Markus Armbruster <armbru@redhat.com>
wrote:

> Fails to compile for me:
>
> ../block/crypto.c: In function ‘block_crypto_co_create_luks’:
> ../block/crypto.c:784:1: error: control reaches end of non-void function
> [-Werror=return-type]
>   784 | }
>       | ^
>
>
Ok, I'll check it out and fix it in the next version.

By the way, I'm stuck with some other work, so version 4 might not be
finished for
a few weeks, but I'll get to it as soon as I can.

Thanks,
Yong
diff mbox series

Patch

diff --git a/block/crypto.c b/block/crypto.c
index 78fbe79c95..76cc8bda49 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -160,6 +160,48 @@  error:
     return ret;
 }
 
+static int coroutine_fn GRAPH_UNLOCKED
+block_crypto_co_format_luks_payload(BlockdevCreateOptionsLUKS *luks_opts,
+                                    Error **errp)
+{
+    BlockDriverState *bs = NULL;
+    BlockBackend *blk = NULL;
+    Error *local_error = NULL;
+    int ret;
+
+    if (luks_opts->size > INT64_MAX) {
+        return -EFBIG;
+    }
+
+    bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
+    if (bs == NULL) {
+        return -EIO;
+    }
+
+    blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
+                             BLK_PERM_ALL, errp);
+    if (!blk) {
+        ret = -EPERM;
+        goto fail;
+    }
+
+    ret = blk_truncate(blk, luks_opts->size, true,
+                       luks_opts->preallocation, 0, &local_error);
+    if (ret < 0) {
+        if (ret == -EFBIG) {
+            /* Replace the error message with a better one */
+            error_free(local_error);
+            error_setg(errp, "The requested file size is too large");
+        }
+        goto fail;
+    }
+
+    ret = 0;
+
+fail:
+    bdrv_co_unref(bs);
+    return ret;
+}
 
 static QemuOptsList block_crypto_runtime_opts_luks = {
     .name = "crypto",
@@ -651,6 +693,7 @@  static int coroutine_fn GRAPH_UNLOCKED
 block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
 {
     BlockdevCreateOptionsLUKS *luks_opts;
+    BlockDriverState *hdr_bs = NULL;
     BlockDriverState *bs = NULL;
     QCryptoBlockCreateOptions create_opts;
     PreallocMode preallocation = PREALLOC_MODE_OFF;
@@ -659,8 +702,22 @@  block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
     assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
     luks_opts = &create_options->u.luks;
 
-    if (luks_opts->file == NULL) {
-        error_setg(errp, "Formatting LUKS disk requires parameter 'file'");
+    if (luks_opts->header == NULL && luks_opts->file == NULL) {
+        error_setg(errp, "Either the parameter 'header' or 'file' should "
+                   "be specified");
+        return -EINVAL;
+    }
+
+    if (luks_opts->detached_header && luks_opts->header == NULL) {
+        error_setg(errp, "Formatting a detached LUKS disk requries "
+                   "'header' to be specified");
+        return -EINVAL;
+    }
+
+    if ((luks_opts->preallocation != PREALLOC_MODE_OFF) &&
+        (luks_opts->file == NULL)) {
+        error_setg(errp, "Parameter 'preallocation' requries 'file' to be "
+                   "specified for formatting LUKS disk");
         return -EINVAL;
     }
 
@@ -673,7 +730,40 @@  block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
         preallocation = luks_opts->preallocation;
     }
 
-    if (luks_opts->file) {
+    if (luks_opts->header) {
+        hdr_bs = bdrv_co_open_blockdev_ref(luks_opts->header, errp);
+        if (hdr_bs == NULL) {
+            return -EIO;
+        }
+
+        /*
+         * If blockdev reference of header is specified,
+         * detached_header default to true
+         */
+        create_opts.u.luks.detached_header = true;
+
+        /* Format the LUKS header node */
+        ret = block_crypto_co_create_generic(hdr_bs, 0, &create_opts,
+                                             PREALLOC_MODE_OFF, errp);
+        if (ret < 0) {
+            goto hdr_bs_failed;
+        }
+
+        /* Format the LUKS payload node */
+        if (luks_opts->file) {
+            ret = block_crypto_co_format_luks_payload(luks_opts, errp);
+            if (ret < 0) {
+                goto hdr_bs_failed;
+            }
+        }
+
+        ret = 0;
+
+hdr_bs_failed:
+        bdrv_co_unref(hdr_bs);
+        return ret;
+    } else if (luks_opts->file) {
+        /* None detached LUKS header path */
         bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
         if (bs == NULL) {
             return -EIO;
@@ -682,14 +772,15 @@  block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
         ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
                                              preallocation, errp);
         if (ret < 0) {
-            goto fail;
+            goto bs_failed;
         }
-    }
 
-    ret = 0;
-fail:
-    bdrv_co_unref(bs);
-    return ret;
+        ret = 0;
+
+bs_failed:
+        bdrv_co_unref(bs);
+        return ret;
+    }
 }
 
 static int coroutine_fn GRAPH_UNLOCKED
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 48443ffcae..474c7aee2e 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1561,8 +1561,12 @@  qcrypto_block_luks_create(QCryptoBlock *block,
     block->payload_offset =
         qcrypto_block_luks_payload_offset(luks->header.payload_offset_sector);
 
+    block->detached_header_size =
+        (header_sectors + QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS *
+         split_key_sectors) * block->sector_size;
+
     /* Reserve header space to match payload offset */
-    initfunc(block, block->payload_offset, opaque, &local_err);
+    initfunc(block, block->detached_header_size, opaque, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto error;
diff --git a/crypto/block.c b/crypto/block.c
index 7bb4b74a37..ea493f056e 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -102,6 +102,7 @@  QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
     }
 
     block->driver = qcrypto_block_drivers[options->format];
+    block->detached_header = options->u.luks.detached_header;
 
     if (block->driver->create(block, options, optprefix, initfunc,
                               writefunc, opaque, errp) < 0) {