| Message ID | 20210107124657.650-1-changzihao1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vnc: add qmp to support change authz | expand |
Hi, > This patch add a new qmp 'change-vnc-authz' to support change the > tls/sasl authz of vm. If index='', unset tlsauthzid/sasl.authzid > { > "execute":"change-vnc-authz", > "arguments":{ > "index":"object-authz-id", > "type":"tls/sasl" > } > } > +## > +# @change-vnc-authz: > +# > +# Change the VNC server authz. > +# > +# @type: the new authz type to use with VNC authentication > +# @index: the new authz object index to use with VNC authentication > +# > +# Since: 5.2 > +# > +## > +{ 'command': 'change-vnc-authz', > + 'data': { 'type' : 'str', > + 'index': 'str'}, > + 'if': 'defined(CONFIG_VNC_SASL)' } type should be an enum I think. Also index should be an int (and possibly an optional argument so you can just not specify index to unset). take care, Gerd
On Thu, Jan 07, 2021 at 08:46:57PM +0800, Zihao Chang wrote: > This patch add a new qmp 'change-vnc-authz' to support change the > tls/sasl authz of vm. If index='', unset tlsauthzid/sasl.authzid > { > "execute":"change-vnc-authz", > "arguments":{ > "index":"object-authz-id", > "type":"tls/sasl" > } > } I'm not a fan of adding adhoc new commands for specific properties. It feels like that is just repeating the design mistakes we made for years with the CLI where we added countless -set-this-feature-too args. We realized this mistake and now have the general purpose "-display" arg. If we want to support changing properties on the fly in QMP, then I think we should do a general purpose "display_update" QMP command, to replace the existing 'set_pasword', 'expire_password', 'change-vnc-password' commands, and also enable this authz change. It can also enable changing the TLS credentials object, and the authentication scheme, and be extensible to any number of other VNC properties we might like to make configurable in the future. Similarly it would then be extensible for SPICE and other graphics backends. > > Signed-off-by: Zihao Chang <changzihao1@huawei.com> > --- > include/ui/console.h | 3 +++ > monitor/qmp-cmds.c | 10 ++++++++++ > qapi/ui.json | 16 ++++++++++++++++ > ui/vnc.c | 32 ++++++++++++++++++++++++++++++++ > 4 files changed, 61 insertions(+) > > diff --git a/include/ui/console.h b/include/ui/console.h > index 5dd21976a3..6b85546105 100644 > --- a/include/ui/console.h > +++ b/include/ui/console.h > @@ -441,6 +441,9 @@ int vnc_display_password(const char *id, const char *password); > int vnc_display_pw_expire(const char *id, time_t expires); > QemuOpts *vnc_parse(const char *str, Error **errp); > int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp); > +#ifdef CONFIG_VNC_SASL > +int vnc_change_authz(const char *id, const char *type, const char *index); > +#endif > > /* input.c */ > int index_from_key(const char *key, size_t key_length); > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c > index 34f7e75b7b..085aeb9bec 100644 > --- a/monitor/qmp-cmds.c > +++ b/monitor/qmp-cmds.c > @@ -289,6 +289,16 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg, > } > #endif /* !CONFIG_VNC */ > > +#ifdef CONFIG_VNC_SASL > +void qmp_change_vnc_authz(const char *type, const char *index, Error **errp) > +{ > + if (vnc_change_authz(NULL, type, index) < 0) { > + error_setg(errp, "Could not set authz, type:%s, index:%s", > + type, index); > + } > +} > +#endif > + > void qmp_change(const char *device, const char *target, > bool has_arg, const char *arg, Error **errp) > { > diff --git a/qapi/ui.json b/qapi/ui.json > index d08d72b439..37ddeabbd2 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1179,3 +1179,19 @@ > ## > { 'command': 'query-display-options', > 'returns': 'DisplayOptions' } > + > +## > +# @change-vnc-authz: > +# > +# Change the VNC server authz. > +# > +# @type: the new authz type to use with VNC authentication > +# @index: the new authz object index to use with VNC authentication > +# > +# Since: 5.2 > +# > +## > +{ 'command': 'change-vnc-authz', > + 'data': { 'type' : 'str', > + 'index': 'str'}, > + 'if': 'defined(CONFIG_VNC_SASL)' } > diff --git a/ui/vnc.c b/ui/vnc.c > index 7452ac7df2..f0809290a8 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -3276,6 +3276,38 @@ int vnc_display_password(const char *id, const char *password) > return 0; > } > > +#ifdef CONFIG_VNC_SASL > +int vnc_change_authz(const char *id, const char *type, const char *index) > +{ > + VncDisplay *vd = vnc_display_find(id); > + > + if (!vd) { > + return -EINVAL; > + } > + > + if (strcmp(type, "sasl") == 0) { > + g_free(vd->sasl.authzid); > + vd->sasl.authzid = NULL; > + > + if (strcmp(index, "") != 0) { > + vd->sasl.authzid = g_strdup(index); > + } > + } else if (strcmp(type, "tls") == 0) { > + g_free(vd->tlsauthzid); > + vd->tlsauthzid = NULL; > + > + if (strcmp(index, "") != 0) { > + vd->tlsauthzid = g_strdup(index); > + } > + } else { > + error_printf_unless_qmp("unsupport authz type: %s", type); > + return -EOPNOTSUPP; > + } > + > + return 0; > +} > +#endif > + > int vnc_display_pw_expire(const char *id, time_t expires) > { > VncDisplay *vd = vnc_display_find(id); > -- > 2.23.0 > > Regards, Daniel
Hi, On 2021/1/8 0:04, Gerd Hoffmann wrote: > Hi, > >> This patch add a new qmp 'change-vnc-authz' to support change the >> tls/sasl authz of vm. If index='', unset tlsauthzid/sasl.authzid >> { >> "execute":"change-vnc-authz", >> "arguments":{ >> "index":"object-authz-id", >> "type":"tls/sasl" >> } >> } > >> +## >> +# @change-vnc-authz: >> +# >> +# Change the VNC server authz. >> +# >> +# @type: the new authz type to use with VNC authentication >> +# @index: the new authz object index to use with VNC authentication >> +# >> +# Since: 5.2 >> +# >> +## >> +{ 'command': 'change-vnc-authz', >> + 'data': { 'type' : 'str', >> + 'index': 'str'}, >> + 'if': 'defined(CONFIG_VNC_SASL)' } > > type should be an enum I think. It is a good idea to set type to enum, I will fix it in the next version. > Also index should be an int (and possibly an optional argument so you > can just not specify index to unset). > Index is the id of authz object(myauthz, user can set it as they wanted), may not use int. -object authz-simple,id=myauthz,identity=test1 > take care, > Gerd > > . > BTW, Daniel P . Berrangé suggests that all property set QMP commands should be integrated as an general purpose "display_update" QMP command. That's really a good idea. This may take longer time to design and code. Will we not think about adding new property set QMP until the general purpose "display_update" QMP is ready？ https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg00941.html Thanks， Zihao
> >> +{ 'command': 'change-vnc-authz', > >> + 'data': { 'type' : 'str', > >> + 'index': 'str'}, > >> + 'if': 'defined(CONFIG_VNC_SASL)' } > > > > type should be an enum I think. > It is a good idea to set type to enum, I will fix it in the next version. > > > Also index should be an int (and possibly an optional argument so you > > can just not specify index to unset). > > > Index is the id of authz object(myauthz, user can set it as they wanted), may not use int. > -object authz-simple,id=myauthz,identity=test1 Name it "auth-id" then? "index" sounds like a number ... > BTW, Daniel P . Berrangé suggests that all property set QMP commands should be integrated > as an general purpose "display_update" QMP command. That's really a good idea. Makes sense indeed. take care, Gerd
diff --git a/include/ui/console.h b/include/ui/console.h index 5dd21976a3..6b85546105 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -441,6 +441,9 @@ int vnc_display_password(const char *id, const char *password); int vnc_display_pw_expire(const char *id, time_t expires); QemuOpts *vnc_parse(const char *str, Error **errp); int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp); +#ifdef CONFIG_VNC_SASL +int vnc_change_authz(const char *id, const char *type, const char *index); +#endif /* input.c */ int index_from_key(const char *key, size_t key_length); diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 34f7e75b7b..085aeb9bec 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -289,6 +289,16 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg, } #endif /* !CONFIG_VNC */ +#ifdef CONFIG_VNC_SASL +void qmp_change_vnc_authz(const char *type, const char *index, Error **errp) +{ + if (vnc_change_authz(NULL, type, index) < 0) { + error_setg(errp, "Could not set authz, type:%s, index:%s", + type, index); + } +} +#endif + void qmp_change(const char *device, const char *target, bool has_arg, const char *arg, Error **errp) { diff --git a/qapi/ui.json b/qapi/ui.json index d08d72b439..37ddeabbd2 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1179,3 +1179,19 @@ ## { 'command': 'query-display-options', 'returns': 'DisplayOptions' } + +## +# @change-vnc-authz: +# +# Change the VNC server authz. +# +# @type: the new authz type to use with VNC authentication +# @index: the new authz object index to use with VNC authentication +# +# Since: 5.2 +# +## +{ 'command': 'change-vnc-authz', + 'data': { 'type' : 'str', + 'index': 'str'}, + 'if': 'defined(CONFIG_VNC_SASL)' } diff --git a/ui/vnc.c b/ui/vnc.c index 7452ac7df2..f0809290a8 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3276,6 +3276,38 @@ int vnc_display_password(const char *id, const char *password) return 0; } +#ifdef CONFIG_VNC_SASL +int vnc_change_authz(const char *id, const char *type, const char *index) +{ + VncDisplay *vd = vnc_display_find(id); + + if (!vd) { + return -EINVAL; + } + + if (strcmp(type, "sasl") == 0) { + g_free(vd->sasl.authzid); + vd->sasl.authzid = NULL; + + if (strcmp(index, "") != 0) { + vd->sasl.authzid = g_strdup(index); + } + } else if (strcmp(type, "tls") == 0) { + g_free(vd->tlsauthzid); + vd->tlsauthzid = NULL; + + if (strcmp(index, "") != 0) { + vd->tlsauthzid = g_strdup(index); + } + } else { + error_printf_unless_qmp("unsupport authz type: %s", type); + return -EOPNOTSUPP; + } + + return 0; +} +#endif + int vnc_display_pw_expire(const char *id, time_t expires) { VncDisplay *vd = vnc_display_find(id);
This patch add a new qmp 'change-vnc-authz' to support change the tls/sasl authz of vm. If index='', unset tlsauthzid/sasl.authzid { "execute":"change-vnc-authz", "arguments":{ "index":"object-authz-id", "type":"tls/sasl" } } Signed-off-by: Zihao Chang <changzihao1@huawei.com> --- include/ui/console.h | 3 +++ monitor/qmp-cmds.c | 10 ++++++++++ qapi/ui.json | 16 ++++++++++++++++ ui/vnc.c | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+)