Re: [PATCH XEN v8 28/29] tools/libs/*: Introduce APIs to restrict handles to a specific domain.

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Jan 19, 2016 at 01:44:53PM +0000, Ian Campbell wrote:
> On Tue, 2016-01-19 at 13:24 +0000, Wei Liu wrote:
> > On Fri, Jan 15, 2016 at 01:23:07PM +0000, Ian Campbell wrote:
> > > These are intended to allow user space processes (in particular QEMU)
> > > to lock down all the handles at start of day and then drop the
> > > privileges which would allow them to open any new unrestricted handles
> > > (e.g. setuid or similar). This will reduce the privileges which taking
> > > over such a process would gain an attacker wrt other domains in the
> > > system.
> > > 
> > > These are currently unimplemented on all platforms, however the API
> > > semantics are defined as the basis for discussion, and so that
> > > consumers can rely on this interface always having been present rather
> > > than requiring compile time API checks.
> > > 
> > > It is expected that these will be implemented by adding new ioctl
> > > calls on the underlying driver and that the restrictions will be
> > > enforced at the kernel interface layer (most likely by the kernel
> > > itself).
> > > 
> > > For evtchn, foreignmemory, gnttab and gntshr this is hopefully
> > > reasonably straightforward.
> > > 
> > > For call it is not so clear cut. Clearly the kernel cannot enforce
> > > these restrictions for hypercalls which are not stable (domctl et al)
> > > so they can never be on the whitelist. It may also be that potential
> > > users would like to restrict the handle further than just a given
> > > target domain, i.e. to a specific set of functionality (e.g. "things a
> > > device model might reasonably do"). I think we will also need some way
> > > to discover whether a given set of interfaces is available to a
> > > restricted handle, in order to support the addition of new
> > > functionality.
> > > 
> > > Notes:
> > > 
> > > - On many (all?) platforms libxencall and libxenforeignmemory are
> > >   implemented by the same underlying privcmd driver. The platform
> > >   level ioctl interface should support restricting the handle to only
> > >   one or the other.
> > 
> > IIRC mini-os doesn't have ioctl. That would require some special
> > handling
> 
> The actual implementation of this functionality would be OS specific and
> therefore need to be in $os.c, where mini-os.c is under no obligation to
> use an ioctl if it doesn't want to.
> 
> The only reason it is done in the common code here is to avoid adding a
> dozen stubs prior to even one OS actually implementing this. I could add a
> norestrict.c to each lib, put the stub there and link it on all platforms,
> that would reduce the churn when someone comes to add the actual
> functionality.
> 

I don't think you need to do that. Doing this in common code is fine by
me.

> >  -- if we want to use the new API in qemu-trad, too.
> > We shall cross the bridge when we get there.
> > 
> > > - On platforms with multiple privilege mapping ioctl variants should
> > >   consider only allowing the newest/currently preferred one on a
> > >   restricted handle. e.g. on Linux this would allow
> > >   IOCTL_PRIVCMD_MMAPBATCH_V2 but not IOCTL_PRIVCMD_MMAPBATCH. (Of
> > >   course any subsequently introduced _V3 would be subject to
> > >   compatibility concerns)
> > > 
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > [...]
> > >  /*
> > > + * Attempt to restrict the given xcall handle to only be able to
> > > + * target the given domain.
> > > + *
> > > + * On success returns 0, after which only hypercalls which are on a
> > > + * platform specific whitelist can be called and the arguments will be
> > > + * audited by the platform to ensure that the target domain is
> > > + * domid.
> > > + *
> > > + * Subsequent attempts to call any hypercall not on the platform
> > > + * specific whitelist will return -1 setting errno to ENOSYS.
> > > + *
> > > + * Subsequent attempts to call any hypercall on the platform specific
> > > + * whitelist with any other target domain return -1 setting errno to
> > > + * EPERM.
> > > + *
> > > + * These restrictions will be implemented by the platform in a way
> > > + * which cannot be circumvented by a userspace process. Further
> > > + * privilege drops (such as using setuid(2) etc) may also be required
> > > + * to prevent a compromised process from simply opening a second
> > > + * handle
> > > + *
> > > + * XXX which hypercalls are restricted, per platform list, do we need
> > > + * a way to probe? Do we want to be able to restrict to particular
> > > + * subsets of whitelisted hypercalls?
> > > + *
> > 
> > TBH given the semantics of this call is not yet clear I don't think we
> > should rush committing this interface.
> 
> The intention was to try and get enough confidence that we could include
> the call in the initial implementation such that applications could
> unconditionally use it in the future.
> 
> If we can't manage a sufficient level of confidence in the proposed
> interface then we should skip it for now of course.
> 

Let's see what other people think about this particular function.

Wei.

> Ian.