[Qemu-trivial] [PATCH] vnc: Clean up vncws_send_handshake_response()

[Qemu-trivial] [PATCH] vnc: Clean up vncws_send_handshake_response()

[Markus Armbruster]

Use appropriate types, drop superfluous casts, use sizeof, don't
exploit that this particular call of gnutls_fingerprint() doesn't
change its last argument.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ui/vnc-ws.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 9ccdc19..de7e74c 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -120,8 +120,8 @@ static char *vncws_extract_handshake_entry(const char *handshake,
 static void vncws_send_handshake_response(VncState *vs, const char* key)
 {
     char combined_key[WS_CLIENT_KEY_LEN + WS_GUID_LEN + 1];
-    char hash[SHA1_DIGEST_LEN];
-    size_t hash_size = SHA1_DIGEST_LEN;
+    unsigned char hash[SHA1_DIGEST_LEN];
+    size_t hash_size = sizeof(hash);
     char *accept = NULL, *response = NULL;
     gnutls_datum_t in;
 
@@ -133,7 +133,7 @@ static void vncws_send_handshake_response(VncState *vs, const char* key)
     in.size = WS_CLIENT_KEY_LEN + WS_GUID_LEN;
     if (gnutls_fingerprint(GNUTLS_DIG_SHA1, &in, hash, &hash_size)
             == GNUTLS_E_SUCCESS) {
-        accept = g_base64_encode((guchar *)hash, SHA1_DIGEST_LEN);
+        accept = g_base64_encode(hash, hash_size);
     }
     if (accept == NULL) {
         VNC_DEBUG("Hashing Websocket combined key failed\n");
-- 
1.7.11.7

Re: [Qemu-trivial] [PATCH] vnc: Clean up vncws_send_handshake_response()

[Tim Hardeck]

[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]

Hi Markus,

On 01/23/2013 06:16 PM, Markus Armbruster wrote:
> Use appropriate types, drop superfluous casts, use sizeof, don't
> exploit that this particular call of gnutls_fingerprint() doesn't
> change its last argument.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Tim Hardeck <thardeck@suse.de>

Regards
Tim

> ---
>  ui/vnc-ws.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index 9ccdc19..de7e74c 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -120,8 +120,8 @@ static char *vncws_extract_handshake_entry(const char *handshake,
>  static void vncws_send_handshake_response(VncState *vs, const char* key)
>  {
>      char combined_key[WS_CLIENT_KEY_LEN + WS_GUID_LEN + 1];
> -    char hash[SHA1_DIGEST_LEN];
> -    size_t hash_size = SHA1_DIGEST_LEN;
> +    unsigned char hash[SHA1_DIGEST_LEN];
> +    size_t hash_size = sizeof(hash);
>      char *accept = NULL, *response = NULL;
>      gnutls_datum_t in;
>  
> @@ -133,7 +133,7 @@ static void vncws_send_handshake_response(VncState *vs, const char* key)
>      in.size = WS_CLIENT_KEY_LEN + WS_GUID_LEN;
>      if (gnutls_fingerprint(GNUTLS_DIG_SHA1, &in, hash, &hash_size)
>              == GNUTLS_E_SUCCESS) {
> -        accept = g_base64_encode((guchar *)hash, SHA1_DIGEST_LEN);
> +        accept = g_base64_encode(hash, hash_size);
>      }
>      if (accept == NULL) {
>          VNC_DEBUG("Hashing Websocket combined key failed\n");
> 


-- 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix
Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstr. 5, 90409 Nürnberg, Germany
T: +49 (0) 911 74053-0  F: +49 (0) 911 74053-483
http://www.suse.de/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [Qemu-trivial] [PATCH] vnc: Clean up vncws_send_handshake_response()

[Tim Hardeck]

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

Hi Markus,

thanks for your input.

On Wed, 2013-01-23 at 18:16 +0100, Markus Armbruster wrote:
> Use appropriate types, drop superfluous casts, use sizeof, don't
> exploit that this particular call of gnutls_fingerprint() doesn't
> change its last argument.

your patch does work fine but if we expect gnutls_fingerprint to change
the hash_size there has to be an additional check if the hash_size is
bigger than SHA1_DIGEST_LEN.

For example:

diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index de7e74c..e64c895 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -132,7 +132,7 @@ static void vncws_send_handshake_response(VncState
*vs, const char* key)
     in.data = (void *)combined_key;
     in.size = WS_CLIENT_KEY_LEN + WS_GUID_LEN;
     if (gnutls_fingerprint(GNUTLS_DIG_SHA1, &in, hash, &hash_size)
-            == GNUTLS_E_SUCCESS) {
+            == GNUTLS_E_SUCCESS && hash_size <= SHA1_DIGEST_LEN) {
         accept = g_base64_encode(hash, hash_size);
     }
     if (accept == NULL) {


-- 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix
Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstr. 5, 90409 Nürnberg, Germany
T: +49 (0) 911 74053-0  F: +49 (0) 911 74053-483
http://www.suse.de/

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [Qemu-trivial] [PATCH] vnc: Clean up vncws_send_handshake_response()

[Markus Armbruster]

Tim Hardeck <thardeck@suse.de> writes:

> Hi Markus,
>
> thanks for your input.
>
> On Wed, 2013-01-23 at 18:16 +0100, Markus Armbruster wrote:
>> Use appropriate types, drop superfluous casts, use sizeof, don't
>> exploit that this particular call of gnutls_fingerprint() doesn't
>> change its last argument.
>
> your patch does work fine but if we expect gnutls_fingerprint to change
> the hash_size there has to be an additional check if the hash_size is
> bigger than SHA1_DIGEST_LEN.
>
> For example:
>
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index de7e74c..e64c895 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -132,7 +132,7 @@ static void vncws_send_handshake_response(VncState
> *vs, const char* key)
>      in.data = (void *)combined_key;
>      in.size = WS_CLIENT_KEY_LEN + WS_GUID_LEN;
>      if (gnutls_fingerprint(GNUTLS_DIG_SHA1, &in, hash, &hash_size)
> -            == GNUTLS_E_SUCCESS) {
> +            == GNUTLS_E_SUCCESS && hash_size <= SHA1_DIGEST_LEN) {
>          accept = g_base64_encode(hash, hash_size);
>      }
>      if (accept == NULL) {

Makes sense.  I'll respin.  Thanks!