[Qemu-devel] [PATCH v3 08/23] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc

Richard Henderson posted 23 patches 6 years, 7 months ago
Maintainers: David Gibson <david@gibson.dropbear.id.au>, Laurent Vivier <laurent@vivier.eu>, Riku Voipio <riku.voipio@iki.fi>, Paolo Bonzini <pbonzini@redhat.com>, Joel Stanley <joel@jms.id.au>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v3 08/23] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc
Posted by Richard Henderson 6 years, 7 months ago
Use a better interface for random numbers than rand().
Fail gracefully if for some reason we cannot use the crypto system.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Use qcrypto_random_bytes, not qemu_getrandom, as there is
    no need for deterministic results for this interface.
v3: Fail gracefully in the event qcrypto_random_bytes fails.
---
 ui/vnc.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 785edf3af1..d83f4a6ff9 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -43,6 +43,7 @@
 #include "crypto/hash.h"
 #include "crypto/tlscredsanon.h"
 #include "crypto/tlscredsx509.h"
+#include "crypto/random.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
 #include "io/dns-resolver.h"
@@ -2547,16 +2548,6 @@ static void authentication_failed(VncState *vs)
     vnc_client_error(vs);
 }
 
-static void make_challenge(VncState *vs)
-{
-    int i;
-
-    srand(time(NULL)+getpid()+getpid()*987654+rand());
-
-    for (i = 0 ; i < sizeof(vs->challenge) ; i++)
-        vs->challenge[i] = (int) (256.0*rand()/(RAND_MAX+1.0));
-}
-
 static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
 {
     unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
@@ -2628,7 +2619,16 @@ reject:
 
 void start_auth_vnc(VncState *vs)
 {
-    make_challenge(vs);
+    Error *err = NULL;
+
+    if (qcrypto_random_bytes(vs->challenge, sizeof(vs->challenge), &err)) {
+        trace_vnc_auth_fail(vs, vs->auth, "cannot get random bytes",
+                            error_get_pretty(err));
+        error_free(err);
+        authentication_failed(vs);
+        return;
+    }
+
     /* Send client a 'random' challenge */
     vnc_write(vs, vs->challenge, sizeof(vs->challenge));
     vnc_flush(vs);
-- 
2.17.2


Re: [Qemu-devel] [PATCH v3 08/23] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc
Posted by Gerd Hoffmann 6 years, 7 months ago
On Thu, Mar 14, 2019 at 08:26:14PM -0700, Richard Henderson wrote:
> Use a better interface for random numbers than rand().
> Fail gracefully if for some reason we cannot use the crypto system.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Use qcrypto_random_bytes, not qemu_getrandom, as there is
>     no need for deterministic results for this interface.
> v3: Fail gracefully in the event qcrypto_random_bytes fails.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>


Re: [Qemu-devel] [PATCH v3 08/23] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc
Posted by Daniel P. Berrangé 6 years, 7 months ago
On Thu, Mar 14, 2019 at 08:26:14PM -0700, Richard Henderson wrote:
> Use a better interface for random numbers than rand().
> Fail gracefully if for some reason we cannot use the crypto system.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Use qcrypto_random_bytes, not qemu_getrandom, as there is
>     no need for deterministic results for this interface.
> v3: Fail gracefully in the event qcrypto_random_bytes fails.
> ---
>  ui/vnc.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

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


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 08/23] ui/vnc: Use gcrypto_random_bytes for start_auth_vnc
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago
On 3/15/19 4:26 AM, Richard Henderson wrote:
> Use a better interface for random numbers than rand().
> Fail gracefully if for some reason we cannot use the crypto system.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
> v2: Use qcrypto_random_bytes, not qemu_getrandom, as there is
>     no need for deterministic results for this interface.
> v3: Fail gracefully in the event qcrypto_random_bytes fails.
> ---
>  ui/vnc.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 785edf3af1..d83f4a6ff9 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -43,6 +43,7 @@
>  #include "crypto/hash.h"
>  #include "crypto/tlscredsanon.h"
>  #include "crypto/tlscredsx509.h"
> +#include "crypto/random.h"
>  #include "qom/object_interfaces.h"
>  #include "qemu/cutils.h"
>  #include "io/dns-resolver.h"
> @@ -2547,16 +2548,6 @@ static void authentication_failed(VncState *vs)
>      vnc_client_error(vs);
>  }
>  
> -static void make_challenge(VncState *vs)
> -{
> -    int i;
> -
> -    srand(time(NULL)+getpid()+getpid()*987654+rand());
> -
> -    for (i = 0 ; i < sizeof(vs->challenge) ; i++)
> -        vs->challenge[i] = (int) (256.0*rand()/(RAND_MAX+1.0));
> -}
> -
>  static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
>  {
>      unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
> @@ -2628,7 +2619,16 @@ reject:
>  
>  void start_auth_vnc(VncState *vs)
>  {
> -    make_challenge(vs);
> +    Error *err = NULL;
> +
> +    if (qcrypto_random_bytes(vs->challenge, sizeof(vs->challenge), &err)) {
> +        trace_vnc_auth_fail(vs, vs->auth, "cannot get random bytes",
> +                            error_get_pretty(err));
> +        error_free(err);
> +        authentication_failed(vs);
> +        return;
> +    }
> +
>      /* Send client a 'random' challenge */
>      vnc_write(vs, vs->challenge, sizeof(vs->challenge));
>      vnc_flush(vs);
>