diff mbox series

[v2,08/13] qcrypto-luks: extract store and load header

Message ID 20190826135103.22410-9-mlevitsk@redhat.com (mailing list archive)
State New, archived
Headers show
Series RFC crypto/luks: preparation for encryption key managment | expand

Commit Message

Maxim Levitsky Aug. 26, 2019, 1:50 p.m. UTC 
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 crypto/block-luks.c | 166 +++++++++++++++++++++++++++-----------------
 1 file changed, 102 insertions(+), 64 deletions(-)

Comments

Daniel P. Berrangé Sept. 6, 2019, 1:06 p.m. UTC | #1 
On Mon, Aug 26, 2019 at 04:50:58PM +0300, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  crypto/block-luks.c | 166 +++++++++++++++++++++++++++-----------------
>  1 file changed, 102 insertions(+), 64 deletions(-)
> 
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index cad65ae0aa..b4dc6fc899 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -409,6 +409,105 @@ qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
>      }
>  }
>  
> +/*
> + * Stores the main LUKS header, taking care of endianess
> + */
> +static int
> +qcrypto_block_luks_store_header(QCryptoBlock *block,
> +                                QCryptoBlockWriteFunc writefunc,
> +                                void *opaque,
> +                                Error **errp)
> +{
> +    const QCryptoBlockLUKS *luks = block->opaque;
> +    Error *local_err = NULL;
> +    size_t i;
> +    QCryptoBlockLUKSHeader *hdr_copy;

Initialize to NULL and mark with g_autofree

> +
> +    /* Create a copy of the header */
> +    hdr_copy = g_new0(QCryptoBlockLUKSHeader, 1);
> +    memcpy(hdr_copy, &luks->header, sizeof(QCryptoBlockLUKSHeader));
> +
> +    /*
> +     * Everything on disk uses Big Endian (tm), so flip header fields
> +     * before writing them
> +     */
> +    cpu_to_be16s(&hdr_copy->version);
> +    cpu_to_be32s(&hdr_copy->payload_offset_sector);
> +    cpu_to_be32s(&hdr_copy->master_key_len);
> +    cpu_to_be32s(&hdr_copy->master_key_iterations);
> +
> +    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> +        cpu_to_be32s(&hdr_copy->key_slots[i].active);
> +        cpu_to_be32s(&hdr_copy->key_slots[i].iterations);
> +        cpu_to_be32s(&hdr_copy->key_slots[i].key_offset_sector);
> +        cpu_to_be32s(&hdr_copy->key_slots[i].stripes);
> +    }
> +
> +    /* Write out the partition header and key slot headers */
> +    writefunc(block, 0, (const uint8_t *)hdr_copy, sizeof(*hdr_copy),
> +              opaque, &local_err);
> +
> +    g_free(hdr_copy);

And then this can be removed

> +
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +/*
> + * Loads the main LUKS header,and byteswaps it to native endianess
> + * And run basic sanity checks on it
> + */
> +static int
> +qcrypto_block_luks_load_header(QCryptoBlock *block,
> +                                QCryptoBlockReadFunc readfunc,
> +                                void *opaque,
> +                                Error **errp)
> +{
> +    ssize_t rv;
> +    size_t i;
> +    int ret = 0;
> +    QCryptoBlockLUKS *luks = block->opaque;
> +
> +    /*
> +     * Read the entire LUKS header, minus the key material from
> +     * the underlying device
> +     */
> +
> +    rv = readfunc(block, 0,
> +                  (uint8_t *)&luks->header,
> +                  sizeof(luks->header),
> +                  opaque,
> +                  errp);
> +    if (rv < 0) {
> +        ret = rv;
> +        goto fail;

Nothing happens at the fail: label, so you can just 'return rv'
straightaway IMHO

> +    }
> +
> +    /*
> +     * The header is always stored in big-endian format, so
> +     * convert everything to native
> +     */
> +    be16_to_cpus(&luks->header.version);
> +    be32_to_cpus(&luks->header.payload_offset_sector);
> +    be32_to_cpus(&luks->header.master_key_len);
> +    be32_to_cpus(&luks->header.master_key_iterations);
> +
> +    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> +        be32_to_cpus(&luks->header.key_slots[i].active);
> +        be32_to_cpus(&luks->header.key_slots[i].iterations);
> +        be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
> +        be32_to_cpus(&luks->header.key_slots[i].stripes);
> +    }
> +
> +
> +    return 0;
> +fail:
> +    return ret;
> +}

> -    /* Read the entire LUKS header, minus the key material from
> -     * the underlying device */
> -    rv = readfunc(block, 0,
> -                  (uint8_t *)&luks->header,
> -                  sizeof(luks->header),
> -                  opaque,
> -                  errp);
> -    if (rv < 0) {
> -        ret = rv;
> +    ret = qcrypto_block_luks_load_header(block, readfunc, opaque, errp);
> +    if (ret) {

if (ret < 0)

>          goto fail;
>      }
>  
> -    /* The header is always stored in big-endian format, so
> -     * convert everything to native */
> -    be16_to_cpus(&luks->header.version);
> -    be32_to_cpus(&luks->header.payload_offset_sector);
> -    be32_to_cpus(&luks->header.master_key_len);
> -    be32_to_cpus(&luks->header.master_key_iterations);
> -
> -    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> -        be32_to_cpus(&luks->header.key_slots[i].active);
> -        be32_to_cpus(&luks->header.key_slots[i].iterations);
> -        be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
> -        be32_to_cpus(&luks->header.key_slots[i].stripes);
> -    }
>  
>      if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
>                 QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
> @@ -1235,46 +1312,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>          goto error;
>      }

> -    if (local_err) {
> -        error_propagate(errp, local_err);
> +    if (qcrypto_block_luks_store_header(block,  writefunc, opaque, errp)) {

The comparison should be "< 0"

>          goto error;
>      }

Regards,
Daniel
diff mbox series

Patch

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index cad65ae0aa..b4dc6fc899 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -409,6 +409,105 @@  qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
     }
 }
 
+/*
+ * Stores the main LUKS header, taking care of endianess
+ */
+static int
+qcrypto_block_luks_store_header(QCryptoBlock *block,
+                                QCryptoBlockWriteFunc writefunc,
+                                void *opaque,
+                                Error **errp)
+{
+    const QCryptoBlockLUKS *luks = block->opaque;
+    Error *local_err = NULL;
+    size_t i;
+    QCryptoBlockLUKSHeader *hdr_copy;
+
+    /* Create a copy of the header */
+    hdr_copy = g_new0(QCryptoBlockLUKSHeader, 1);
+    memcpy(hdr_copy, &luks->header, sizeof(QCryptoBlockLUKSHeader));
+
+    /*
+     * Everything on disk uses Big Endian (tm), so flip header fields
+     * before writing them
+     */
+    cpu_to_be16s(&hdr_copy->version);
+    cpu_to_be32s(&hdr_copy->payload_offset_sector);
+    cpu_to_be32s(&hdr_copy->master_key_len);
+    cpu_to_be32s(&hdr_copy->master_key_iterations);
+
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        cpu_to_be32s(&hdr_copy->key_slots[i].active);
+        cpu_to_be32s(&hdr_copy->key_slots[i].iterations);
+        cpu_to_be32s(&hdr_copy->key_slots[i].key_offset_sector);
+        cpu_to_be32s(&hdr_copy->key_slots[i].stripes);
+    }
+
+    /* Write out the partition header and key slot headers */
+    writefunc(block, 0, (const uint8_t *)hdr_copy, sizeof(*hdr_copy),
+              opaque, &local_err);
+
+    g_free(hdr_copy);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return -1;
+    }
+    return 0;
+}
+
+/*
+ * Loads the main LUKS header,and byteswaps it to native endianess
+ * And run basic sanity checks on it
+ */
+static int
+qcrypto_block_luks_load_header(QCryptoBlock *block,
+                                QCryptoBlockReadFunc readfunc,
+                                void *opaque,
+                                Error **errp)
+{
+    ssize_t rv;
+    size_t i;
+    int ret = 0;
+    QCryptoBlockLUKS *luks = block->opaque;
+
+    /*
+     * Read the entire LUKS header, minus the key material from
+     * the underlying device
+     */
+
+    rv = readfunc(block, 0,
+                  (uint8_t *)&luks->header,
+                  sizeof(luks->header),
+                  opaque,
+                  errp);
+    if (rv < 0) {
+        ret = rv;
+        goto fail;
+    }
+
+    /*
+     * The header is always stored in big-endian format, so
+     * convert everything to native
+     */
+    be16_to_cpus(&luks->header.version);
+    be32_to_cpus(&luks->header.payload_offset_sector);
+    be32_to_cpus(&luks->header.master_key_len);
+    be32_to_cpus(&luks->header.master_key_iterations);
+
+    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+        be32_to_cpus(&luks->header.key_slots[i].active);
+        be32_to_cpus(&luks->header.key_slots[i].iterations);
+        be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
+        be32_to_cpus(&luks->header.key_slots[i].stripes);
+    }
+
+
+    return 0;
+fail:
+    return ret;
+}
+
 /*
  * Given a key slot, and user password, this will attempt to unlock
  * the master encryption key from the key slot.
@@ -623,8 +722,6 @@  qcrypto_block_luks_open(QCryptoBlock *block,
     QCryptoBlockLUKS *luks = NULL;
     Error *local_err = NULL;
     int ret = 0;
-    size_t i;
-    ssize_t rv;
     g_autofree uint8_t *masterkey = NULL;
     char *ivgen_name, *ivhash_name;
     g_autofree char *password = NULL;
@@ -646,31 +743,11 @@  qcrypto_block_luks_open(QCryptoBlock *block,
     luks = g_new0(QCryptoBlockLUKS, 1);
     block->opaque = luks;
 
-    /* Read the entire LUKS header, minus the key material from
-     * the underlying device */
-    rv = readfunc(block, 0,
-                  (uint8_t *)&luks->header,
-                  sizeof(luks->header),
-                  opaque,
-                  errp);
-    if (rv < 0) {
-        ret = rv;
+    ret = qcrypto_block_luks_load_header(block, readfunc, opaque, errp);
+    if (ret) {
         goto fail;
     }
 
-    /* The header is always stored in big-endian format, so
-     * convert everything to native */
-    be16_to_cpus(&luks->header.version);
-    be32_to_cpus(&luks->header.payload_offset_sector);
-    be32_to_cpus(&luks->header.master_key_len);
-    be32_to_cpus(&luks->header.master_key_iterations);
-
-    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
-        be32_to_cpus(&luks->header.key_slots[i].active);
-        be32_to_cpus(&luks->header.key_slots[i].iterations);
-        be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
-        be32_to_cpus(&luks->header.key_slots[i].stripes);
-    }
 
     if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
                QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
@@ -1235,46 +1312,7 @@  qcrypto_block_luks_create(QCryptoBlock *block,
         goto error;
     }
 
-    /* Everything on disk uses Big Endian, so flip header fields
-     * before writing them */
-    cpu_to_be16s(&luks->header.version);
-    cpu_to_be32s(&luks->header.payload_offset_sector);
-    cpu_to_be32s(&luks->header.master_key_len);
-    cpu_to_be32s(&luks->header.master_key_iterations);
-
-    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
-        cpu_to_be32s(&luks->header.key_slots[i].active);
-        cpu_to_be32s(&luks->header.key_slots[i].iterations);
-        cpu_to_be32s(&luks->header.key_slots[i].key_offset_sector);
-        cpu_to_be32s(&luks->header.key_slots[i].stripes);
-    }
-
-
-    /* Write out the partition header and key slot headers */
-    writefunc(block, 0,
-              (const uint8_t *)&luks->header,
-              sizeof(luks->header),
-              opaque,
-              &local_err);
-
-    /* Delay checking local_err until we've byte-swapped */
-
-    /* Byte swap the header back to native, in case we need
-     * to read it again later */
-    be16_to_cpus(&luks->header.version);
-    be32_to_cpus(&luks->header.payload_offset_sector);
-    be32_to_cpus(&luks->header.master_key_len);
-    be32_to_cpus(&luks->header.master_key_iterations);
-
-    for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
-        be32_to_cpus(&luks->header.key_slots[i].active);
-        be32_to_cpus(&luks->header.key_slots[i].iterations);
-        be32_to_cpus(&luks->header.key_slots[i].key_offset_sector);
-        be32_to_cpus(&luks->header.key_slots[i].stripes);
-    }
-
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (qcrypto_block_luks_store_header(block,  writefunc, opaque, errp)) {
         goto error;
     }