Re: [PATCH 00/14] Improve mechanism for configuring allowed commands

[Daniel P . Berrangé]

Ping: any review comments from QGA maintainers ?

On Tue, Jun 04, 2024 at 04:32:28PM +0100, Daniel P. Berrangé wrote:
> The QGA supports dynamically filtering what commands are enabled via a
> combination of allow lists and deny lists. This is very flexible, but
> at the same time very fragile.
> 
> Consider that a user wants to block all commands that allow unrestricted
> file access/command execution, so they set the deny list when starting
> QGA. Now their OS vendor issues a software update which includes a new
> version of QGA. This new QGA version is liable to contain new commands,
> some of which might undermine the intent of the user's configured deny
> list.
> 
> IOW, the generic deny list functionality is inherently dangerous as a
> mechanism for limiting risk exposure.
> 
> Using an allow list is much safer, but means on every update the user
> should check the list of new commands to decide which are safe or not,
> putting a burden on every user.
> 
> In the context of RHEL, there has been a long term deny list that blocks
> use of guest-file and guest-exec commands, since they give unrestricted
> access to the guest.
> 
> With the advent of confidential computing, a far greater number of QGA
> commands are very unsafe to permit, and it is unreasonable to expect
> each user and/or downstream vendor to repeat the work to figure out
> what commands are OK.
> 
> This is a similar problem seen in the "seccomp" world where new syscalls
> appear frequently and users can't be expected to understand all of them.
> Systemd pioneered the approach of defining "profiles"  which group
> together sets of syscalls, which we subsequently copied in QEMU.
> 
> This series applies this same conceptual idea to QGA command filtering,
> making use of the QAPI "features" facility to associate commands into
> one or more groups.
> 
> This grouping is then exposed via some new higher level command line
> arguments.
> 
> * --no-unrestricted / -u
> 
>   A flag to block all the guest-file and guest-exec commands
> 
>   This replicates the policy RHEL currently defines via a deny list.
> 
> * --no-user-auth / -e
> 
>   A flag to block all the commands for manipulating user account
>   authentication credentials.
> 
> * --confidential / -i
> 
>   A flag to block all commands, except for those which have been
>   explicitly marked as not violating guest owner data privacy
> 
> This feature mechanism is further utilized internally to track the
> commands which are safe to use while FS are frozen.
> 
> A key benefit of using the QAPI "features" facility is that these
> groupings are visible in the documentation of the QGA commands.
> 
> By using these high level command lines arguments, deployments will
> be safe wrt software upgrades, as long as QEMU maintainers apply
> appropriate tags to any new commands.
> 
> The allow/deny list command line flags can still be used to further
> refine the command lines, but ideally that would be rare.
> 
> A missing piece in this series is getting the --confidential flag to
> be automatically passed to QGA when running in a confidential VM. This
> is something that will likely be done via systemd unit files. My thought
> is that the existing 'qemu-guest-agent.service' would get a parameter
> 
>    ConditionSecurity=!cvm
> 
> while a new qemu-guest-agent-confidential.service' would have the same
> content but with ConditionSecurity=cvm instead, and would pass the
> --confidential flag.
> 
> This series depends on the one I sent earlier:
> 
>   https://lists.nongnu.org/archive/html/qemu-devel/2024-06/msg00743.html
> 
> Daniel P. Berrangé (14):
>   qapi: use "QAPI_FEATURE" as namespace for special features
>   qapi: add helper for checking if a command feature is set
>   qapi: cope with special feature names containing a '-'
>   qapi: add a 'command-features' pragma
>   qapi: stop hardcoding list of special features
>   qapi: define enum for custom special features on commands
>   qga: use special feature to mark those that can run when FS are frozen
>   qga: add command line to limit commands for confidential guests
>   qga: define commands which can be run in confidential mode
>   qga: add command line to block unrestricted command/file access
>   qga: mark guest-file-* commands with 'unrestricted' flag
>   qga: mark guest-exec-* commands with 'unrestricted' flag
>   qga: add command line to block user authentication commands
>   qga: mark guest-ssh-* / guest-*-password commands with 'unrestricted'
>     flag
> 
>  include/qapi/qmp/dispatch.h   |   1 +
>  include/qapi/util.h           |   6 +-
>  qapi/qapi-util.c              |   4 +-
>  qapi/qmp-registry.c           |   5 +
>  qapi/qobject-output-visitor.c |   4 +-
>  qga/main.c                    |  66 ++++++---
>  qga/qapi-schema.json          | 248 +++++++++++++++++++++++++++++++---
>  scripts/qapi/commands.py      |  20 +++
>  scripts/qapi/gen.py           |   2 +-
>  scripts/qapi/parser.py        |   2 +
>  scripts/qapi/schema.py        |  33 ++++-
>  scripts/qapi/source.py        |   2 +
>  12 files changed, 341 insertions(+), 52 deletions(-)
> 
> -- 
> 2.45.1
> 

With 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 :|