Re: [Xen-devel] [PATCH 3/3] domctl: tighten XEN_DOMCTL_*_permission

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 05/02/2014 04:19 AM, Ian Campbell wrote:
> On Fri, 2014-05-02 at 08:49 +0100, Jan Beulich wrote:
> > > > > On 30.04.14 at 19:17, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > On 30/04/14 15:24, Jan Beulich wrote:
> > > > --- a/xen/arch/x86/domctl.c
> > > > +++ b/xen/arch/x86/domctl.c
> > > > @@ -72,13 +72,11 @@ long arch_do_domctl(
> > > >           unsigned int np = domctl->u.ioport_permission.nr_ports;
> > > >           int allow = domctl->u.ioport_permission.allow_access;
> > > >
> > > > -        ret = -EINVAL;
> > > > -        if ( (fp + np) > 65536 )
> > > > -            break;
> > > > -
> > > > -        if ( np == 0 )
> > > > -            ret = 0;
> > > > -        else if ( xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, allow)
> > > )
> > > > +        if ( (fp + np - 1) < fp || (fp + np) > 0x10000 )
> > > > +            ret = -EINVAL;
> > > > +        else if ( !ioports_access_permitted(current->domain,
> > > > +                                            fp, fp + np - 1) ||
> > > > +                  xsm_ioport_permission(XSM_HOOK, d, fp, fp + np - 1, allow) )
> > > I don't see what the ioport permissions of the current domain have to do
> > > with whether a domain is permitted to change the permissions for another
> > > domain.
> > >
> > > I would expect that any domain builder domains would have no ioport
> > > permissions at all, which would cause this hypercall to unconditionally
> > > fail with -EPERM even if the domain builder domain is permitted to
> > > assign ioport permissions to the domain it is building.
> > My perspective is quite different - a building/controlling domain shouldn't
> > be able to assign any resources it doesn't itself have control over. Which
> > matches up with XEN_DOMCTL_{memory,ioport}_mapping, which too
> > check requester permissions before doing the permission assignment
> > (albeit there seems to be a tendency to agreement that the combination
> > of permission granting and mapping is undesirable).
> AIUI that's pretty much how Daniel explained it to me as well.
>
> Ian.
This is my understanding of the problem and the motivation for choosing
to require that the domain builder have access to resources it grants:

If a domain D has permission to use the XEN_DOMCTL_*_permission
hypercalls on a domain T, then D can try to grant T access to any
resource R on the platform.  If an XSM policy is used, the XSM policy
restricts the set of resources (T must have RESOURCE__USE on R), the
ability to grant access (D must have RESOURCE_ADD_{IOMEM,IOPORT,IRQ} on
R), and the ability to modify a given target (D must have RESOURCE__ADD
on T).  Removal has similar checks, but only on the domain D.

Because device model domains have access to XEN_DOMCTL_*_mapping, which
is a superset of XEN_DOMCTL_*_permission, in the absence of a tightly
written XSM policy, there need to be additional checks so that a device
model domain cannot grant its guest access to any resource on the
platform.  This was implemented by limiting the resources that a device
model domain could specify in a permission grant to resources that it
could access itself.

Extending these models to disaggregated domain building can be done in
two ways (at least):

Method 1: domain builder not restricted.  A domain builder does not need
to have access (as in ioports_access_permitted) to a resource in order
to grant a guest access to this resource.  The XSM policy may still
restrict the addition, as described above, but there are no additional
checks.  In particular, there is no check that the domain builder is not
granting access to any of the ranges that the hypervisor has removed
from domain 0's access list (on x86 this includes the PIC, PIT, ACPI PM
Timer, APIC, PCI configuration ports, ...).

Method 2: domain builder restricted by its own resource list.  This
is what is being done now; it requires that a domain builder have access
to any resources that a domain it creates could use.  Since the domain
builder is part of the TCB for any domain it builds, this is generally
considered harmless: a restricted domain builder could just create
sock-puppet domains and grant them the access it requires.

Creating an XSM policy that prevents sock-puppet domains is possible if
another trusted domain besides the domain builder is present.  Newly
created domains with access to important resources could be required to
be inspected prior to being unpaused for the first time.  Implementing
this type of check requires a stronger ability to revoke the access of
the domain builder than is currently provided by Xen: at the moment, a
malicious domain builder could refuse to unmap pages after building and
modify the domain after this inspection is complete.  Without the
ability to prevent sock-puppet domains, there is no real advantage to
using method 1 over method 2.

While a tightly crafted XSM policy is capable of restricting method 2 to
be just as secure as method 1, this is likely to make the XSM policy
platform-specific as numeric values of I/O memory ranges will need to be
encoded in the policy.  In some cases, a minor hardware or BIOS change
could cause resources to move around and would require changing the XSM
policy, which is not desirable.

--
Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel