[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL obj
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name |
Date: |
Tue, 22 Mar 2016 15:38:14 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> The VNC server has historically had support for ACLs to check
> both the SASL username and the TLS x509 distinguished name.
> The VNC server was responsible for creating the initial ACL,
> and the client app was then responsible for populating it with
> rules using the HMP 'acl_add' command.
>
> This is not satisfactory for a variety of reasons. There is
> no way to populate the ACLs from the command line, users are
> forced to use the HMP. With multiple network services all
> supporting TLS and ACLs now, it is desirable to be able to
> define a single ACL that is referenced by all services.
>
> To address these limitations, two new options are added to the
> VNC server CLI. The 'tls-acl' option takes the ID of a QAuthZ
> object to use for checking TLS x509 distinguished names, and
> the 'sasl-acl' option takes the ID of another object to use for
> checking SASL usernames.
>
> In this example, we setup two ACLs. The first allows any client
> with a certificate issued by the 'RedHat' organization in the
> 'London' locality. The second ACL allows clients with either
> the 'address@hidden' or 'address@hidden' kerberos usernames.
> Both ACLs must pass for the user to be allowed.
>
> $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> endpoint=server,verify-peer=yes \
> -object authz-simple,id=acl0,policy=deny,\
> rules.0.match=O=RedHat,,L=London,rules.0.policy=allow \
> -object authz-simple,id=acl0,policy=deny,\
Umm, you can't reuse 'acl0' as the id.
> address@hidden,rules.0.policy=allow \
> address@hidden,rules.0.policy=allow \
> -vnc 0.0.0.0:1,tls-creds=tls0,tls-acl=tlsacl0,
> sasl,sasl-acl=saslacl0 \
And this fails because the ids don't exist. I think you meant
authz-simple,id=tlsacl0 in the first instance, and
authz-simple,id=saslacl0 in the second instance.
> ...other QEMU args...
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> ui/vnc.c | 73
> ++++++++++++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 60 insertions(+), 13 deletions(-)
>
> @@ -3670,6 +3680,21 @@ void vnc_display_open(const char *id, Error **errp)
> }
> }
> acl = qemu_opt_get_bool(opts, "acl", false);
> + tlsacl = qemu_opt_get(opts, "tls-acl");
> + if (acl && tlsacl) {
> + error_setg(errp, "'acl' option is mutually exclusive with the "
> + "'tls-acl' options");
> + goto fail;
> + }
> +
> +#ifdef CONFIG_VNC_SASL
> + saslacl = qemu_opt_get(opts, "sasl-acl");
> + if (acl && saslacl) {
> + error_setg(errp, "'acl' option is mutually exclusive with the "
> + "'sasl-acl' options");
> + goto fail;
> + }
> +#endif
Do we explicitly fail if sasl-acl was provided but CONFIG_VNC_SASL is
not defined? It looks here like you silently ignore it, which would not
be good.
> @@ -3710,19 +3737,39 @@ void vnc_display_open(const char *id, Error **errp)
> &error_abort);
> }
> #ifdef CONFIG_VNC_SASL
> - if (acl && sasl) {
> - char *aclname;
> + if (sasl) {
> + if (saslacl) {
> + Object *container, *acl;
> + container = object_get_objects_root();
> + acl = object_resolve_path_component(container, saslacl);
> + if (!acl) {
> + error_setg(errp, "Cannot find ACL %s", saslacl);
> + goto fail;
> + }
>
> - if (strcmp(vs->id, "default") == 0) {
> - aclname = g_strdup("vnc.username");
> - } else {
> - aclname = g_strdup_printf("vnc.%s.username", vs->id);
> - }
> - vs->sasl.acl =
> - QAUTHZ(qauthz_simple_new(aclname,
> - QAUTHZ_SIMPLE_POLICY_DENY,
> - &error_abort));
> - g_free(aclname);
> + if (!object_dynamic_cast(acl, TYPE_QAUTHZ)) {
> + error_setg(errp, "Object '%s' is not a QAuthZ subclass",
> + saslacl);
> + goto fail;
> + }
> + vs->sasl.acl = QAUTHZ(acl);
> + } else if (acl) {
> + char *aclname;
> +
> + if (strcmp(vs->id, "default") == 0) {
> + aclname = g_strdup("vnc.username");
> + } else {
> + aclname = g_strdup_printf("vnc.%s.username", vs->id);
> + }
> + vs->sasl.acl =
> + QAUTHZ(qauthz_simple_new(aclname,
> + QAUTHZ_SIMPLE_POLICY_DENY,
> + &error_abort));
> + g_free(aclname);
> + }
> + } else if (saslacl) {
> + error_setg(errp, "SASL ACL provided when SASL is disabled");
> + goto fail;
> }
> #endif
>
Again, the saslacl check is only mentioned inside the #if; what happens
when the #if is not compiled?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name, Daniel P. Berrange, 2016/03/10
[Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation, Daniel P. Berrange, 2016/03/10
[Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command, Daniel P. Berrange, 2016/03/10
Re: [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict, Eric Blake, 2016/03/21