Re: [PATCH 1/1] MAINTAINERS: introduce cve or security quotient field

[Michael S. Tsirkin]

On Tue, Jul 14, 2020 at 10:52:33AM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 14, 2020 at 10:42:55AM +0100, Peter Maydell wrote:
> > On Tue, 14 Jul 2020 at 09:40, P J P <ppandit@redhat.com> wrote:
> > >
> > > From: Prasad J Pandit <pjp@fedoraproject.org>
> > >
> > > QEMU supports numerous virtualisation and emulation use cases.
> > > It also offers many features to support guest's function(s).
> > >
> > > All of these use cases and features are not always security relevant.
> > > Because some maybe used in trusted environments only. Some may still
> > > be in experimental stage. While other could be very old and not
> > > used or maintained actively.
> > >
> > > For security bug analysis we generally consider use cases wherein
> > > QEMU is used in conjunction with the KVM hypervisor, which enables
> > > guest to use hardware processor's virtualisation features.
> > >
> > > The CVE (or Security or Trust) Quotient field tries to capture this
> > > sensitivity pertaining to a feature or section of the code.
> > >
> > > It indicates whether a potential issue should be treated as a security
> > > one OR it could be fixed as a regular non-security bug.
> > 
> > How does this interact with the way we already document our
> > level of security support in docs/system/security.rst ?
> > 
> > > +       C: CVE/Security/Trust Quotient
> > > +          H:High - Feature (or code) is meant to be safe and used by 
> > > untrusted
> > > +                   guests. So any potential security issue must be 
> > > processed with
> > > +                   due care and be considered as a CVE issue.
> > > +          L:Low  - Feature (or code) is not meant to be safe OR is 
> > > experimental
> > > +                   OR is used in trusted environments only OR is not well
> > > +                   maintained. So any potential security issue can be 
> > > processed
> > > +                   and fixed as regular non-security bug. No need for a 
> > > CVE.
> > 
> > The difficulty with this is that MAINTAINERS is not set up
> > with a split between "security issues" and "non-security
> > issues". For instance this stanza:
> > 
> > > @@ -149,6 +161,7 @@ ARM TCG CPUs
> > >  M: Peter Maydell <peter.maydell@linaro.org>
> > >  L: qemu-arm@nongnu.org
> > >  S: Maintained
> > > +C: Low
> > >  F: target/arm/
> > >  F: tests/tcg/arm/
> > >  F: tests/tcg/aarch64/
> > 
> > you have marked "Low", but the files it covers include
> > both ones used by TCG (not security-critical) and ones
> > used by KVM (security-critical).
> > 
> > Also, MAINTAINERS is not user-facing. If we want to say
> > that vvfat or 9pfs are not suitable for use on a security
> > boundary and that we do not consider bugs in them to
> > be security issues, we should do that in the user-facing
> > documentation.
> > 
> > Broadly speaking, it feels like you're trying to come up
> > with an automatic way to say "does this patch touch a
> > security-relevant part of the code", and I'm not sure
> > that that's possible.
> 
> I agree that it isn't possible in the MAINTAINERS file, as the level
> of granularity is a very poor match for what we want to express.
> 
> My high level thought would be that we should ultimately be able to
> have a build flag to request only security-critical code is built
> into the binaries.

And for people who want to build QEMU with lots of functionality (like
Fedora does), I think a -security flag would be a useful addition.
We can then tell security researchers "only a high security issue
if it reproduces with -security=high, only a security issue
if it reproduces with -security=low".


> This is probably a bit too much of a stretch goal right now, but it
> at least points towards maintaining the information on a per-file
> level of granularity. There might be some individual files which
> currently contain a mix security-critical/not-security critical
> code. Either they can be split eventually, or we can simply declare
> that the entire file is none the less security critical.
> 
> We could perhaps have a magic comment at the top of each file that
> is security critical. eg
> 
>  /* @security: maintained */
> 
> we don't need any comment in files we consider non-maintained from
> a security POV.  Eventually we could do some (insert hand waving)
> magic in meson to pull out this list of comments and use it to
> exclude build of files that are not security critical. Maybe we
> find out that using a magic comment isn't the best option, but
> at least if we start now by keeping a per-file comment, we can
> probably do an automated transformation to any other data storage
> later.
> 
> 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 :|