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

2013-01-25 Thread Tim Hardeck 
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/


signature.asc
Description: This is a digitally signed message part

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

2013-01-25 Thread Markus Armbruster 
Tim Hardeck thard...@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!

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

2013-01-24 Thread Tim Hardeck 
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 arm...@redhat.com

Reviewed-by: Tim Hardeck thard...@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/



signature.asc
Description: OpenPGP digital signature

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

2013-01-23 Thread 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 arm...@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