mbox series

[00/14] Improve mechanism for configuring allowed commands

Message ID 20240604153242.251334-1-berrange@redhat.com (mailing list archive)
Headers show
Series Improve mechanism for configuring allowed commands | expand

Message

Daniel P. Berrangé June 4, 2024, 3:32 p.m. UTC
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(-)

Comments

Daniel P. Berrangé July 2, 2024, 6:09 p.m. UTC | #1
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
Markus Armbruster July 15, 2024, 9:52 a.m. UTC | #2
Hi Daniel, got a public branch I could pull?
Daniel P. Berrangé July 15, 2024, 10:56 a.m. UTC | #3
On Mon, Jul 15, 2024 at 11:52:10AM +0200, Markus Armbruster wrote:
> Hi Daniel, got a public branch I could pull?

This particular v1 posting:

   https://gitlab.com/berrange/qemu/-/tags/qga-features-v1

Or latest git master rebase

   https://gitlab.com/berrange/qemu/-/tree/qga-features

NB, this is on top of my other qga patch series changing build time
conditions, so the above branch includes this branch:

   https://gitlab.com/berrange/qemu/-/tree/qga-conditions

With regards,
Daniel