Re: [Qemu-devel] [PATCH v3 07/23] ui/vnc: Split out authentication_failure

[Philippe Mathieu-Daudé]

On 3/15/19 4:26 AM, Richard Henderson wrote:
> There were 3 copies of this code, one of which used the wrong
> data size for the failure indicator.

Which, as the comment said, has never been called.

Good cleanup.

Reviewed-by: Philippe Mathieu-Daudé 

> Cc: Gerd Hoffmann 
> Signed-off-by: Richard Henderson 
> ---
>  ui/vnc.c | 37 +++--
>  1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 1871422e1d..785edf3af1 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2535,6 +2535,18 @@ void start_client_init(VncState *vs)
>  vnc_read_when(vs, protocol_client_init, 1);
>  }
>  
> +static void authentication_failed(VncState *vs)
> +{
> +vnc_write_u32(vs, 1); /* Reject auth */
> +if (vs->minor >= 8) {
> +static const char err[] = "Authentication failed";
> +vnc_write_u32(vs, sizeof(err));
> +vnc_write(vs, err, sizeof(err));
> +}
> +vnc_flush(vs);
> +vnc_client_error(vs);
> +}
> +
>  static void make_challenge(VncState *vs)
>  {
>  int i;
> @@ -2609,14 +2621,7 @@ static int protocol_client_auth_vnc(VncState *vs, 
> uint8_t *data, size_t len)
>  return 0;
>  
>  reject:
> -vnc_write_u32(vs, 1); /* Reject auth */
> -if (vs->minor >= 8) {
> -static const char err[] = "Authentication failed";
> -vnc_write_u32(vs, sizeof(err));
> -vnc_write(vs, err, sizeof(err));
> -}
> -vnc_flush(vs);
> -vnc_client_error(vs);
> +authentication_failed(vs);
>  qcrypto_cipher_free(cipher);
>  return 0;
>  }
> @@ -2638,13 +2643,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
> *data, size_t len)
>   * must pick the one we sent. Verify this */
>  if (data[0] != vs->auth) { /* Reject auth */
> trace_vnc_auth_reject(vs, vs->auth, (int)data[0]);
> -   vnc_write_u32(vs, 1);
> -   if (vs->minor >= 8) {
> -   static const char err[] = "Authentication failed";
> -   vnc_write_u32(vs, sizeof(err));
> -   vnc_write(vs, err, sizeof(err));
> -   }
> -   vnc_client_error(vs);
> +   authentication_failed(vs);
>  } else { /* Accept requested auth */
> trace_vnc_auth_start(vs, vs->auth);
> switch (vs->auth) {
> @@ -2673,13 +2672,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
> *data, size_t len)
>  
> default: /* Should not be possible, but just in case */
> trace_vnc_auth_fail(vs, vs->auth, "Unhandled auth method", "");
> -   vnc_write_u8(vs, 1);
> -   if (vs->minor >= 8) {
> -   static const char err[] = "Authentication failed";
> -   vnc_write_u32(vs, sizeof(err));
> -   vnc_write(vs, err, sizeof(err));
> -   }
> -   vnc_client_error(vs);
> +   authentication_failed(vs);
> }
>  }
>  return 0;
> 

Re: [Qemu-devel] [PATCH v3 07/23] ui/vnc: Split out authentication_failure

[Daniel P . Berrangé]

On Thu, Mar 14, 2019 at 08:26:13PM -0700, Richard Henderson wrote:
> There were 3 copies of this code, one of which used the wrong
> data size for the failure indicator.
> 
> Cc: Gerd Hoffmann 
> Signed-off-by: Richard Henderson 
> ---
>  ui/vnc.c | 37 +++--
>  1 file changed, 15 insertions(+), 22 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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 07/23] ui/vnc: Split out authentication_failure

[Gerd Hoffmann]

On Thu, Mar 14, 2019 at 08:26:13PM -0700, Richard Henderson wrote:
> There were 3 copies of this code, one of which used the wrong
> data size for the failure indicator.
> 
> Cc: Gerd Hoffmann 
> Signed-off-by: Richard Henderson 

Reviewed-by: Gerd Hoffmann 

[Qemu-devel] [PATCH v3 07/23] ui/vnc: Split out authentication_failure

[Richard Henderson]

There were 3 copies of this code, one of which used the wrong
data size for the failure indicator.

Cc: Gerd Hoffmann 
Signed-off-by: Richard Henderson 
---
 ui/vnc.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 1871422e1d..785edf3af1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2535,6 +2535,18 @@ void start_client_init(VncState *vs)
 vnc_read_when(vs, protocol_client_init, 1);
 }
 
+static void authentication_failed(VncState *vs)
+{
+vnc_write_u32(vs, 1); /* Reject auth */
+if (vs->minor >= 8) {
+static const char err[] = "Authentication failed";
+vnc_write_u32(vs, sizeof(err));
+vnc_write(vs, err, sizeof(err));
+}
+vnc_flush(vs);
+vnc_client_error(vs);
+}
+
 static void make_challenge(VncState *vs)
 {
 int i;
@@ -2609,14 +2621,7 @@ static int protocol_client_auth_vnc(VncState *vs, 
uint8_t *data, size_t len)
 return 0;
 
 reject:
-vnc_write_u32(vs, 1); /* Reject auth */
-if (vs->minor >= 8) {
-static const char err[] = "Authentication failed";
-vnc_write_u32(vs, sizeof(err));
-vnc_write(vs, err, sizeof(err));
-}
-vnc_flush(vs);
-vnc_client_error(vs);
+authentication_failed(vs);
 qcrypto_cipher_free(cipher);
 return 0;
 }
@@ -2638,13 +2643,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
*data, size_t len)
  * must pick the one we sent. Verify this */
 if (data[0] != vs->auth) { /* Reject auth */
trace_vnc_auth_reject(vs, vs->auth, (int)data[0]);
-   vnc_write_u32(vs, 1);
-   if (vs->minor >= 8) {
-   static const char err[] = "Authentication failed";
-   vnc_write_u32(vs, sizeof(err));
-   vnc_write(vs, err, sizeof(err));
-   }
-   vnc_client_error(vs);
+   authentication_failed(vs);
 } else { /* Accept requested auth */
trace_vnc_auth_start(vs, vs->auth);
switch (vs->auth) {
@@ -2673,13 +2672,7 @@ static int protocol_client_auth(VncState *vs, uint8_t 
*data, size_t len)
 
default: /* Should not be possible, but just in case */
trace_vnc_auth_fail(vs, vs->auth, "Unhandled auth method", "");
-   vnc_write_u8(vs, 1);
-   if (vs->minor >= 8) {
-   static const char err[] = "Authentication failed";
-   vnc_write_u32(vs, sizeof(err));
-   vnc_write(vs, err, sizeof(err));
-   }
-   vnc_client_error(vs);
+   authentication_failed(vs);
}
 }
 return 0;
-- 
2.17.2