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

Richard Henderson posted 14 patches 6 years, 11 months ago
Maintainers: Richard Henderson <rth@twiddle.net>, Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Gerd Hoffmann <kraxel@redhat.com>, Laurent Vivier <laurent@vivier.eu>
[Qemu-devel] [PATCH v2 05/14] crypto: Use getrandom for qcrypto_random_bytes
Posted by Richard Henderson 6 years, 11 months ago
Prefer it to direct use of /dev/urandom.

Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 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;
+    }
 #else
     /* TBD perhaps also add support for BSD getentropy / Linux
      * getrandom syscalls directly */
@@ -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;
+        }
+    }
 #else
     while (buflen > 0) {
         ssize_t got = read(fd, buf, buflen);
diff --git a/configure b/configure
index 8992b3aade..6a32284d26 100755
--- a/configure
+++ b/configure
@@ -5783,6 +5783,20 @@ if compile_prog "" "" ; then
     have_utmpx=yes
 fi
 
+##########################################
+# check for getrandom()
+
+have_getrandom=no
+cat > $TMPC << EOF
+#include <sys/random.h>
+int main(void) {
+    return getrandom(0, 0, GRND_NONBLOCK);
+}
+EOF
+if compile_prog "" "" ; then
+    have_getrandom=yes
+fi
+
 ##########################################
 # checks for sanitizers
 
@@ -7170,7 +7184,9 @@ fi
 if test "$have_utmpx" = "yes" ; then
   echo "HAVE_UTMPX=y" >> $config_host_mak
 fi
-
+if test "$have_getrandom" = "yes" ; then
+  echo "CONFIG_GETRANDOM=y" >> $config_host_mak
+fi
 if test "$ivshmem" = "yes" ; then
   echo "CONFIG_IVSHMEM=y" >> $config_host_mak
 fi
-- 
2.17.1


Re: [Qemu-devel] [PATCH v2 05/14] crypto: Use getrandom for qcrypto_random_bytes
Posted by Daniel P. Berrangé 6 years, 10 months ago
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é <berrange@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  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 :|