diff mbox series

[v3,1/4] luks: extract qcrypto_block_calculate_payload_offset()

Message ID 20200211160318.453650-2-stefanha@redhat.com (mailing list archive)
State New, archived
Headers show
Series luks: add qemu-img measure support | expand

Commit Message

Stefan Hajnoczi Feb. 11, 2020, 4:03 p.m. UTC
The qcow2 .bdrv_measure() code calculates the crypto payload offset.
This logic really belongs in crypto/block.c where it can be reused by
other image formats.

The "luks" block driver will need this same logic in order to implement
.bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
function now.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c          | 74 +++++++++++-------------------------------
 crypto/block.c         | 40 +++++++++++++++++++++++
 include/crypto/block.h | 22 +++++++++++++
 3 files changed, 81 insertions(+), 55 deletions(-)

Comments

Max Reitz Feb. 19, 2020, 1:03 p.m. UTC | #1
On 11.02.20 17:03, Stefan Hajnoczi wrote:
> The qcow2 .bdrv_measure() code calculates the crypto payload offset.
> This logic really belongs in crypto/block.c where it can be reused by
> other image formats.
> 
> The "luks" block driver will need this same logic in order to implement
> .bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
> function now.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/qcow2.c          | 74 +++++++++++-------------------------------
>  crypto/block.c         | 40 +++++++++++++++++++++++
>  include/crypto/block.h | 22 +++++++++++++
>  3 files changed, 81 insertions(+), 55 deletions(-)

[...]

> diff --git a/crypto/block.c b/crypto/block.c
> index 325752871c..a9e1b8cc36 100644
> --- a/crypto/block.c
> +++ b/crypto/block.c
> @@ -115,6 +115,46 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,

[...]

> +bool
> +qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
> +                                       const char *optprefix,
> +                                       size_t *len,
> +                                       Error **errp)
> +{
> +    QCryptoBlock *crypto;
> +    bool ok;
> +
> +    /* Fake LUKS creation in order to determine the payload size */
> +    crypto = qcrypto_block_create(create_opts, optprefix,
> +                                  qcrypto_block_headerlen_hdr_init_func,
> +                                  qcrypto_block_headerlen_hdr_write_func,
> +                                  len, errp);
> +    ok = crypto != NULL;
> +    qcrypto_block_free(crypto);
> +    return ok;

Speaking of g_autoptr...  Would g_autoptr(QCryptoBlock) crypto; suffice
to contract these three lines into “return crypto != NULL;”?

Either way:

Reviewed-by: Max Reitz <mreitz@redhat.com>
Stefan Hajnoczi Feb. 21, 2020, 11:17 a.m. UTC | #2
On Wed, Feb 19, 2020 at 02:03:50PM +0100, Max Reitz wrote:
> On 11.02.20 17:03, Stefan Hajnoczi wrote:
> > The qcow2 .bdrv_measure() code calculates the crypto payload offset.
> > This logic really belongs in crypto/block.c where it can be reused by
> > other image formats.
> > 
> > The "luks" block driver will need this same logic in order to implement
> > .bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
> > function now.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  block/qcow2.c          | 74 +++++++++++-------------------------------
> >  crypto/block.c         | 40 +++++++++++++++++++++++
> >  include/crypto/block.h | 22 +++++++++++++
> >  3 files changed, 81 insertions(+), 55 deletions(-)
> 
> [...]
> 
> > diff --git a/crypto/block.c b/crypto/block.c
> > index 325752871c..a9e1b8cc36 100644
> > --- a/crypto/block.c
> > +++ b/crypto/block.c
> > @@ -115,6 +115,46 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
> 
> [...]
> 
> > +bool
> > +qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
> > +                                       const char *optprefix,
> > +                                       size_t *len,
> > +                                       Error **errp)
> > +{
> > +    QCryptoBlock *crypto;
> > +    bool ok;
> > +
> > +    /* Fake LUKS creation in order to determine the payload size */
> > +    crypto = qcrypto_block_create(create_opts, optprefix,
> > +                                  qcrypto_block_headerlen_hdr_init_func,
> > +                                  qcrypto_block_headerlen_hdr_write_func,
> > +                                  len, errp);
> > +    ok = crypto != NULL;
> > +    qcrypto_block_free(crypto);
> > +    return ok;
> 
> Speaking of g_autoptr...  Would g_autoptr(QCryptoBlock) crypto; suffice
> to contract these three lines into “return crypto != NULL;”?
> 
> Either way:
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Yes, it can be simplified.  Will fix in v3.

Stefan
diff mbox series

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index ef96606f8d..aca2c78fd9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4603,60 +4603,6 @@  static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
     return ret;
 }
 
-static ssize_t qcow2_measure_crypto_hdr_init_func(QCryptoBlock *block,
-        size_t headerlen, void *opaque, Error **errp)
-{
-    size_t *headerlenp = opaque;
-
-    /* Stash away the payload size */
-    *headerlenp = headerlen;
-    return 0;
-}
-
-static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
-        size_t offset, const uint8_t *buf, size_t buflen,
-        void *opaque, Error **errp)
-{
-    /* Discard the bytes, we're not actually writing to an image */
-    return buflen;
-}
-
-/* Determine the number of bytes for the LUKS payload */
-static bool qcow2_measure_luks_headerlen(QemuOpts *opts, size_t *len,
-                                         Error **errp)
-{
-    QDict *opts_qdict;
-    QDict *cryptoopts_qdict;
-    QCryptoBlockCreateOptions *cryptoopts;
-    QCryptoBlock *crypto;
-
-    /* Extract "encrypt." options into a qdict */
-    opts_qdict = qemu_opts_to_qdict(opts, NULL);
-    qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
-    qobject_unref(opts_qdict);
-
-    /* Build QCryptoBlockCreateOptions object from qdict */
-    qdict_put_str(cryptoopts_qdict, "format", "luks");
-    cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);
-    qobject_unref(cryptoopts_qdict);
-    if (!cryptoopts) {
-        return false;
-    }
-
-    /* Fake LUKS creation in order to determine the payload size */
-    crypto = qcrypto_block_create(cryptoopts, "encrypt.",
-                                  qcow2_measure_crypto_hdr_init_func,
-                                  qcow2_measure_crypto_hdr_write_func,
-                                  len, errp);
-    qapi_free_QCryptoBlockCreateOptions(cryptoopts);
-    if (!crypto) {
-        return false;
-    }
-
-    qcrypto_block_free(crypto);
-    return true;
-}
-
 static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
                                        Error **errp)
 {
@@ -4707,9 +4653,27 @@  static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
     g_free(optstr);
 
     if (has_luks) {
+        g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
+        QDict *opts_qdict;
+        QDict *cryptoopts;
         size_t headerlen;
 
-        if (!qcow2_measure_luks_headerlen(opts, &headerlen, &local_err)) {
+        opts_qdict = qemu_opts_to_qdict(opts, NULL);
+        qdict_extract_subqdict(opts_qdict, &cryptoopts, "encrypt.");
+        qobject_unref(opts_qdict);
+
+        qdict_put_str(cryptoopts, "format", "luks");
+
+        create_opts = block_crypto_create_opts_init(cryptoopts, errp);
+        qobject_unref(cryptoopts);
+        if (!create_opts) {
+            goto err;
+        }
+
+        if (!qcrypto_block_calculate_payload_offset(create_opts,
+                                                    "encrypt.",
+                                                    &headerlen,
+                                                    &local_err)) {
             goto err;
         }
 
diff --git a/crypto/block.c b/crypto/block.c
index 325752871c..a9e1b8cc36 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -115,6 +115,46 @@  QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
 }
 
 
+static ssize_t qcrypto_block_headerlen_hdr_init_func(QCryptoBlock *block,
+        size_t headerlen, void *opaque, Error **errp)
+{
+    size_t *headerlenp = opaque;
+
+    /* Stash away the payload size */
+    *headerlenp = headerlen;
+    return 0;
+}
+
+
+static ssize_t qcrypto_block_headerlen_hdr_write_func(QCryptoBlock *block,
+        size_t offset, const uint8_t *buf, size_t buflen,
+        void *opaque, Error **errp)
+{
+    /* Discard the bytes, we're not actually writing to an image */
+    return buflen;
+}
+
+
+bool
+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
+                                       const char *optprefix,
+                                       size_t *len,
+                                       Error **errp)
+{
+    QCryptoBlock *crypto;
+    bool ok;
+
+    /* Fake LUKS creation in order to determine the payload size */
+    crypto = qcrypto_block_create(create_opts, optprefix,
+                                  qcrypto_block_headerlen_hdr_init_func,
+                                  qcrypto_block_headerlen_hdr_write_func,
+                                  len, errp);
+    ok = crypto != NULL;
+    qcrypto_block_free(crypto);
+    return ok;
+}
+
+
 QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
                                          Error **errp)
 {
diff --git a/include/crypto/block.h b/include/crypto/block.h
index d49d2c2da9..c77ccaf9c0 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -145,6 +145,26 @@  QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
                                    Error **errp);
 
 
+/**
+ * qcrypto_block_calculate_payload_offset:
+ * @create_opts: the encryption options
+ * @optprefix: name prefix for options
+ * @len: output for number of header bytes before payload
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Calculate the number of header bytes before the payload in an encrypted
+ * storage volume.  The header is an area before the payload that is reserved
+ * for encryption metadata.
+ *
+ * Returns: true on success, false on error
+ */
+bool
+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
+                                       const char *optprefix,
+                                       size_t *len,
+                                       Error **errp);
+
+
 /**
  * qcrypto_block_get_info:
  * @block: the block encryption object
@@ -269,5 +289,7 @@  uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);
 void qcrypto_block_free(QCryptoBlock *block);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlock, qcrypto_block_free)
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlockCreateOptions,
+                              qapi_free_QCryptoBlockCreateOptions)
 
 #endif /* QCRYPTO_BLOCK_H */