@samitouri / QOSamiQemu / commits / 912f969e35

ui: add proper error reporting for password changes

Neither the VNC or SPICE code for password changes provides error reporting at source, leading the callers to report a largely useless generic error message. Fixing this removes one of the two remaining needs for the undesirable error_printf_unless_qmp() method. While fixing this the error message hint is improved to recommend the 'password-secret' option which allows securely passing a password at startup. Reported-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Daniel P. Berrangé committed Jan 14, 2026 at 11:49 UTC 912f969e35d07c4619272fa3dcbdbbc9e3785eae
8 files changed +43 -33
include/ui/console.h
+1 -1
@@ -457,7 +457,7 @@ void qemu_display_help(void);
457 void vnc_display_init(const char *id, Error **errp);
458 void vnc_display_open(const char *id, Error **errp);
459 void vnc_display_add_client(const char *id, int csock, bool skipauth);
460 -int vnc_display_password(const char *id, const char *password);
460 +int vnc_display_password(const char *id, const char *password, Error **errp);
461 int vnc_display_pw_expire(const char *id, time_t expires);
462 void vnc_parse(const char *str);
463 int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
include/ui/qemu-spice-module.h
+2 -1
@@ -29,7 +29,8 @@ struct QemuSpiceOps {
29 void (*display_init)(void);
30 int (*migrate_info)(const char *h, int p, int t, const char *s);
31 int (*set_passwd)(const char *passwd,
32 - bool fail_if_connected, bool disconnect_if_connected);
32 + bool fail_if_connected, bool disconnect_if_connected,
33 + Error **errp);
34 int (*set_pw_expire)(time_t expires);
35 int (*display_add_client)(int csock, int skipauth, int tls);
36 #ifdef CONFIG_SPICE
tests/functional/generic/test_vnc.py
+2 -2
@@ -48,7 +48,7 @@ class Vnc(QemuSystemTest):
48 self.assertEqual(set_password_response['error']['class'],
49 'GenericError')
50 self.assertEqual(set_password_response['error']['desc'],
51 - 'Could not set password')
51 + 'No VNC display is present');
52
53 def launch_guarded(self):
54 try:
@@ -73,7 +73,7 @@ class Vnc(QemuSystemTest):
73 self.assertEqual(set_password_response['error']['class'],
74 'GenericError')
75 self.assertEqual(set_password_response['error']['desc'],
76 - 'Could not set password')
76 + 'VNC password authentication is disabled')
77
78 def test_change_password(self):
79 self.set_machine('none')
ui/spice-core.c
+18 -7
@@ -757,7 +757,7 @@ static void qemu_spice_init(void)
757 tls_ciphers);
758 }
759 if (password) {
760 - qemu_spice.set_passwd(password, false, false);
760 + qemu_spice.set_passwd(password, false, false, NULL);
761 }
762 if (qemu_opt_get_bool(opts, "sasl", 0)) {
763 if (spice_server_set_sasl(spice_server, 1) == -1) {
@@ -920,8 +920,10 @@ int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con)
920 return qemu_spice_add_interface(&qxlin->base);
921 }
922
923 -static int qemu_spice_set_ticket(bool fail_if_conn, bool disconnect_if_conn)
923 +static int qemu_spice_set_ticket(bool fail_if_conn, bool disconnect_if_conn,
924 + Error **errp)
925 {
926 + int ret;
927 time_t lifetime, now = time(NULL);
928 char *passwd;
929
@@ -935,26 +937,35 @@ static int qemu_spice_set_ticket(bool fail_if_conn, bool disconnect_if_conn)
937 passwd = NULL;
938 lifetime = 1;
939 }
938 - return spice_server_set_ticket(spice_server, passwd, lifetime,
939 - fail_if_conn, disconnect_if_conn);
940 + ret = spice_server_set_ticket(spice_server, passwd, lifetime,
941 + fail_if_conn, disconnect_if_conn);
942 + if (ret < 0) {
943 + error_setg(errp, "Unable to set SPICE server ticket");
944 + return -1;
945 + }
946 + return 0;
947 }
948
949 static int qemu_spice_set_passwd(const char *passwd,
943 - bool fail_if_conn, bool disconnect_if_conn)
950 + bool fail_if_conn, bool disconnect_if_conn,
951 + Error **errp)
952 {
953 if (strcmp(auth, "spice") != 0) {
954 + error_setg(errp, "SPICE authentication is disabled");
955 + error_append_hint(errp,
956 + "To enable it use '-spice ...,password-secret=ID'");
957 return -1;
958 }
959
960 g_free(auth_passwd);
961 auth_passwd = g_strdup(passwd);
951 - return qemu_spice_set_ticket(fail_if_conn, disconnect_if_conn);
962 + return qemu_spice_set_ticket(fail_if_conn, disconnect_if_conn, errp);
963 }
964
965 static int qemu_spice_set_pw_expire(time_t expires)
966 {
967 auth_expires = expires;
957 - return qemu_spice_set_ticket(false, false);
968 + return qemu_spice_set_ticket(false, false, NULL);
969 }
970
971 static int qemu_spice_display_add_client(int csock, int skipauth, int tls)
ui/spice-module.c
+4 -3
@@ -45,14 +45,15 @@ static int qemu_spice_migrate_info_stub(const char *h, int p, int t,
45
46 static int qemu_spice_set_passwd_stub(const char *passwd,
47 bool fail_if_connected,
48 - bool disconnect_if_connected)
48 + bool disconnect_if_connected,
49 + Error **errp)
50 {
50 - return -1;
51 + g_assert_not_reached();
52 }
53
54 static int qemu_spice_set_pw_expire_stub(time_t expires)
55 {
55 - return -1;
56 + g_assert_not_reached();
57 }
58
59 static int qemu_spice_display_add_client_stub(int csock, int skipauth,
ui/ui-qmp-cmds.c
+6 -13
@@ -31,15 +31,14 @@
31
32 void qmp_set_password(SetPasswordOptions *opts, Error **errp)
33 {
34 - int rc;
35 -
34 if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
35 if (!qemu_using_spice(errp)) {
36 return;
37 }
40 - rc = qemu_spice.set_passwd(opts->password,
41 - opts->connected == SET_PASSWORD_ACTION_FAIL,
42 - opts->connected == SET_PASSWORD_ACTION_DISCONNECT);
38 + qemu_spice.set_passwd(opts->password,
39 + opts->connected == SET_PASSWORD_ACTION_FAIL,
40 + opts->connected == SET_PASSWORD_ACTION_DISCONNECT,
41 + errp);
42 } else {
43 assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
44 if (opts->connected != SET_PASSWORD_ACTION_KEEP) {
@@ -52,11 +51,7 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp)
51 * Note that setting an empty password will not disable login
52 * through this interface.
53 */
55 - rc = vnc_display_password(opts->u.vnc.display, opts->password);
56 - }
57 -
58 - if (rc != 0) {
59 - error_setg(errp, "Could not set password");
54 + vnc_display_password(opts->u.vnc.display, opts->password, errp);
55 }
56 }
57
@@ -107,9 +102,7 @@ void qmp_expire_password(ExpirePasswordOptions *opts, Error **errp)
102 #ifdef CONFIG_VNC
103 void qmp_change_vnc_password(const char *password, Error **errp)
104 {
110 - if (vnc_display_password(NULL, password) < 0) {
111 - error_setg(errp, "Could not set password");
112 - }
105 + vnc_display_password(NULL, password, errp);
106 }
107 #endif
108
ui/vnc-stubs.c
+3 -3
@@ -2,11 +2,11 @@
2 #include "ui/console.h"
3 #include "qapi/error.h"
4
5 -int vnc_display_password(const char *id, const char *password)
5 +int vnc_display_password(const char *id, const char *password, Error **errp)
6 {
7 - return -ENODEV;
7 + g_assert_not_reached();
8 }
9 int vnc_display_pw_expire(const char *id, time_t expires)
10 {
11 - return -ENODEV;
11 + g_assert_not_reached();
12 };
ui/vnc.c
+7 -3
@@ -3526,16 +3526,20 @@ static void vnc_display_close(VncDisplay *vd)
3526 #endif
3527 }
3528
3529 -int vnc_display_password(const char *id, const char *password)
3529 +int vnc_display_password(const char *id, const char *password, Error **errp)
3530 {
3531 VncDisplay *vd = vnc_display_find(id);
3532
3533 if (!vd) {
3534 + error_setg(errp, "No VNC display is present");
3535 + error_append_hint(errp,
3536 + "To enable it, use '-vnc ...'");
3537 return -EINVAL;
3538 }
3539 if (vd->auth == VNC_AUTH_NONE) {
3537 - error_printf_unless_qmp("If you want use passwords please enable "
3538 - "password auth using '-vnc ${dpy},password'.\n");
3540 + error_setg(errp, "VNC password authentication is disabled");
3541 + error_append_hint(errp,
3542 + "To enable it, use '-vnc ...,password-secret=ID'");
3543 return -EINVAL;
3544 }
3545