Re: [Qemu-devel] [PATCH v2 1/7] crypto: use uint64_t for pbkdf iteration count parameters

[Eric Blake]

On 09/12/2016 09:13 AM, Daniel P. Berrange wrote:
> The qcrypto_pbkdf_count_iters method uses a 64 bit int
> but then checks its value against INT32_MAX before
> returning it. This bounds check is premature, because
> the calling code may well scale the interation count

s/interation/iteration/

> by some value. It is thus better to return a 64-bit
> integer and let the caller do range checking.
> 
> For consistency the qcrypto_pbkdf method is also changed
> to accept a 64bit int, though this is somewhat academic
> since nettle is limited to taking an 'int' while gcrypt
> is limited to taking a 'long int'.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---

> +    uint64_t iters;
>  
>      memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
>      if (!luks_opts.has_cipher_alg) {

> @@ -1079,11 +1079,15 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>       * explanation why they chose /= 8... Probably so that
>       * if all 8 keyslots are active we only spend 1 second
>       * in total time to check all keys */
> -    luks->header.master_key_iterations /= 8;
> -    luks->header.master_key_iterations = MAX(
> -        luks->header.master_key_iterations,
> -        QCRYPTO_BLOCK_LUKS_MIN_MASTER_KEY_ITERS);
> -
> +    iters /= 8;
> +    if (iters > UINT32_MAX) {
> +        error_setg_errno(errp, ERANGE,
> +                         "PBKDF iterations %llu larger than %u",
> +                         (unsigned long long)iters, UINT32_MAX);
> +        goto error;

We could avoid the cast by using PRIu64 (couple of times in this patch).
 But not the end of the world to leave it.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature