[Qemu-devel] [PATCH v3] crypto: qcrypto_random_bytes() now works on windows w/o any other crypto lib

Geert Martin Ijewski posted 1 patch 6 years, 11 months ago
Failed in applying to current master (apply log)
include/crypto/random.h  |  9 +++++++++
5 files changed, 57 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH v3] crypto: qcrypto_random_bytes() now works on windows w/o any other crypto lib
Posted by Geert Martin Ijewski 6 years, 11 months ago
If no crypto library is included in the build, QEMU uses
qcrypto_random_bytes() to generate random data. That function tried to 
open /dev/urandom or /dev/random and if openeing neither file worked it 
errored out.

Those files obviously do not exist on windows, so there the code uses
CryptGenRandom().

Furthermore there was some refactoring and a new function
qcrypto_random_init() was introduced. If a proper crypto library (gnutls 
or libgcrypt) is included in the build, this function does nothing. If 
neither is included it initalizes the (platform specific) handles that 
are used by qcrypto_random_bytes().
Either:
* a handle to /dev/urandom | /dev/random on unix like systems
* a handle to a cryptographic service provider on windows

Signed-off-by: Geert Martin Ijewski <gm.ijewski@web.de>
---
  crypto/init.c            |  6 ++++++
  crypto/random-gcrypt.c   |  2 ++
  crypto/random-gnutls.c   |  3 +++
  crypto/random-platform.c | 45 
+++++++++++++++++++++++++++++++++++++--------
  include/crypto/random.h  |  9 +++++++++
  5 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/crypto/init.c b/crypto/init.c
index f65207e..f131c42 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -32,6 +32,8 @@
  #include <gcrypt.h>
  #endif

+#include "crypto/random.h"
+
  /* #define DEBUG_GNUTLS */

  /*
@@ -146,5 +148,9 @@ int qcrypto_init(Error **errp)
      gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0);
  #endif

+    if (qcrypto_random_init(errp) < 0) {
+        return -1;
+    }
+
      return 0;
  }
diff --git a/crypto/random-gcrypt.c b/crypto/random-gcrypt.c
index 0de9a09..2446142 100644
--- a/crypto/random-gcrypt.c
+++ b/crypto/random-gcrypt.c
@@ -31,3 +31,5 @@ int qcrypto_random_bytes(uint8_t *buf,
      gcry_randomize(buf, buflen, GCRY_STRONG_RANDOM);
      return 0;
  }
+
+int qcrpto_random_init(Error **errp G_GNUC_UNUSED) { return 0; }
diff --git a/crypto/random-gnutls.c b/crypto/random-gnutls.c
index 04b45a8..d80c786 100644
--- a/crypto/random-gnutls.c
+++ b/crypto/random-gnutls.c
@@ -41,3 +41,6 @@ int qcrypto_random_bytes(uint8_t *buf,

      return 0;
  }
+
+
+int qcrpto_random_init(Error **errp G_GNUC_UNUSED) { return 0; }
diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index 82b755a..c613b23 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -22,14 +22,23 @@

  #include "crypto/random.h"

-int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
-                         size_t buflen G_GNUC_UNUSED,
-                         Error **errp)
-{
-    int fd;
-    int ret = -1;
-    int got;
+#ifdef _WIN32
+#include <Wincrypt.h>
+static HCRYPTPROV hCryptProv;
+#else
+static int fd; /* a file handle to either /dev/urandom or /dev/random */
+#endif

+int qcrypto_random_init(Error **errp)
+{
+#ifdef _WIN32
+    if (!CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL,
+                             CRYPT_SILENT | CRYPT_VERIFYCONTEXT)) {
+        error_setg_win32(errp, GetLastError(),
+                         "Unable to create cryptographic provider");
+        return -1;
+    }
+#else
      /* TBD perhaps also add support for BSD getentropy / Linux
       * getrandom syscalls directly */
      fd = open("/dev/urandom", O_RDONLY);
@@ -41,6 +50,18 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
          error_setg(errp, "No /dev/urandom or /dev/random found");
          return -1;
      }
+#endif
+
+    return 0;
+}
+
+int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
+                         size_t buflen G_GNUC_UNUSED,
+                         Error **errp)
+{
+#ifndef _WIN32
+    int ret = -1;
+    int got;

      while (buflen > 0) {
          got = read(fd, buf, buflen);
@@ -59,6 +80,14 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,

      ret = 0;
   cleanup:
-    close(fd);
      return ret;
+#else
+    if (!CryptGenRandom(hCryptProv, buflen, buf)) {
+        error_setg_win32(errp, GetLastError(),
+                         "Unable to read random bytes");
+        return -1;
+    }
+
+    return 0;
+#endif
  }
diff --git a/include/crypto/random.h b/include/crypto/random.h
index a101353..82a3209 100644
--- a/include/crypto/random.h
+++ b/include/crypto/random.h
@@ -40,5 +40,14 @@ int qcrypto_random_bytes(uint8_t *buf,
                           size_t buflen,
                           Error **errp);

+/**
+ * qcrypto_random_init:
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Initalizes the handles used by qcrypto_random_bytes
+ *
+ * Returns 0 on success, -1 on error
+ */
+int qcrypto_random_init(Error **errp);

  #endif /* QCRYPTO_RANDOM_H */
-- 
2.10.1.windows.1



Re: [Qemu-devel] [PATCH v3] crypto: qcrypto_random_bytes() now works on windows w/o any other crypto lib
Posted by Daniel P. Berrange 6 years, 11 months ago
On Tue, Apr 25, 2017 at 07:28:07PM +0200, Geert Martin Ijewski wrote:
> If no crypto library is included in the build, QEMU uses
> qcrypto_random_bytes() to generate random data. That function tried to open
> /dev/urandom or /dev/random and if openeing neither file worked it errored
> out.
> 
> Those files obviously do not exist on windows, so there the code uses
> CryptGenRandom().
> 
> Furthermore there was some refactoring and a new function
> qcrypto_random_init() was introduced. If a proper crypto library (gnutls or
> libgcrypt) is included in the build, this function does nothing. If neither
> is included it initalizes the (platform specific) handles that are used by
> qcrypto_random_bytes().
> Either:
> * a handle to /dev/urandom | /dev/random on unix like systems
> * a handle to a cryptographic service provider on windows
> 
> Signed-off-by: Geert Martin Ijewski <gm.ijewski@web.de>

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


I'll add this to my queue unless someone raises further comments.


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

Re: [Qemu-devel] [PATCH v3] crypto: qcrypto_random_bytes() now works on windows w/o any other crypto lib
Posted by Eric Blake 6 years, 11 months ago
On 04/25/2017 12:28 PM, Geert Martin Ijewski wrote:
> If no crypto library is included in the build, QEMU uses
> qcrypto_random_bytes() to generate random data. That function tried to
> open /dev/urandom or /dev/random and if openeing neither file worked it

s/openeing/opening/
s/opening neither file worked/opening both files failed/

> errored out.
> 
> Those files obviously do not exist on windows, so there the code uses
> CryptGenRandom().
> 
> Furthermore there was some refactoring and a new function
> qcrypto_random_init() was introduced. If a proper crypto library (gnutls
> or libgcrypt) is included in the build, this function does nothing. If
> neither is included it initalizes the (platform specific) handles that

s/initalizes/initializes/

> are used by qcrypto_random_bytes().
> Either:
> * a handle to /dev/urandom | /dev/random on unix like systems
> * a handle to a cryptographic service provider on windows
> 
> Signed-off-by: Geert Martin Ijewski <gm.ijewski@web.de>
> ---

> +int qcrypto_random_init(Error **errp)
> +{
> +#ifdef _WIN32

Weird that you use #ifdef _WIN32 here...


> +int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
> +                         size_t buflen G_GNUC_UNUSED,
> +                         Error **errp)
> +{
> +#ifndef _WIN32

...but #ifndef here. It would be more consistent if all dual-implemented
functions have the #ifdefs in the same order, so that I can focus on
first or second half without having to check sense.


> +++ b/include/crypto/random.h
> @@ -40,5 +40,14 @@ int qcrypto_random_bytes(uint8_t *buf,
>                           size_t buflen,
>                           Error **errp);
> 
> +/**
> + * qcrypto_random_init:
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Initalizes the handles used by qcrypto_random_bytes

s/Initalizes/Initializes/

> + *
> + * Returns 0 on success, -1 on error
> + */
> +int qcrypto_random_init(Error **errp);
> 
>  #endif /* QCRYPTO_RANDOM_H */

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