diff mbox series

[v2,11/13] qcrypto-luks: refactoring: simplify the math used for keyslot locations

Message ID 20190826135103.22410-12-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:51 p.m. UTC 
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 crypto/block-luks.c | 64 +++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 23 deletions(-)

Comments

Daniel P. Berrangé Sept. 6, 2019, 1:17 p.m. UTC | #1 
On Mon, Aug 26, 2019 at 04:51:01PM +0300, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  crypto/block-luks.c | 64 +++++++++++++++++++++++++++++----------------
>  1 file changed, 41 insertions(+), 23 deletions(-)
> 
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index d713125925..6a43d97ce5 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -409,6 +409,32 @@ qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
>      }
>  }
>  
> +/*
> + * Returns number of sectors needed to store the key material
> + * given number of anti forensic stripes
> + */
> +static int
> +qcrypto_block_luks_splitkeylen_sectors(const QCryptoBlockLUKS *luks,
> +                                       unsigned int stripes)
> +{
> +    /*
> +     * This calculation doesn't match that shown in the spec,
> +     * but instead follows the cryptsetup implementation.
> +     */
> +
> +    size_t header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> +        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;

The caller already calculated that so just pass it in

> +
> +    size_t splitkeylen = luks->header.master_key_len * stripes;
> +
> +    /* First align the key material size to block size*/
> +    size_t splitkeylen_sectors =
> +        DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE);
> +
> +    /* Then also align the key material size to the size of the header */
> +    return ROUND_UP(splitkeylen_sectors, header_sectors);
> +}
> +
>  /*
>   * Stores the main LUKS header, taking care of endianess
>   */
> @@ -1151,7 +1177,8 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>      QCryptoBlockCreateOptionsLUKS luks_opts;
>      Error *local_err = NULL;
>      g_autofree uint8_t *masterkey = NULL;
> -    size_t splitkeylen = 0;
> +    size_t header_sectors;
> +    size_t split_key_sectors;
>      size_t i;
>      g_autofree char *password;
>      const char *cipher_alg;
> @@ -1370,37 +1397,28 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>          goto error;
>      }
>  
> +    /* start with the sector that follows the header*/
> +    header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> +        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> +
> +    split_key_sectors =
> +        qcrypto_block_luks_splitkeylen_sectors(luks,
> +                                               QCRYPTO_BLOCK_LUKS_STRIPES);
>  
> -    /* Although LUKS has multiple key slots, we're just going
> -     * to use the first key slot */
> -    splitkeylen = luks->header.master_key_len * QCRYPTO_BLOCK_LUKS_STRIPES;
>      for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> -        luks->header.key_slots[i].active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> -        luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> +        QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[i];
> +        slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
>  
> -        /* This calculation doesn't match that shown in the spec,
> -         * but instead follows the cryptsetup implementation.
> -         */
> -        luks->header.key_slots[i].key_offset_sector =
> -            (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> -             QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
> -            (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
> -                      (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> -                       QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) * i);
> +        slot->key_offset_sector = header_sectors + i * split_key_sectors;
> +        slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
>      }
>  
> -
>      /* The total size of the LUKS headers is the partition header + key
>       * slot headers, rounded up to the nearest sector, combined with
>       * the size of each master key material region, also rounded up
>       * to the nearest sector */
> -    luks->header.payload_offset_sector =
> -        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> -         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
> -        (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
> -                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> -                   QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
> -         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> +    luks->header.payload_offset_sector = header_sectors +
> +            QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
>  
>      block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
>      block->payload_offset = luks->header.payload_offset_sector *

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
Maxim Levitsky Sept. 12, 2019, 7:40 a.m. UTC | #2 
On Fri, 2019-09-06 at 14:17 +0100, Daniel P. Berrangé wrote:
> On Mon, Aug 26, 2019 at 04:51:01PM +0300, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  crypto/block-luks.c | 64 +++++++++++++++++++++++++++++----------------
> >  1 file changed, 41 insertions(+), 23 deletions(-)
> > 
> > diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> > index d713125925..6a43d97ce5 100644
> > --- a/crypto/block-luks.c
> > +++ b/crypto/block-luks.c
> > @@ -409,6 +409,32 @@ qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
> >      }
> >  }
> >  
> > +/*
> > + * Returns number of sectors needed to store the key material
> > + * given number of anti forensic stripes
> > + */
> > +static int
> > +qcrypto_block_luks_splitkeylen_sectors(const QCryptoBlockLUKS *luks,
> > +                                       unsigned int stripes)
> > +{
> > +    /*
> > +     * This calculation doesn't match that shown in the spec,
> > +     * but instead follows the cryptsetup implementation.
> > +     */
> > +
> > +    size_t header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > +        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> 
> The caller already calculated that so just pass it in

All right, no problem.

> 
> > +
> > +    size_t splitkeylen = luks->header.master_key_len * stripes;
> > +
> > +    /* First align the key material size to block size*/
> > +    size_t splitkeylen_sectors =
> > +        DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE);
> > +
> > +    /* Then also align the key material size to the size of the header */
> > +    return ROUND_UP(splitkeylen_sectors, header_sectors);
> > +}
> > +
> >  /*
> >   * Stores the main LUKS header, taking care of endianess
> >   */
> > @@ -1151,7 +1177,8 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> >      QCryptoBlockCreateOptionsLUKS luks_opts;
> >      Error *local_err = NULL;
> >      g_autofree uint8_t *masterkey = NULL;
> > -    size_t splitkeylen = 0;
> > +    size_t header_sectors;
> > +    size_t split_key_sectors;
> >      size_t i;
> >      g_autofree char *password;
> >      const char *cipher_alg;
> > @@ -1370,37 +1397,28 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> >          goto error;
> >      }
> >  
> > +    /* start with the sector that follows the header*/
> > +    header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > +        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> > +
> > +    split_key_sectors =
> > +        qcrypto_block_luks_splitkeylen_sectors(luks,
> > +                                               QCRYPTO_BLOCK_LUKS_STRIPES);
> >  
> > -    /* Although LUKS has multiple key slots, we're just going
> > -     * to use the first key slot */
> > -    splitkeylen = luks->header.master_key_len * QCRYPTO_BLOCK_LUKS_STRIPES;
> >      for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > -        luks->header.key_slots[i].active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> > -        luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> > +        QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[i];
> > +        slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> >  
> > -        /* This calculation doesn't match that shown in the spec,
> > -         * but instead follows the cryptsetup implementation.
> > -         */
> > -        luks->header.key_slots[i].key_offset_sector =
> > -            (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > -             QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
> > -            (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
> > -                      (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > -                       QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) * i);
> > +        slot->key_offset_sector = header_sectors + i * split_key_sectors;
> > +        slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> >      }
> >  
> > -
> >      /* The total size of the LUKS headers is the partition header + key
> >       * slot headers, rounded up to the nearest sector, combined with
> >       * the size of each master key material region, also rounded up
> >       * to the nearest sector */
> > -    luks->header.payload_offset_sector =
> > -        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > -         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
> > -        (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
> > -                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > -                   QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
> > -         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> > +    luks->header.payload_offset_sector = header_sectors +
> > +            QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
> >  
> >      block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> >      block->payload_offset = luks->header.payload_offset_sector *
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Regards,
> Daniel

Best regards,
	Maxim Levitsky
diff mbox series

Patch

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index d713125925..6a43d97ce5 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -409,6 +409,32 @@  qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher,
     }
 }
 
+/*
+ * Returns number of sectors needed to store the key material
+ * given number of anti forensic stripes
+ */
+static int
+qcrypto_block_luks_splitkeylen_sectors(const QCryptoBlockLUKS *luks,
+                                       unsigned int stripes)
+{
+    /*
+     * This calculation doesn't match that shown in the spec,
+     * but instead follows the cryptsetup implementation.
+     */
+
+    size_t header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
+        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+
+    size_t splitkeylen = luks->header.master_key_len * stripes;
+
+    /* First align the key material size to block size*/
+    size_t splitkeylen_sectors =
+        DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE);
+
+    /* Then also align the key material size to the size of the header */
+    return ROUND_UP(splitkeylen_sectors, header_sectors);
+}
+
 /*
  * Stores the main LUKS header, taking care of endianess
  */
@@ -1151,7 +1177,8 @@  qcrypto_block_luks_create(QCryptoBlock *block,
     QCryptoBlockCreateOptionsLUKS luks_opts;
     Error *local_err = NULL;
     g_autofree uint8_t *masterkey = NULL;
-    size_t splitkeylen = 0;
+    size_t header_sectors;
+    size_t split_key_sectors;
     size_t i;
     g_autofree char *password;
     const char *cipher_alg;
@@ -1370,37 +1397,28 @@  qcrypto_block_luks_create(QCryptoBlock *block,
         goto error;
     }
 
+    /* start with the sector that follows the header*/
+    header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
+        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+
+    split_key_sectors =
+        qcrypto_block_luks_splitkeylen_sectors(luks,
+                                               QCRYPTO_BLOCK_LUKS_STRIPES);
 
-    /* Although LUKS has multiple key slots, we're just going
-     * to use the first key slot */
-    splitkeylen = luks->header.master_key_len * QCRYPTO_BLOCK_LUKS_STRIPES;
     for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
-        luks->header.key_slots[i].active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
-        luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
+        QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[i];
+        slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
 
-        /* This calculation doesn't match that shown in the spec,
-         * but instead follows the cryptsetup implementation.
-         */
-        luks->header.key_slots[i].key_offset_sector =
-            (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
-             QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
-            (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
-                      (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
-                       QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) * i);
+        slot->key_offset_sector = header_sectors + i * split_key_sectors;
+        slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
     }
 
-
     /* The total size of the LUKS headers is the partition header + key
      * slot headers, rounded up to the nearest sector, combined with
      * the size of each master key material region, also rounded up
      * to the nearest sector */
-    luks->header.payload_offset_sector =
-        (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
-         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
-        (ROUND_UP(DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
-                  (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
-                   QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
-         QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
+    luks->header.payload_offset_sector = header_sectors +
+            QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS * split_key_sectors;
 
     block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
     block->payload_offset = luks->header.payload_offset_sector *