Re: [Xen-devel] Xenstore domains and XS_RESTRICT

[Juergen Gross <jgross@xxxxxxxx>]

On 18/01/17 12:39, Wei Liu wrote:
> 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.

I'm not sure it is a good decision to let them use XS_RESTRICT. It is
an interface with weird consequences in some cases which are not
visible until some rare use cases (like hot-plugging a qdisk) are
effective.

> 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.

Today XS_RESTRICT is encapsulated by xs_restrict(). We could keep
this function and let it return false always. This way XS_RESTRICT
could be removed without breaking any current users as xs_restrict()
is returning false with xenstored today.

> 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.

I'm not concerned about my workload. :-)

I'd like to have a solution for the original problem (reduction of
possible problems due to a compromised device model) without having to
limit the overall functionality (Xenstore not capable to run as stubdom,
no qemu-based backends possible).

> 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.

Might be not so easy... :-(


Juergen


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