Re: Xenstore domains and XS_RESTRICT

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

On Wed, Jan 18, 2017 at 12:21:48PM +0100, Juergen Gross wrote:
> On 18/01/17 12:03, Wei Liu wrote:
> > On Mon, Jan 16, 2017 at 05:47:15PM +0100, Juergen Gross wrote:
> >> On 07/12/16 08:44, Juergen Gross wrote:
> >>> Hi,
> >>>
> >>> today the XS_RESTRICT wire command of Xenstore is supported by
> >>> oxenstored only to drop the privilege of a connection to that of the
> >>> domid given as a parameter to the command.
> >>>
> >>> Using this mechanism with Xenstore running in a stubdom will lead to
> >>> problems as instead of only a dom0 process dropping its privileges
> >>> the privileges of dom0 will be dropped (all dom0 Xenstore requests
> >>> share the same connection).
> >>>
> >>> In order to solve the problem I suggest the following change to the
> >>> Xenstore wire protocol:
> >>>
> >>>  struct xsd_sockmsg
> >>>  {
> >>> -    uint32_t type;  /* XS_??? */
> >>> +    uint16_t type;  /* XS_??? */
> >>> +    uint16_t domid; /* Use privileges of this domain */
> >>>      uint32_t req_id;/* Request identifier, echoed in daemon's response.  */
> >>>      uint32_t tx_id; /* Transaction id (0 if not related to a
> >>> transaction). */
> >>>      uint32_t len;   /* Length of data following this. */
> >>>
> >>>      /* Generally followed by nul-terminated string(s). */
> >>>  };
> >>>
> >>> domid will normally be zero having the same effect as today.
> >>>
> >>> Using XS_RESTRICT via a socket connection will run as today by dropping
> >>> the privileges of that connection.
> >>>
> >>> Using XS_RESTRICT via the kernel (Xenstore domain case) will save the
> >>> domid given as parameter in the connection specific private kernel
> >>> structure. All future Xenstore commands of the connection will have
> >>> this domid set in xsd_sockmsg. The kernel will never forward the
> >>> XS_RESTRICT command to Xenstore.
> >>>
> >>> A domid other than 0 in xsd_sockmsg will be handled by Xenstore to use
> >>> the privileges of that domain. Specifying a domid in xsd_sockmsg is
> >>> allowed for privileged domain only, of course. XS_RESTRICT via a
> >>> non-socket connection will be rejected in all cases.
> >>>
> >>> The needed modifications for Xenstore and the kernel are rather small.
> >>> As there is currently no Xenstore domain available supporting
> >>> XS_RESTRICT there are no compatibility issues to expect.
> >>>
> >>> Thoughts?
> >>
> >> As I don't get any further constructive responses even after asking for
> >> them: would patches removing all XS_RESTRICT support be accepted?
> >>
> > 
> > We don't need to actually remove it, do we? If XS_RESTRICT is not supported by
> > xenstored, the client would get meaningful error code. A patch to
> > deprecate that command should be good enough, right?
> 
> Uuh, no.
> 
> oxenstored does support XS_RESTRICT. The longer it stays the better the
> chances someone is using it.
> 

Right. That's what I'm getting at.

As a developer I'm in favour of ripping XS_RESTRICT out completely, but
as a maintainer I'm a bit uncomfortable with that...

If current users are happy with this limiting interface, let them use
it.  We just need to provide a better alternative for future users.

And even if we want to eventually remove it, we should try our best
provide an upgrade path. In this particular case, I think whatever
scheme we agree on is going to be a natural upgrade path. We can choose
to either keep XS_RESTRICT or remove it after that.

I know we're paying for passed mistakes, but the above plan doesn't seem
to increase your workload. I have the feeling that you're in favour of
working on something more adequate, and I'm in favour of that, too.

Does that make sense?

> > And sorry for the late reply, I'm still mulling over your proposal, I
> > will try to respond as soon as possible.
> 
> I thought a little bit further: the idea of XS_RESTRICT is to avoid qemu
> being capable to overwrite any Xenstore entries of other domains
> including dom0.
> 
> I fail to see how this should work with qemu-based backends (qdisk,
> pvusb), as those rely on paths in Xenstore writable by dom0 only.
> 
> We already have a mechanism to de-privilege the device model of a HVM
> domain without hurting the backends: ioemu-stubdom. So I believe we
> should try to make qmeu upstream usable in stubdom instead of
> introducing mechanisms limited in usability ("if you want a secure
> device model you can't use features x, y and z.").
> 

Right, we would like to see that happen, too. This is an useful thing in
and of itself.

Wei.

> 
> Juergen

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