Re: [Qemu-devel] [PATCH v2 05/14] crypto: Use getrandom for qcrypto_random_bytes

[Daniel P . Berrangé]

On Wed, Mar 13, 2019 at 09:55:17PM -0700, Richard Henderson wrote:
> Prefer it to direct use of /dev/urandom.
> 
> Cc: Daniel P. Berrangé <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  crypto/random-platform.c | 21 +++++++++++++++++++++
>  configure                | 18 +++++++++++++++++-
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/random-platform.c b/crypto/random-platform.c
> index 8bfce99a65..bdaa8fbbfb 100644
> --- a/crypto/random-platform.c
> +++ b/crypto/random-platform.c
> @@ -26,6 +26,8 @@
>  #ifdef _WIN32
>  #include <wincrypt.h>
>  static HCRYPTPROV hCryptProv;
> +#elif defined(CONFIG_GETRANDOM)
> +#include <sys/random.h>
>  #else
>  static int fd; /* a file handle to either /dev/urandom or /dev/random */
>  #endif
> @@ -39,6 +41,12 @@ int qcrypto_random_init(Error **errp)
>                           "Unable to create cryptographic provider");
>          return -1;
>      }
> +#elif defined(CONFIG_GETRANDOM)
> +    errno = 0;
> +    if (getrandom(NULL, 0, 0) < 0 && errno == ENOSYS) {
> +        error_setg_errno(errp, errno, "getrandom");
> +        return -1;
> +    }

I'm not seeing why you do this ?  This ought to set a global
flag which the later code below can use to decide whether to
use getrandom or /dev/random

>  #else
>      /* TBD perhaps also add support for BSD getentropy / Linux
>       * getrandom syscalls directly */

Comment needs updating now.

> @@ -65,6 +73,19 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
>                           "Unable to read random bytes");
>          return -1;
>      }
> +#elif defined(CONFIG_GETRANDOM)
> +    while (buflen > 0) {
> +        ssize_t got = getrandom(buf, buflen, 0);
> +        if (unlikely(got < 0)) {
> +            if (errno != EINTR) {
> +                error_setg_errno(errp, errno, "getrandom");
> +                return -1;
> +            }
> +        } else {
> +            buflen -= got;
> +            buf += got;
> +        }
> +    }

This needs to be able to conditionally fall back to reading
from /dev/urandom as We can't assume that the kernel headers
we compile against match the kernel we run against. IOW we
might have enabled getrandom but not be able to use it.


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 :|