| Message ID | 1400669661-18353-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
> -----Original Message----- > From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On > Behalf Of Gerd Hoffmann > Sent: Wednesday, May 21, 2014 6:54 PM > To: qemu-devel@nongnu.org > Cc: Gerd Hoffmann; Anthony Liguori > Subject: [Qemu-devel] [PATCH] vnc: refuse to set a password with > VNC_AUTH_NONE > > Current code silently changes the authentication settings > in case you try to set a password without password authentication > turned on. This is bad. Return an error instead. > > If we want allow changing auth settings at runtime this should > be done explicitly using a separate monitor command, not as > side effect of set_passwd. > Agreed. Do you have a plan to do this? Thanks. > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > ui/vnc.c | 34 ++++++---------------------------- > 1 file changed, 6 insertions(+), 28 deletions(-) > Reviewed-by: Gonglei <arei.gonglei@huawei.com> Best regards, -Gonglei
On Do, 2014-05-22 at 04:05 +0000, Gonglei (Arei) wrote: > > -----Original Message----- > > From: qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org > > [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On > > Behalf Of Gerd Hoffmann > > Sent: Wednesday, May 21, 2014 6:54 PM > > To: qemu-devel@nongnu.org > > Cc: Gerd Hoffmann; Anthony Liguori > > Subject: [Qemu-devel] [PATCH] vnc: refuse to set a password with > > VNC_AUTH_NONE > > > > Current code silently changes the authentication settings > > in case you try to set a password without password authentication > > turned on. This is bad. Return an error instead. > > > > If we want allow changing auth settings at runtime this should > > be done explicitly using a separate monitor command, not as > > side effect of set_passwd. > > > Agreed. Do you have a plan to do this? Thanks. No, unless someone comes up with a reasonable use case for this. cheers, Gerd
diff --git a/ui/vnc.c b/ui/vnc.c index 2d7def9..64aa2fa 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2976,26 +2976,6 @@ static void vnc_display_close(DisplayState *ds) #endif } -static int vnc_display_disable_login(DisplayState *ds) -{ - VncDisplay *vs = vnc_display; - - if (!vs) { - return -1; - } - - if (vs->password) { - g_free(vs->password); - } - - vs->password = NULL; - if (vs->auth == VNC_AUTH_NONE) { - vs->auth = VNC_AUTH_VNC; - } - - return 0; -} - int vnc_display_password(DisplayState *ds, const char *password) { VncDisplay *vs = vnc_display; @@ -3003,20 +2983,18 @@ int vnc_display_password(DisplayState *ds, const char *password) if (!vs) { return -EINVAL; } - - if (!password) { - /* This is not the intention of this interface but err on the side - of being safe */ - return vnc_display_disable_login(ds); + if (vs->auth == VNC_AUTH_NONE) { + error_printf_unless_qmp("If you want use passwords please enable " + "password auth using '-vnc ${dpy},password'."); + return -EINVAL; } if (vs->password) { g_free(vs->password); vs->password = NULL; } - vs->password = g_strdup(password); - if (vs->auth == VNC_AUTH_NONE) { - vs->auth = VNC_AUTH_VNC; + if (password) { + vs->password = g_strdup(password); } return 0;
Current code silently changes the authentication settings in case you try to set a password without password authentication turned on. This is bad. Return an error instead. If we want allow changing auth settings at runtime this should be done explicitly using a separate monitor command, not as side effect of set_passwd. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- ui/vnc.c | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-)