qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).