[libvirt] [PATCH 04/10] virCryptoGenerateRandom: Don't allocate return buffer

[Eric Blake]

On 05/29/2018 03:24 AM, Michal Privoznik wrote:
> To unify our vir*Random() functions we need to make
> virCryptoGenerateRandom NOT allocate return buffer. It should
> just fill given buffer with random data.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>   src/qemu/qemu_domain.c   | 12 ++++++++----
>   src/util/vircrypto.c     | 29 ++++++++++++-----------------
>   src/util/vircrypto.h     |  3 ++-
>   tests/qemuxml2argvmock.c | 14 ++++----------
>   4 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 47910acb83..2d13a03344 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -930,12 +930,13 @@ qemuDomainMasterKeyCreate(virDomainObjPtr vm)
>       if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET))
>           return 0;
>   
> -    if (!(priv->masterKey =
> -          virCryptoGenerateRandom(QEMU_DOMAIN_MASTER_KEY_LEN)))
> +    if (VIR_ALLOC_N(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
>           return -1;
> -
>       priv->masterKeyLen = QEMU_DOMAIN_MASTER_KEY_LEN;
>   
> +    if (virCryptoGenerateRandom(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
> +        return -1;

Should this free priv->masterKey and set it back to NULL, so that no 
other client is tempted to use a half-baked buffer as a key prior to the 
object being destroyed?


> +++ b/tests/qemuxml2argvmock.c
> @@ -190,17 +190,11 @@ virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
>       /* nada */
>   }
>   
> -uint8_t *
> -virCryptoGenerateRandom(size_t nbytes)
> +int
> +virCryptoGenerateRandom(unsigned char *buf,
> +                       size_t buflen)

Indentation looks off.

>   {
> -    uint8_t *buf;
> -
> -    if (VIR_ALLOC_N(buf, nbytes) < 0)
> -        return NULL;
> -
> -    ignore_value(virRandomBytes(buf, nbytes));
> -
> -    return buf;
> +    return virRandomBytes(buf, buflen);

Hmm, my earlier comment about the #if 0 for debugging might be more 
relevant here - if we are going to mock the random numbers to be 
reproducible during the testsuite, THIS would be a nice place to fall 
back to rand() and friends with a reliable sequence when given a fixed 
seed (rather than directly in src/util/virrandom.c).


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org