Re: [Xen-devel] Xenstore domains and XS_RESTRICT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/01/17 12:08, Juergen Gross wrote:
> 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 don't think XS_RESTRICT is actually used by anyone.

It was added to oxenstored in the dim and distant past, but nothing I
can find in XenServer uses it.  In particular, its intended usecase (for
deprivileging qemu) doesn't work because qemu currently needs access to
dom0 keys to work.

I'd tentatively rip it out.  No point keeping unused broken
functionality around and getting in the way of fixing the problem properly.

~Andrew

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