diff mbox series

[v2,05/14] crypto: Use getrandom for qcrypto_random_bytes

Message ID 20190314045526.13342-6-richard.henderson@linaro.org
State New
Headers show
Series Add qemu_getrandom and ARMv8.5-RNG | expand

Commit Message

Richard Henderson March 14, 2019, 4:55 a.m. UTC
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(-)

-- 
2.17.1

Comments

Daniel P. Berrangé March 14, 2019, 3:38 p.m. UTC | #1
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 :|
diff mbox series

Patch

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