Re: [Qemu-devel] [PATCH v6 0/7] Provide a QOM-based authorization API

[Daniel P. Berrange]

Ping, does anyone have further feedback about this series ?

On Tue, Jun 14, 2016 at 05:07:16PM +0100, Daniel P. Berrange wrote:
> This is a followup of previously posted work in 2.6 cycle:
> 
>  v1: https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04618.html
>  v2: https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg01454.html
>  v3: https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg02498.html
>  v4: https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg01661.html
>  v5: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg00485.html
> 
> Many years ago I was responsible for adding the 'qemu_acl' type
> and associated HMP commands. Looking back at it now, it is quite
> a poor facility with a couple of bad limitations. First, the
> responsibility for creating the ACLs was left with the QEMU network
> service (VNC server was only thing ever doing it). This meant you
> could not share ACLs across multiple services. Second, there was
> no way to populate ACLs on the command line, you had no choice but
> to use the HMP commands. Third, the API was hardcoded around the
> idea of an in-QEMU implementation, leaving no scope for plugging
> in alternative implementations backed by, for example, LDAP or PAM.
> 
> This series introduces a much better authorization API design
> to QEMU that addresses all these problems, and maintains back
> compatibility. It of course is based on the QOM framework, so
> that immediately gives us ability to create objects via the
> CLI, HMP or QMP. There is an abstract base clss "QAuthZ" which
> defines the basic API for QEMU network services to use, and a
> specific implementation "QAuthZ" simple which replicates the
> functionality of 'qemu_acl'. It is thus possible to add other
> impls, without changing any other part of QEMU in the future.
> Finally, the user is responsible for creating the ACL objects,
> so they can have one ACL associated with all their TLS enabled
> network services.
> 
> There was only one small problem with this, specifically the
> -object CLI arg and HMP 'object_add' command had no way to let
> the user specify non-scalar properties for objects. eg if an
> object had a property which is a list of structs, you are out
> of luck if you want to create it without using QMP.
> 
> Thus the first four patches do some work around QAPI / QOM
> to make it possible to specify non-scalar properties with
> the -object CLI arg and HMP 'object_add' command. See the
> respective patches for illustration of the syntax used. Some
> of Max's recent block patches also depend on the qdict_crumple
> method in patch 1.
> 
> The patches 5 and 6 introduce the new base class and specific
> implementation.
> 
> Patch 7 kills the old qemu_acl code, updating any existing
> callers of it to use the QAuthZSimple QOM class instead.
> 
> Previously there were further patches adding ACL support for
> chardevs, migration, nbd, etc. These will be posted later
> once this core code is merged, so they can flow via the
> respective maintainer's trees
> 
> Changed in v6:
> 
>  - Switch from while() to for() loop for iterating over
>    dicts (Markus)
>  - Avoid redundant strdup (Markus)
>  - Rewrap comments at 70 chars (Markus)
>  - Change qdict_list_size() to qdict_is_list() (Markus)
>  - Misc docs changes (Markus)
>  - Change QmpInputVisitor so the code for handling the
>    string types is separate from code using native
>    scalar types (Paolo)
>  - Centralize code parsing bool strings (Markus)
>  - Centralize code parsing int strings (Markus)
> 
> Changed in v5:
> 
>  - Resolved conflicts with Eric's visitor refactoring which
>    made it stricter about struct begin/end calls
>  - Added support for ACLs to migration code now its TLS
>    support is merged.
>  - Fixed typos in example in commit message
> 
> Changed in v4:
> 
>  - Ensure examples use shell escaping for '*' (Eric)
>  - Add more tests for crumple impl (Eric)
>  - Raise error if sasl-acl/tls-acl are requested but
>    sasl/tls auth are not enabled (Eric)
>  - Document return codes for auth check more clearly (Eric)
>  - Don't silently turn a glob match into a strcmp
>    if fnmatch is not present (Eric)
>  - Other misc small typos/fixes (Eric)
> 
> Changed in v3:
> 
>  - Created separate qdict_list_size method (Max)
>  - Added unit tests for case of empty dict (Max)
>  - Fix variable names to use underscore separator (Max)
>  - Fix potential free of uninitialized variables (Max)
>  - Use QObject APIs for casts, instead of C type casts (Max)
> 
> Changed in v2:
> 
>  - Adapt to changes in qapi visitor APIs
>  - Add a 'bool recursive' flag to qdict_crumple (Max)
>  - Fix memory leaks in qdict_crumple (Max)
>  - Split out key splitting code from qdict_crumple (Max)
>  - Use saner variable names in qdict_crumple (Max)
>  - Added some tests for bad inputs to qdict_crumple
> 
> Daniel P. Berrange (7):
>   qdict: implement a qdict_crumple method for un-flattening a dict
>   option: make parse_option_bool/number non-static
>   qapi: add a QmpInputVisitor that does string conversion
>   qom: support arbitrary non-scalar properties with -object
>   util: add QAuthZ object as an authorization base class
>   util: add QAuthZSimple object type for a simple access control list
>   acl: delete existing ACL implementation
> 
>  MAINTAINERS                      |   7 +
>  Makefile                         |   9 +-
>  Makefile.objs                    |   2 +
>  Makefile.target                  |   2 +
>  crypto/tlssession.c              |  28 +++-
>  hmp.c                            |  18 +--
>  include/qapi/qmp-input-visitor.h |  42 +++++-
>  include/qapi/qmp/qdict.h         |   1 +
>  include/qemu/acl.h               |  74 ---------
>  include/qemu/authz-simple.h      | 115 ++++++++++++++
>  include/qemu/authz.h             |  89 +++++++++++
>  include/qemu/option.h            |   4 +
>  include/qom/object_interfaces.h  |  10 +-
>  monitor.c                        | 181 ++++++++++++++--------
>  qapi-schema.json                 |   6 +-
>  qapi/opts-visitor.c              |  19 +--
>  qapi/qmp-input-visitor.c         |  80 ++++++++++
>  qapi/util.json                   |  47 ++++++
>  qmp.c                            |   2 +-
>  qobject/qdict.c                  | 283 ++++++++++++++++++++++++++++++++++
>  qom/object_interfaces.c          |  49 ++++--
>  tests/.gitignore                 |   1 +
>  tests/Makefile.include           |   5 +-
>  tests/check-qdict.c              | 241 +++++++++++++++++++++++++++++
>  tests/check-qom-proplist.c       | 319 
> ++++++++++++++++++++++++++++++++++++++-
>  tests/test-authz-simple.c        | 172 +++++++++++++++++++++
>  tests/test-crypto-tlssession.c   |  15 +-
>  tests/test-io-channel-tls.c      |  16 +-
>  tests/test-qmp-input-visitor.c   | 119 ++++++++++++++-
>  ui/vnc-auth-sasl.c               |   2 +-
>  ui/vnc-auth-sasl.h               |   4 +-
>  ui/vnc.c                         |  11 +-
>  util/Makefile.objs               |   4 +-
>  util/acl.c                       | 188 -----------------------
>  util/authz-simple.c              | 314 ++++++++++++++++++++++++++++++++++++++
>  util/authz.c                     |  46 ++++++
>  util/qemu-option.c               |  27 ++--
>  37 files changed, 2133 insertions(+), 419 deletions(-)
>  delete mode 100644 include/qemu/acl.h
>  create mode 100644 include/qemu/authz-simple.h
>  create mode 100644 include/qemu/authz.h
>  create mode 100644 qapi/util.json
>  create mode 100644 tests/test-authz-simple.c
>  delete mode 100644 util/acl.c
>  create mode 100644 util/authz-simple.c
>  create mode 100644 util/authz.c
> 
> -- 
> 2.5.5
> 

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|