| Message ID | 1457636396-24983-10-git-send-email-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 'joe@REDHAT.COM' or 'fred@REDHAT.COM' 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. > rules.0.match=fred@REDHAT.COM,rules.0.policy=allow \ > rules.0.match=joe@REDHAT.COM,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 <berrange@redhat.com> > --- > 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?
On Tue, Mar 22, 2016 at 03:38:14PM -0600, Eric Blake wrote: > 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 'joe@REDHAT.COM' or 'fred@REDHAT.COM' 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. > > > rules.0.match=fred@REDHAT.COM,rules.0.policy=allow \ > > rules.0.match=joe@REDHAT.COM,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. Heh, yeah, I really ought to try the examples I put in the commit message tomake sure they work :-) > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > 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. Yes, we should really raise the error unconditionally. > > @@ -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? Yeah, I should fix that. Regards, Daniel
diff --git a/ui/vnc.c b/ui/vnc.c index 324512d..7090f0b 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3261,6 +3261,12 @@ static QemuOptsList qemu_vnc_opts = { .name = "acl", .type = QEMU_OPT_BOOL, },{ + .name = "tls-acl", + .type = QEMU_OPT_STRING, + },{ + .name = "sasl-acl", + .type = QEMU_OPT_STRING, + },{ .name = "lossy", .type = QEMU_OPT_BOOL, },{ @@ -3483,6 +3489,10 @@ void vnc_display_open(const char *id, Error **errp) int saslErr; #endif int acl = 0; + const char *tlsacl; +#ifdef CONFIG_VNC_SASL + const char *saslacl; +#endif int lock_key_sync = 1; if (!vs) { @@ -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 share = qemu_opt_get(opts, "share"); if (share) { @@ -3699,7 +3724,9 @@ void vnc_display_open(const char *id, Error **errp) vs->non_adaptive = true; } - if (acl) { + if (tlsacl) { + vs->tlsaclname = g_strdup(tlsacl); + } else if (acl) { if (strcmp(vs->id, "default") == 0) { vs->tlsaclname = g_strdup("vnc.x509dname"); } else { @@ -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
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 'joe@REDHAT.COM' or 'fred@REDHAT.COM' 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,\ rules.0.match=fred@REDHAT.COM,rules.0.policy=allow \ rules.0.match=joe@REDHAT.COM,rules.0.policy=allow \ -vnc 0.0.0.0:1,tls-creds=tls0,tls-acl=tlsacl0, sasl,sasl-acl=saslacl0 \ ...other QEMU args... Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- ui/vnc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 13 deletions(-)