All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Andryuk <jandryuk@gmail.com>
To: Scott Davis <scottwd@gmail.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	"Scott Davis" <scott.davis@starlab.io>,
	"Ian Jackson" <iwj@xenproject.org>, "Wei Liu" <wl@xen.org>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Nick Rosbrook" <rosbrookn@ainfosec.com>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	"Juergen Gross" <jgross@suse.com>,
	"Daniel De Graaf" <dgdegra@tycho.nsa.gov>,
	"Daniel P . Smith" <dpsmith@apertussolutions.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2] tools/xl: Add device_model_stubdomain_init_seclabel option to xl.cfg
Date: Tue, 17 Aug 2021 21:15:19 -0400	[thread overview]
Message-ID: <CAKf6xpuAzbz-PP-k3oNGo-3_iXWhjBF=nm4zdDb16UqB8ov_Og@mail.gmail.com> (raw)
In-Reply-To: <edb72ed62c7c1154d5ed282e57e1750b6d79fa15.1627672412.git.scott.davis@starlab.io>

Hi, Scott

On Fri, Jul 30, 2021 at 3:35 PM Scott Davis <scottwd@gmail.com> wrote:
>
> This adds an option to the xl domain configuration syntax for specifying
> a build-time XSM security label for device-model stubdomains separate
> from the run-time label specified by 'device_model_stubdomain_seclabel'.
> Fields are also added to the 'libxl_domain_build_info' struct to contain
> the new information, and a new call to 'xc_flask_relabel_domain'
> inserted to affect the change at the appropriate time.
>
> The device-model stubdomain is relabeled at the same time as its guest,
> just before the guest is unpaused. This requires the stubdomain itself
> to be unpaused and run for a short time prior to being relabeled, but
> allows PCI device assignments specified in xl.cfg to be completed prior
> to relabeling. This choice allows the privileges required to perform
> assignments to be dropped in the relabeling.
>
> The implementation mirrors that of the 'seclabel' and 'init_seclabel'
> options for user domains. When all used in concert, this enables the

Drop all -> "When used in concert"?

> creation of security policies that minimize run-time privileges between
> the toolstack domain, device-model stubdomains, and user domains.
>
> Signed-off-by: Scott Davis <scott.davis@starlab.io>
> ---
> Changes in v2:
> - removed superfluous chanegs to libxl_dm.c
> - changed all security label lookup failures due to FLASK being disabled
>   from warnings to hard errors based on mailing list discussion

I think this should be a separate patch before this one, since it is
distinct from adding device_model_stubdomain_init_seclabel.

> - added discussion of relabel point to commit message

I was hoping for more people's thoughts on this.  I'm just going to
write down my thoughts on this to get them out there.

The non-stubdom case is very straightforward:
<-init-><-relabel-><-unpause-><-exec->

Relabeling before unpause is a nice delineation point.  Since the
guest hasn't run, it can't be mid-operation when the relabel happens.

The stubdom case as implemented by the patch is this:
Guest:
<---------init------------->|<-relabel-><-unpause-><-exec->
Stubdom:                    |
<-init--><-unpause-><-exec->|<-relabel-><-exec->

Here the stubdom runs for some time and then gets relabeled.

The alternative would be to relabel the stubdom before unpause and
then relabel the guest:
Guest:
<----------init---------------------->|<-relabel-><-unpause-><-exec->
Stubdom:                              |
<-init-><-relabel-><-unpause-><-exec--|--exec->

Here relabel then unpause is maintained for both guest and stubdom.

Relabeling the *running* stubdom gives me pause.  We don't have any
synchronization with the stubdom's & QEMU's state.  QEMU wrote
"running" to indicate it was up, but that doesn't say anything about
the rest of the kernel.  From the other side, the stubdom and QEMU
running and having the guest label change could be surprising.

Scott, you are using this, so I may be imagining concerns that don't
exist.  And you did mention your approach gives the PCI assignment
only to the stubdom init label, which could be a nice feature.  It's
just the lack of clear delineation for the stubdom that makes me
wonder about it.

Having said that, for your approach, I think the code looks good.

Regards,
Jason


  reply	other threads:[~2021-08-18  1:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 19:34 [PATCH v2] tools/xl: Add device_model_stubdomain_init_seclabel option to xl.cfg Scott Davis
2021-08-18  1:15 ` Jason Andryuk [this message]
2021-08-23 17:52   ` Scott Davis

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='CAKf6xpuAzbz-PP-k3oNGo-3_iXWhjBF=nm4zdDb16UqB8ov_Og@mail.gmail.com' \
    --to=jandryuk@gmail.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=dpsmith@apertussolutions.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jgross@suse.com \
    --cc=marmarek@invisiblethingslab.com \
    --cc=rosbrookn@ainfosec.com \
    --cc=scott.davis@starlab.io \
    --cc=scottwd@gmail.com \
    --cc=wl@xen.org \
    --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.