All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Maxim Levitsky <mlevitsk@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
	qemu-block@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 10/13] qcrypto-luks: refactoring: extract store key function
Date: Fri, 6 Sep 2019 14:14:14 +0100	[thread overview]
Message-ID: <20190906131414.GG5119@redhat.com> (raw)
In-Reply-To: <20190826135103.22410-11-mlevitsk@redhat.com>

On Mon, Aug 26, 2019 at 04:51:00PM +0300, Maxim Levitsky wrote:
> This function will be used later to store
> new keys to the luks metadata
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  crypto/block-luks.c | 310 ++++++++++++++++++++++++++------------------
>  1 file changed, 184 insertions(+), 126 deletions(-)
> 
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index cc9a52c9af..d713125925 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -650,6 +650,176 @@ out:
>  
>  }
>  
> +/*
> + * Given a key slot,  user password, and the master key,
> + * will store the encrypted master key there, and update the
> + * in-memory header. User must then write the in-memory header
> + *
> + * Returns:
> + *    0 if the keyslot was written successfully
> + *      with the provided password
> + *   -1 if a fatal error occurred while storing the key
> + */
> +static int
> +qcrypto_block_luks_store_key(QCryptoBlock *block,
> +                             unsigned int slot_idx,
> +                             const char *password,
> +                             uint8_t *masterkey,
> +                             uint64_t iter_time,
> +                             QCryptoBlockWriteFunc writefunc,
> +                             void *opaque,
> +                             Error **errp)
> +{
> +    QCryptoBlockLUKS *luks = block->opaque;
> +    QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
> +    g_autofree uint8_t *splitkey = NULL;
> +    size_t splitkeylen;
> +    g_autofree uint8_t *slotkey = NULL;
> +    g_autoptr(QCryptoCipher) cipher = NULL;
> +    g_autoptr(QCryptoIVGen) ivgen = NULL;
> +    Error *local_err = NULL;
> +    uint64_t iters;
> +    int ret = -1;
> +
> +    if (qcrypto_random_bytes(slot->salt,
> +                             QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                             errp) < 0) {
> +        goto cleanup;
> +    }
> +
> +    splitkeylen = luks->header.master_key_len * slot->stripes;
> +
> +    /*
> +     * Determine how many iterations are required to
> +     * hash the user password while consuming 1 second of compute
> +     * time
> +     */
> +    iters = qcrypto_pbkdf2_count_iters(luks->hash_alg,
> +                                       (uint8_t *)password, strlen(password),
> +                                       slot->salt,
> +                                       QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                                       luks->header.master_key_len,
> +                                       &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto cleanup;
> +    }
> +
> +    if (iters > (ULLONG_MAX / iter_time)) {
> +        error_setg_errno(errp, ERANGE,
> +                         "PBKDF iterations %llu too large to scale",
> +                         (unsigned long long)iters);
> +        goto cleanup;
> +    }
> +
> +    /* iter_time was in millis, but count_iters reported for secs */
> +    iters = iters * iter_time / 1000;
> +
> +    if (iters > UINT32_MAX) {
> +        error_setg_errno(errp, ERANGE,
> +                         "PBKDF iterations %llu larger than %u",
> +                         (unsigned long long)iters, UINT32_MAX);
> +        goto cleanup;
> +    }
> +
> +    slot->iterations =
> +        MAX(iters, QCRYPTO_BLOCK_LUKS_MIN_SLOT_KEY_ITERS);
> +
> +
> +    /*
> +     * Generate a key that we'll use to encrypt the master
> +     * key, from the user's password
> +     */
> +    slotkey = g_new0(uint8_t, luks->header.master_key_len);
> +    if (qcrypto_pbkdf2(luks->hash_alg,
> +                       (uint8_t *)password, strlen(password),
> +                       slot->salt,
> +                       QCRYPTO_BLOCK_LUKS_SALT_LEN,
> +                       slot->iterations,
> +                       slotkey, luks->header.master_key_len,
> +                       errp) < 0) {
> +        goto cleanup;
> +    }
> +
> +
> +    /*
> +     * Setup the encryption objects needed to encrypt the
> +     * master key material
> +     */
> +    cipher = qcrypto_cipher_new(luks->cipher_alg,
> +                                luks->cipher_mode,
> +                                slotkey, luks->header.master_key_len,
> +                                errp);
> +    if (!cipher) {
> +        goto cleanup;
> +    }
> +
> +    ivgen = qcrypto_ivgen_new(luks->ivgen_alg,
> +                              luks->ivgen_cipher_alg,
> +                              luks->ivgen_hash_alg,
> +                              slotkey, luks->header.master_key_len,
> +                              errp);
> +    if (!ivgen) {
> +        goto cleanup;
> +    }
> +
> +    /*
> +     * Before storing the master key, we need to vastly
> +     * increase its size, as protection against forensic
> +     * disk data recovery
> +     */
> +    splitkey = g_new0(uint8_t, splitkeylen);
> +
> +    if (qcrypto_afsplit_encode(luks->hash_alg,
> +                               luks->header.master_key_len,
> +                               slot->stripes,
> +                               masterkey,
> +                               splitkey,
> +                               errp) < 0) {
> +        goto cleanup;
> +    }
> +
> +    /*
> +     * Now we encrypt the split master key with the key generated
> +     * from the user's password, before storing it
> +     */
> +    if (qcrypto_block_cipher_encrypt_helper(cipher, block->niv, ivgen,
> +                                            QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
> +                                            0,
> +                                            splitkey,
> +                                            splitkeylen,
> +                                            errp) < 0) {
> +        goto cleanup;
> +    }
> +
> +    /* Write out the slot's master key material. */
> +    if (writefunc(block,
> +                  slot->key_offset_sector *
> +                  QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
> +                  splitkey, splitkeylen,
> +                  opaque,
> +                  errp) != splitkeylen) {
> +        goto cleanup;
> +    }
> +
> +    slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
> +
> +    if (qcrypto_block_luks_store_header(block,  writefunc, opaque, errp)) {

Check < 0

> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> +cleanup:
> +    if (slotkey) {
> +        memset(slotkey, 0, luks->header.master_key_len);
> +    }
> +    if (splitkey) {
> +        memset(splitkey, 0, splitkeylen);
> +    }
> +    return ret;
> +}
> +

> +    /* populate the slot 0 with the password encrypted master key*/
> +    /* This will also store the header */
> +    if (qcrypto_block_luks_store_key(block,
> +                                     0,
> +                                     password,
> +                                     masterkey,
> +                                     luks_opts.iter_time,
> +                                     writefunc,
> +                                     opaque,
> +                                     errp)) {
>          goto error;

Check < 0

> -    }
> +     }

Indent off by one

>  
> -    memset(masterkey, 0, luks->header.master_key_len);
> -    memset(slotkey, 0, luks->header.master_key_len);
>  
> +    memset(masterkey, 0, luks->header.master_key_len);
>      return 0;

The blank line was moved by accident.

>  
>   error:
>      if (masterkey) {
>          memset(masterkey, 0, luks->header.master_key_len);
>      }
> -    if (slotkey) {
> -        memset(slotkey, 0, luks->header.master_key_len);
> -    }
>  
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


  reply	other threads:[~2019-09-06 13:16 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-26 13:50 [Qemu-devel] [PATCH v2 00/13] RFC crypto/luks: preparation for encryption key managment Maxim Levitsky
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 01/13] introduce g_autowipe Maxim Levitsky
2019-08-27 10:46   ` Tony Nguyen
2019-08-27 10:52   ` Daniel P. Berrangé
2019-08-27 11:24     ` Maxim Levitsky
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 02/13] block-crypto: misc refactoring Maxim Levitsky
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 03/13] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader Maxim Levitsky
2019-09-06 12:27   ` Daniel P. Berrangé
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 04/13] qcrypto-luks: don't overwrite cipher_mode in header Maxim Levitsky
2019-09-06 12:29   ` Daniel P. Berrangé
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 05/13] qcrypto-luks: simplify masterkey and masterkey length Maxim Levitsky
2019-09-06 12:30   ` Daniel P. Berrangé
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 06/13] qcrypto-block: pass keyslot index rather that pointer to the keyslot Maxim Levitsky
2019-09-06 12:32   ` Daniel P. Berrangé
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 07/13] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS Maxim Levitsky
2019-09-06 12:35   ` Daniel P. Berrangé
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 08/13] qcrypto-luks: extract store and load header Maxim Levitsky
2019-09-06 13:06   ` Daniel P. Berrangé
2019-08-26 13:50 ` [Qemu-devel] [PATCH v2 09/13] qcrypto-block: extract check and parse header Maxim Levitsky
2019-09-06 13:11   ` Daniel P. Berrangé
2019-09-12  7:24     ` Maxim Levitsky
2019-08-26 13:51 ` [Qemu-devel] [PATCH v2 10/13] qcrypto-luks: refactoring: extract store key function Maxim Levitsky
2019-09-06 13:14   ` Daniel P. Berrangé [this message]
2019-08-26 13:51 ` [Qemu-devel] [PATCH v2 11/13] qcrypto-luks: refactoring: simplify the math used for keyslot locations Maxim Levitsky
2019-09-06 13:17   ` Daniel P. Berrangé
2019-09-12  7:40     ` Maxim Levitsky
2019-08-26 13:51 ` [Qemu-devel] [PATCH v2 12/13] qcrypto-luks: use g_autowipe Maxim Levitsky
2019-08-26 13:51 ` [Qemu-devel] [PATCH v2 13/13] qcrypto-luks: implement more rigorous header checking Maxim Levitsky
2019-09-06 13:34   ` Daniel P. Berrangé
2019-09-12  8:11     ` Maxim Levitsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190906131414.GG5119@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=fam@euphon.net \
    --cc=kwolf@redhat.com \
    --cc=mlevitsk@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.