All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: P J P <pjp@fedoraproject.org>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Thomas Huth" <thuth@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Eric Blake" <eblake@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [RFC PATCH 00/10] security: Introduce qemu_security_policy_taint() API
Date: Thu, 30 Sep 2021 11:30:27 +0100	[thread overview]
Message-ID: <YVWRw0ZKq3G6P5YP@redhat.com> (raw)
In-Reply-To: <798304472.4432617.1631626227208@mail.yahoo.com>

On Tue, Sep 14, 2021 at 01:30:27PM +0000, P J P wrote:
> Hello Philippe, all
> 
> >On Thursday, 9 September, 2021, 03:58:40 pm IST, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >On Thu, Sep 09, 2021 at 01:20:14AM +0200, Philippe Mathieu-Daudé wrote:
> >> This series is experimental! The goal is to better limit the
> >> boundary of what code is considerated security critical, and
> >> what is less critical (but still important!).
> >>
> >> This approach was quickly discussed few months ago with Markus
> >> then Daniel. Instead of classifying the code on a file path
> >> basis (see [1]), we insert (runtime) hints into the code
> >> (which survive code movement). Offending unsafe code can
> >> taint the global security policy. By default this policy
> >> is 'none': the current behavior. It can be changed on the
> >> command line to 'warn' to display warnings, and to 'strict'
> >> to prohibit QEMU running with a tainted policy.
> >
> 
> * Thanks so much for restarting this thread. I've been at it intermittently last few
>   months, thinking about how could we annotate the source/module objects.
> 
>    -> [*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04642.html
> 
> * Last time we discussed about having both compile and run time options for users
>   to be able to select the qualified objects/backends/devices as desired.

Right, we have multiple different use cases here

 - People building QEMU who want to cut down what they ship to
   minimize the stuff they support, which is outside the security
   guarantee. This can be OS distros, but also any other consumer
   of QEMU
   
   eg. RHEL wants to cut out almost all non-virtualization stuff.
   There is a quirk here though, that RHEL still includes TCG
   which is considered outside the security guarantee. So a
   simple build time "--secure on|off" doesn't do the job on
   its own.

   We need something to let people understand the consequences
   of the build options they are enabling.

   NB, when I talk of build options, I include both configure/meson
   args, and also the CONFIG_* options set in configs/**/*.mak


 - Application developers want to check that they're not using
   stuff outside the security guarantee, even if the distro
   has enable it.  These need to be able to query whether the
   VM they've launched has undesirable configuration choices.


Some people fall into both groups, some people fall into neither
group.


> * To confirm: How/Where is the security policy defined? Is it
>   device/module specific OR QEMU project wide?

Currently our only definition is in the docs

  https://qemu-project.gitlab.io/qemu/system/security.html#security-requirements

Philippe's patch is proposing tagging against internal QEMU objects
of various types.  I further proposed that we expose this in QMP
so it is introspectable.

I think there's scope for doing stuff at build time with configure
args and *mak CONFIG_* options, but haven't thought what that might
look like.

> > IOW, the reporting via QAPI introspetion is much more important
> > for libvirt and mgmt apps, than any custom cli arg / printfs
> > at the QEMU level.
> >
> 
> * True, while it makes sense to have a solution that is conversant with
>   the management/libvirt layers, It'll be great if we could address qemu/cli
>   other use cases too.
> 
> >it feels like we need
> >  'secure': 'bool'
> 
> * Though we started the (above [*]) discussion with '--security' option in mind,
>   I wonder if 'secure' annotation is much specific. And if we could widen its scope.
> --- x ---
> 
> 
> Source annotations: I've been thinking over following approaches
> ===================
> 
> 1) Segregate the QEMU sources under
> 
>       ../staging/      <= devel/experimental, not for production usage
>       ../src/          <= good for production usage, hence security relevant
>       ../deprecated/   <= Bad for production usage, not security relevant 
> 
>    - This is similar to Linux staging drivers' tree.
>    - Staging drivers are not considered for production usage and hence CVE allocation.
>    - At build time by default we only build sources under ../src/ tree.
>    - But we can still have options to build /staging/ and /deprecated/ trees.  
>    - It's readily understandable to end users.

I don't think we should be working in terms of source files at all.
Some files contain multiple distinct bits of functionality that are
not easily separated, and will have distinct security levels. Also
IMHO it is unpleasant to be moving files around in git to when code
switches between levels.  Also there are distinct criteria here,
both security levels, and support levels - there can be stuff which
is fully supported but considered insecure, and stuff that is
deprecated but considered secure. 

> 2) pkgconfig(1) way:
>    - If we could define per device/backend a configuration (.pc) file which is then used
>      at build/run time to decide which sources are suitable for the build or usage.
> 
>    - I'm trying to experiment with this.

For build time configuration, we have a pretty clear set of
toggles between the configure/meson options, and the CONFIG_*
make options.  I don't think we need to complicate things by
trying to add pkg-config into the mix here.

> 3) We annotate QEMU devices/backends/modules with macros which define its status.
>    It can then be used at build/run times to decide if it's suitable for usage.

What is what Philippe's patches are doing in essence

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



WARNING: multiple messages have this Message-ID (diff)
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: P J P <pjp@fedoraproject.org>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [RFC PATCH 00/10] security: Introduce qemu_security_policy_taint() API
Date: Thu, 30 Sep 2021 11:30:27 +0100	[thread overview]
Message-ID: <YVWRw0ZKq3G6P5YP@redhat.com> (raw)
In-Reply-To: <798304472.4432617.1631626227208@mail.yahoo.com>

On Tue, Sep 14, 2021 at 01:30:27PM +0000, P J P wrote:
> Hello Philippe, all
> 
> >On Thursday, 9 September, 2021, 03:58:40 pm IST, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >On Thu, Sep 09, 2021 at 01:20:14AM +0200, Philippe Mathieu-Daudé wrote:
> >> This series is experimental! The goal is to better limit the
> >> boundary of what code is considerated security critical, and
> >> what is less critical (but still important!).
> >>
> >> This approach was quickly discussed few months ago with Markus
> >> then Daniel. Instead of classifying the code on a file path
> >> basis (see [1]), we insert (runtime) hints into the code
> >> (which survive code movement). Offending unsafe code can
> >> taint the global security policy. By default this policy
> >> is 'none': the current behavior. It can be changed on the
> >> command line to 'warn' to display warnings, and to 'strict'
> >> to prohibit QEMU running with a tainted policy.
> >
> 
> * Thanks so much for restarting this thread. I've been at it intermittently last few
>   months, thinking about how could we annotate the source/module objects.
> 
>    -> [*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04642.html
> 
> * Last time we discussed about having both compile and run time options for users
>   to be able to select the qualified objects/backends/devices as desired.

Right, we have multiple different use cases here

 - People building QEMU who want to cut down what they ship to
   minimize the stuff they support, which is outside the security
   guarantee. This can be OS distros, but also any other consumer
   of QEMU
   
   eg. RHEL wants to cut out almost all non-virtualization stuff.
   There is a quirk here though, that RHEL still includes TCG
   which is considered outside the security guarantee. So a
   simple build time "--secure on|off" doesn't do the job on
   its own.

   We need something to let people understand the consequences
   of the build options they are enabling.

   NB, when I talk of build options, I include both configure/meson
   args, and also the CONFIG_* options set in configs/**/*.mak


 - Application developers want to check that they're not using
   stuff outside the security guarantee, even if the distro
   has enable it.  These need to be able to query whether the
   VM they've launched has undesirable configuration choices.


Some people fall into both groups, some people fall into neither
group.


> * To confirm: How/Where is the security policy defined? Is it
>   device/module specific OR QEMU project wide?

Currently our only definition is in the docs

  https://qemu-project.gitlab.io/qemu/system/security.html#security-requirements

Philippe's patch is proposing tagging against internal QEMU objects
of various types.  I further proposed that we expose this in QMP
so it is introspectable.

I think there's scope for doing stuff at build time with configure
args and *mak CONFIG_* options, but haven't thought what that might
look like.

> > IOW, the reporting via QAPI introspetion is much more important
> > for libvirt and mgmt apps, than any custom cli arg / printfs
> > at the QEMU level.
> >
> 
> * True, while it makes sense to have a solution that is conversant with
>   the management/libvirt layers, It'll be great if we could address qemu/cli
>   other use cases too.
> 
> >it feels like we need
> >  'secure': 'bool'
> 
> * Though we started the (above [*]) discussion with '--security' option in mind,
>   I wonder if 'secure' annotation is much specific. And if we could widen its scope.
> --- x ---
> 
> 
> Source annotations: I've been thinking over following approaches
> ===================
> 
> 1) Segregate the QEMU sources under
> 
>       ../staging/      <= devel/experimental, not for production usage
>       ../src/          <= good for production usage, hence security relevant
>       ../deprecated/   <= Bad for production usage, not security relevant 
> 
>    - This is similar to Linux staging drivers' tree.
>    - Staging drivers are not considered for production usage and hence CVE allocation.
>    - At build time by default we only build sources under ../src/ tree.
>    - But we can still have options to build /staging/ and /deprecated/ trees.  
>    - It's readily understandable to end users.

I don't think we should be working in terms of source files at all.
Some files contain multiple distinct bits of functionality that are
not easily separated, and will have distinct security levels. Also
IMHO it is unpleasant to be moving files around in git to when code
switches between levels.  Also there are distinct criteria here,
both security levels, and support levels - there can be stuff which
is fully supported but considered insecure, and stuff that is
deprecated but considered secure. 

> 2) pkgconfig(1) way:
>    - If we could define per device/backend a configuration (.pc) file which is then used
>      at build/run time to decide which sources are suitable for the build or usage.
> 
>    - I'm trying to experiment with this.

For build time configuration, we have a pretty clear set of
toggles between the configure/meson options, and the CONFIG_*
make options.  I don't think we need to complicate things by
trying to add pkg-config into the mix here.

> 3) We annotate QEMU devices/backends/modules with macros which define its status.
>    It can then be used at build/run times to decide if it's suitable for usage.

What is what Philippe's patches are doing in essence

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



  parent reply	other threads:[~2021-09-30 10:31 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 23:20 [RFC PATCH 00/10] security: Introduce qemu_security_policy_taint() API Philippe Mathieu-Daudé
2021-09-08 23:20 ` Philippe Mathieu-Daudé
2021-09-08 23:20 ` [RFC PATCH 01/10] sysemu: " Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-09 10:01   ` Paolo Bonzini
2021-09-09 18:45   ` Eric Blake
2021-09-09 18:45     ` Eric Blake
2021-09-08 23:20 ` [RFC PATCH 02/10] accel: Use qemu_security_policy_taint(), mark KVM and Xen as safe Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-09 10:37   ` Daniel P. Berrangé
2021-09-09 10:37     ` Daniel P. Berrangé
2021-10-21 14:47     ` Markus Armbruster
2021-10-21 14:47       ` Markus Armbruster
2021-09-09 18:46   ` Eric Blake
2021-09-09 18:46     ` Eric Blake
2021-09-08 23:20 ` [RFC PATCH 03/10] block: Use qemu_security_policy_taint() API Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-09  9:53   ` Philippe Mathieu-Daudé
2021-09-09  9:53     ` Philippe Mathieu-Daudé
2021-09-09 10:40   ` Daniel P. Berrangé
2021-09-09 10:40     ` Daniel P. Berrangé
2021-09-09 10:55     ` Daniel P. Berrangé
2021-09-09 19:05   ` Eric Blake
2021-09-09 19:05     ` Eric Blake
2021-09-08 23:20 ` [RFC PATCH 04/10] block/vvfat: Mark the driver as unsafe Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-08 23:20 ` [RFC PATCH 05/10] block/null: Mark 'read-zeroes=off' option " Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-08 23:20 ` [RFC PATCH 06/10] qdev: Use qemu_security_policy_taint() API Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-09 11:03   ` Daniel P. Berrangé
2021-09-09 11:03     ` Daniel P. Berrangé
2021-09-08 23:20 ` [RFC PATCH 07/10] hw/display: Mark ATI and Artist devices as unsafe Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-08 23:20 ` [RFC PATCH 08/10] hw/misc: Mark testdev " Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-08 23:20 ` [RFC PATCH 09/10] hw/net: Mark Tulip device " Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-08 23:20 ` [RFC PATCH 10/10] hw/sd: Mark sdhci-pci " Philippe Mathieu-Daudé
2021-09-08 23:20   ` Philippe Mathieu-Daudé
2021-09-09 10:28 ` [RFC PATCH 00/10] security: Introduce qemu_security_policy_taint() API Daniel P. Berrangé
2021-09-09 10:28   ` Daniel P. Berrangé
2021-09-14 13:30   ` P J P
2021-09-14 13:30     ` P J P
2021-09-28 11:39     ` P J P
2021-09-28 11:39       ` P J P
2021-09-30 10:30     ` Daniel P. Berrangé [this message]
2021-09-30 10:30       ` Daniel P. Berrangé
2021-09-09 12:03 ` Alexander Bulekov
2021-09-09 12:03   ` Alexander Bulekov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YVWRw0ZKq3G6P5YP@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=pjp@fedoraproject.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.