Re: [Qemu-devel] [PATCH v6 11/11] authz: delete existing ACL implementation

[Daniel P . Berrangé]

On Thu, Nov 08, 2018 at 12:15:54PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Oct 19, 2018 at 5:51 PM Daniel P. Berrangé <address@hidden> wrote:
> >
> > From: "Daniel P. Berrange" <address@hidden>
> >
> > The 'qemu_acl' type was a previous non-QOM based attempt to provide an
> > authorization facility in QEMU. Because it is non-QOM based it cannot be
> > created via the command line and requires special monitor commands to
> > manipulate it.
> >
> > The new QAuthZ subclasses provide a superset of the functionality in
> > qemu_acl, so the latter can now be deleted. The HMP 'acl_*' monitor
> > commands are converted to use the new QAuthZSimple data type instead
> > in order to provide temporary backwards compatibility.
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > +    monitor_printf(mon, "policy: %s\n",
> > +                   QAuthZListPolicy_lookup.array[auth->policy]);
> 
> please use QAuthZListPolicy_str()
> 
> > +
> > +    rules = auth->rules;
> > +    while (rules) {
> > +        QAuthZListRule *rule = rules->value;
> > +        i++;
> > +        monitor_printf(mon, "%zu: %s %s\n", i,
> > +                       QAuthZListPolicy_lookup.array[rule->policy],
> 
> QAuthZListPolicy_str

Yes.


> > @@ -163,12 +165,19 @@ static int vnc_auth_sasl_check_access(VncState *vs)
> >      vs->sasl.username = g_strdup((const char*)val);
> >      trace_vnc_auth_sasl_username(vs, vs->sasl.username);
> >
> > -    if (vs->vd->sasl.acl == NULL) {
> > +    if (vs->vd->sasl.authzid == NULL) {
> >          trace_vnc_auth_sasl_acl(vs, 1);
> >          return 0;
> >      }
> >
> > -    allow = qemu_acl_party_is_allowed(vs->vd->sasl.acl, vs->sasl.username);
> > +    allow = qauthz_is_allowed_by_id(vs->vd->sasl.authzid,
> > +                                    vs->sasl.username, &err);
> 
> Why not use qauthz_is_allowed() with .authz ?

The .authz object is only non-NULL when using the legacy "-vnc ..,acl"
flag syntax. When using the modern syntax (introduced by the followup
series mentioned in the cover letter) we want to resolve "authzid"
every time. This allows the user to safely delete & recreate the
authorization objects on the fly.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|