Xenstore domains and XS_RESTRICT

Xenstore domains and XS_RESTRICT

[Juergen Gross]

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?


Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Konrad Rzeszutek Wilk]

On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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).

.. which means we can't create new XenStore entries or save
off all the XenStore entries?
> 
> 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

Xenstore domain case? As in Linux kernel running the XenStore as
an stubdomain?

No, that can't be it. I think you mean that the kernel will have
an priviligied connection all the time?

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

Um, but couldn't a malicious guest decide to craft such packet?
> 
> 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?

I think I need to wrap my head about your use-case? Could you enumerate
what it is?

Thanks.
> 
> 
> Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 07/12/16 15:15, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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).
> 
> .. which means we can't create new XenStore entries or save
> off all the XenStore entries?

Example: qemu for domid 4 is dropping its privilege to that of domid 4
with xenstore running as domain (in contrast to xenstored). If we don't
do anything about it dom0 will only be capable to access xenstore with
the privileges domid 4 has (e.g. creation of entries outside of
/local/domain/4/ will be impossible).

>>
>> 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
> 
> Xenstore domain case? As in Linux kernel running the XenStore as
> an stubdomain?

Xenstore is running in a stubdom and thus dom0 is issuing xenstore
commands via the kernel (xenbus driver) to the xenstore domain.

> No, that can't be it. I think you mean that the kernel will have
> an priviligied connection all the time?

No. the kernel will have just one connection. Privilege is derived
from domid (dom0 -> 0).

>> 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.
> 
> Um, but couldn't a malicious guest decide to craft such packet?

There is no socket connection to xenstore domain.

>>
>> 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?
> 
> I think I need to wrap my head about your use-case? Could you enumerate
> what it is?

Xenstore isn't running as a daemon in dom0, but as a domain. All domains
including dom0 have to connect via xenbus. This means all agents in dom0
accessing xenstore share one connection.

In case xenstore is a daemon in dom0 each agent in dom0 accessing
xenstore will open an own socket connection to the daemon, so they can
drop their privileges independent from each other.


Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Konrad Rzeszutek Wilk]

On Wed, Dec 07, 2016 at 03:26:38PM +0100, Juergen Gross wrote:
> On 07/12/16 15:15, Konrad Rzeszutek Wilk wrote:
> > On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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).
> > 
> > .. which means we can't create new XenStore entries or save
> > off all the XenStore entries?
> 
> Example: qemu for domid 4 is dropping its privilege to that of domid 4
> with xenstore running as domain (in contrast to xenstored). If we don't
> do anything about it dom0 will only be capable to access xenstore with
> the privileges domid 4 has (e.g. creation of entries outside of
> /local/domain/4/ will be impossible).

But .. why? Shouldn't it have RWX access to its tree? I should
read up on XS_RESTRICT.

> 
> >>
> >> 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
> > 
> > Xenstore domain case? As in Linux kernel running the XenStore as
> > an stubdomain?
> 
> Xenstore is running in a stubdom and thus dom0 is issuing xenstore
> commands via the kernel (xenbus driver) to the xenstore domain.
> 
> > No, that can't be it. I think you mean that the kernel will have
> > an priviligied connection all the time?
> 
> No. the kernel will have just one connection. Privilege is derived
> from domid (dom0 -> 0).

What if the XenBus Mirage is the first domain? You could have
have it part of the .init section in Xen and it would be started as
the first domain.

Depending on the value of domid I think is incorrect.

> 
> >> 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.
> > 
> > Um, but couldn't a malicious guest decide to craft such packet?
> 
> There is no socket connection to xenstore domain.

Right but it creates its own XenStore ring. Can it send this xsd_sockmsg
with domid_id of zero? Or are you saying this is irrelevant becasue
what you are interested is for the Linux kernel to filter certain
xsd_sockmsg so it won't do something silly?

> 
> >>
> >> 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?
> > 
> > I think I need to wrap my head about your use-case? Could you enumerate
> > what it is?
> 
> Xenstore isn't running as a daemon in dom0, but as a domain. All domains
> including dom0 have to connect via xenbus. This means all agents in dom0
> accessing xenstore share one connection.
> 
> In case xenstore is a daemon in dom0 each agent in dom0 accessing
> xenstore will open an own socket connection to the daemon, so they can
> drop their privileges independent from each other.

OK, so this all sounds like the OS needs to mediate access? Sorry for
being so dense this morning.

This would imply that the kernel driver would need to understand
the format and disallow the XS_RESTRICT under certain conditions?

That seems reasonable.
> 
> 
> Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 07/12/16 16:40, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 07, 2016 at 03:26:38PM +0100, Juergen Gross wrote:
>> On 07/12/16 15:15, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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).
>>>
>>> .. which means we can't create new XenStore entries or save
>>> off all the XenStore entries?
>>
>> Example: qemu for domid 4 is dropping its privilege to that of domid 4
>> with xenstore running as domain (in contrast to xenstored). If we don't
>> do anything about it dom0 will only be capable to access xenstore with
>> the privileges domid 4 has (e.g. creation of entries outside of
>> /local/domain/4/ will be impossible).
> 
> But .. why? Shouldn't it have RWX access to its tree? I should
> read up on XS_RESTRICT.

That would help, yes. :-)

With XS_RESTRICT a privileged connection can drop its privileges to
those of a specific domain. This is meant to be used e.g. by qemu
together with dropping root privileges after initialization to avoid
causing damage to odom0/other domains due to security leaks.

> 
>>
>>>>
>>>> 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
>>>
>>> Xenstore domain case? As in Linux kernel running the XenStore as
>>> an stubdomain?
>>
>> Xenstore is running in a stubdom and thus dom0 is issuing xenstore
>> commands via the kernel (xenbus driver) to the xenstore domain.
>>
>>> No, that can't be it. I think you mean that the kernel will have
>>> an priviligied connection all the time?
>>
>> No. the kernel will have just one connection. Privilege is derived
>> from domid (dom0 -> 0).
> 
> What if the XenBus Mirage is the first domain? You could have
> have it part of the .init section in Xen and it would be started as
> the first domain.
> 
> Depending on the value of domid I think is incorrect.

It is correct. The comparison done is to either 0 or privileged domain
(set via start parameter of xenstore domain/daemon).

> 
>>
>>>> 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.
>>>
>>> Um, but couldn't a malicious guest decide to craft such packet?
>>
>> There is no socket connection to xenstore domain.
> 
> Right but it creates its own XenStore ring. Can it send this xsd_sockmsg
> with domid_id of zero? Or are you saying this is irrelevant becasue
> what you are interested is for the Linux kernel to filter certain
> xsd_sockmsg so it won't do something silly?

The domid has to be correct for being able to get a grant for its ring.
So it isn't transferred via the ring of the new domain, but from dom0
via the XS_INTRODUCE wire command (which is allowed for the privileged
domain only).

> 
>>
>>>>
>>>> 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?
>>>
>>> I think I need to wrap my head about your use-case? Could you enumerate
>>> what it is?
>>
>> Xenstore isn't running as a daemon in dom0, but as a domain. All domains
>> including dom0 have to connect via xenbus. This means all agents in dom0
>> accessing xenstore share one connection.
>>
>> In case xenstore is a daemon in dom0 each agent in dom0 accessing
>> xenstore will open an own socket connection to the daemon, so they can
>> drop their privileges independent from each other.
> 
> OK, so this all sounds like the OS needs to mediate access? Sorry for
> being so dense this morning.
> 
> This would imply that the kernel driver would need to understand
> the format and disallow the XS_RESTRICT under certain conditions?

Yes. As it does today for start/end of transactions and for setting
watches.

> That seems reasonable.

I hope so. :-)


Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("Re: Xenstore domains and XS_RESTRICT"):
> On Wed, Dec 07, 2016 at 03:26:38PM +0100, Juergen Gross wrote:
> > There is no socket connection to xenstore domain.
> 
> Right but it creates its own XenStore ring. Can it send this xsd_sockmsg
> with domid_id of zero? Or are you saying this is irrelevant becasue
> what you are interested is for the Linux kernel to filter certain
> xsd_sockmsg so it won't do something silly?

The latter.

> OK, so this all sounds like the OS needs to mediate access? Sorry for
> being so dense this morning.

The OS already needs to mediate access for all xenstore commands.  The
kernel xenbus driver has a list of the commands.  Some of them it will
"simply" proxy, translating request id and transaction fields, as
applicable.  Some of them it does something special for.  Unknown
commands are rejected.

> This would imply that the kernel driver would need to understand
> the format and disallow the XS_RESTRICT under certain conditions?

There should be a way to tell the kernel driver that the connection
should be restricted.  XS_RESTRICT is as good as any.

But the XS_RESTRICT command must not be forwarded by the kernel proxy
to the real xenstore.  Rather, the driver must make an annotation in
its client struct instead.

That annotation should result in _every_ proxied xenstore command,
from that client, being decorated so as to specify the restriction
domain.

There needs to be an extension to the xenstore protocol to support
this.

Ian.

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

Re: Xenstore domains and XS_RESTRICT

[Ian Jackson]

Juergen Gross writes ("Xenstore domains and XS_RESTRICT"):
> 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 */

I don't think this is a particularly nice extension.  (Also the way
you have written it has an endianness hazard.)  What if we decide, at
some future point, to support domids >2^16 ?  That's hard right now
but I don't want to make it harder.

I suggest the following protocol extension instead:

Add to xenstore.txt (the list of xenstore commands):

  RESTRICTCMD	<domid|><realcommand|><realpayload|>

	Here <domid|> is a domain id, and <realcommand|> is a command
        number; both of these are 32-bit integers in host byte order.

	This is an instruction to execute the command <realcommand|>
        with payload <realpayload|>.  However, all permissions
        checking should be done as if the command had been issued by
        domain <domid|>.

        The req_id and tx_id, and (if the command affects watches) any
        watches that are manipulated, are those of the calling
        connection.  So the reply is sent to the xenstore client
        (usually, ring client) which sent the RESTRICTCMD command.

        If RESTRICTCMD is used to invoke WATCH, the <domid|> from the
        RESTRICTCMD is attached to the watch, in xenstored.  Insofar
        as watch events are filtered by the permission system, the
        <domid|> from the RESTRICTCMD is used for watch events
        originating from such a watch.  But the actual watch events
        are sent to the connection that called RESTRICTCMD.

	This is similar in semantics to RESTRICT but applies to this
	one particular command (and its effects), only.

        RESTRICTCMD has no effect on, and is not visible through, the
        xenstore ring connection of domain <domid|> (if that domain
        has one).

What do you think ?

There is a command length limit implication but I don't think that's
important.

Ian.

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 07/12/16 18:00, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("Re: Xenstore domains and XS_RESTRICT"):
>> On Wed, Dec 07, 2016 at 03:26:38PM +0100, Juergen Gross wrote:
>>> There is no socket connection to xenstore domain.
>>
>> Right but it creates its own XenStore ring. Can it send this xsd_sockmsg
>> with domid_id of zero? Or are you saying this is irrelevant becasue
>> what you are interested is for the Linux kernel to filter certain
>> xsd_sockmsg so it won't do something silly?
> 
> The latter.
> 
>> OK, so this all sounds like the OS needs to mediate access? Sorry for
>> being so dense this morning.
> 
> The OS already needs to mediate access for all xenstore commands.  The
> kernel xenbus driver has a list of the commands.  Some of them it will
> "simply" proxy, translating request id and transaction fields, as
> applicable.  Some of them it does something special for.  Unknown
> commands are rejected.

Sorry, this isn't true. There is special handling for watch/unwatch and
transaction start/end. There is no other command filtering involved,
especially no rejection of unknown commands.

> 
>> This would imply that the kernel driver would need to understand
>> the format and disallow the XS_RESTRICT under certain conditions?
> 
> There should be a way to tell the kernel driver that the connection
> should be restricted.  XS_RESTRICT is as good as any.
> 
> But the XS_RESTRICT command must not be forwarded by the kernel proxy
> to the real xenstore.  Rather, the driver must make an annotation in
> its client struct instead.
> 
> That annotation should result in _every_ proxied xenstore command,
> from that client, being decorated so as to specify the restriction
> domain.
> 
> There needs to be an extension to the xenstore protocol to support
> this.

Right.


Juergen


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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 07/12/16 18:10, Ian Jackson wrote:
> Juergen Gross writes ("Xenstore domains and XS_RESTRICT"):
>> 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 */
> 
> I don't think this is a particularly nice extension.  (Also the way
> you have written it has an endianness hazard.)  What if we decide, at
> some future point, to support domids >2^16 ?  That's hard right now
> but I don't want to make it harder.

Do we support any big endian systems? I don't think so. An endianess
problem could occur only if there are any big endian systems using the
old (current) struct xsd_sockmsg.

Regarding size of domid: this is true.

> 
> I suggest the following protocol extension instead:
> 
> Add to xenstore.txt (the list of xenstore commands):
> 
>   RESTRICTCMD	<domid|><realcommand|><realpayload|>
> 
> 	Here <domid|> is a domain id, and <realcommand|> is a command
>         number; both of these are 32-bit integers in host byte order.

Hmm, normally parameters outside the command header are transferred
using ASCII representation. Do we really want to introduce the first
exception?

> 
> 	This is an instruction to execute the command <realcommand|>
>         with payload <realpayload|>.  However, all permissions
>         checking should be done as if the command had been issued by
>         domain <domid|>.
> 
>         The req_id and tx_id, and (if the command affects watches) any
>         watches that are manipulated, are those of the calling
>         connection.  So the reply is sent to the xenstore client
>         (usually, ring client) which sent the RESTRICTCMD command.
> 
>         If RESTRICTCMD is used to invoke WATCH, the <domid|> from the
>         RESTRICTCMD is attached to the watch, in xenstored.  Insofar
>         as watch events are filtered by the permission system, the
>         <domid|> from the RESTRICTCMD is used for watch events
>         originating from such a watch.  But the actual watch events
>         are sent to the connection that called RESTRICTCMD.
> 
> 	This is similar in semantics to RESTRICT but applies to this
> 	one particular command (and its effects), only.
> 
>         RESTRICTCMD has no effect on, and is not visible through, the
>         xenstore ring connection of domain <domid|> (if that domain
>         has one).
> 
> What do you think ?
> 
> There is a command length limit implication but I don't think that's
> important.

That was the first problem coming to my mind. OTOH I'm not sure this
is really a problem as users of RESTRICTCMD should have no need to
use payloads of 4k.

In summary I see advantages for both solutions. IMO the needed changes
for my solution will be a little bit smaller, though.


Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 08/12/16 08:55, Juergen Gross wrote:
> On 07/12/16 18:10, Ian Jackson wrote:
>> Juergen Gross writes ("Xenstore domains and XS_RESTRICT"):
>>> 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 */
>>
>> I don't think this is a particularly nice extension.  (Also the way
>> you have written it has an endianness hazard.)  What if we decide, at
>> some future point, to support domids >2^16 ?  That's hard right now
>> but I don't want to make it harder.
> 
> Do we support any big endian systems? I don't think so. An endianess
> problem could occur only if there are any big endian systems using the
> old (current) struct xsd_sockmsg.
> 
> Regarding size of domid: this is true.
> 
>>
>> I suggest the following protocol extension instead:
>>
>> Add to xenstore.txt (the list of xenstore commands):
>>
>>   RESTRICTCMD	<domid|><realcommand|><realpayload|>
>>
>> 	Here <domid|> is a domain id, and <realcommand|> is a command
>>         number; both of these are 32-bit integers in host byte order.
> 
> Hmm, normally parameters outside the command header are transferred
> using ASCII representation. Do we really want to introduce the first
> exception?
> 
>>
>> 	This is an instruction to execute the command <realcommand|>
>>         with payload <realpayload|>.  However, all permissions
>>         checking should be done as if the command had been issued by
>>         domain <domid|>.
>>
>>         The req_id and tx_id, and (if the command affects watches) any
>>         watches that are manipulated, are those of the calling
>>         connection.  So the reply is sent to the xenstore client
>>         (usually, ring client) which sent the RESTRICTCMD command.
>>
>>         If RESTRICTCMD is used to invoke WATCH, the <domid|> from the
>>         RESTRICTCMD is attached to the watch, in xenstored.  Insofar
>>         as watch events are filtered by the permission system, the
>>         <domid|> from the RESTRICTCMD is used for watch events
>>         originating from such a watch.  But the actual watch events
>>         are sent to the connection that called RESTRICTCMD.
>>
>> 	This is similar in semantics to RESTRICT but applies to this
>> 	one particular command (and its effects), only.
>>
>>         RESTRICTCMD has no effect on, and is not visible through, the
>>         xenstore ring connection of domain <domid|> (if that domain
>>         has one).
>>
>> What do you think ?
>>
>> There is a command length limit implication but I don't think that's
>> important.
> 
> That was the first problem coming to my mind. OTOH I'm not sure this
> is really a problem as users of RESTRICTCMD should have no need to
> use payloads of 4k.
> 
> In summary I see advantages for both solutions. IMO the needed changes
> for my solution will be a little bit smaller, though.

So we have no resolution which approach should be selected: mine or
Ian's (or any other?).

Any preferences?


Juergen


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

Re: Xenstore domains and XS_RESTRICT

[Wei Liu]

On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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.
> 

I'm slightly concerned about this non-unified model -- I would rather
xenstored only needs to deal with "virtual connection", regardless of
the interface (socket or kernel) they use.

I'm not convinced that socket-based distinction is good enough -- what
if some program tries to multiplex the socket? Is that not possible at
the moment?

(I can't find XS_RESTRICT in xenstore.txt, so I'm not sure if I'm
talking nonsense)

> 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?
> 
> 
> Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 04/01/17 15:59, Wei Liu wrote:
> On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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.
>>
> 
> I'm slightly concerned about this non-unified model -- I would rather
> xenstored only needs to deal with "virtual connection", regardless of
> the interface (socket or kernel) they use.

Rejecting XS_RESTRICT for a non-socket connection is mandatory to
ensure a XS_RESTRICT user on an old kernel not knowing about it can't
drop the privilege of all other user's on that system, too.

> I'm not convinced that socket-based distinction is good enough -- what
> if some program tries to multiplex the socket? Is that not possible at
> the moment?

In this case it would have to handle XS_RESTRICT the same way as the
kernel should do it.


Juergen


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

Re: Xenstore domains and XS_RESTRICT

[Wei Liu]

On Wed, Jan 04, 2017 at 04:05:03PM +0100, Juergen Gross wrote:
> On 04/01/17 15:59, Wei Liu wrote:
> > On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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.
> >>
> > 
> > I'm slightly concerned about this non-unified model -- I would rather
> > xenstored only needs to deal with "virtual connection", regardless of
> > the interface (socket or kernel) they use.
> 
> Rejecting XS_RESTRICT for a non-socket connection is mandatory to
> ensure a XS_RESTRICT user on an old kernel not knowing about it can't
> drop the privilege of all other user's on that system, too.
> 

This highlights the flaw in this command. IMHO it would make sense to
invent something new other than extending something that is already
deemed inadequate.

There is no documentation about the semantics of XS_RESTRICT, which
is another reason to not touch it.

Wei.

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

Re: Xenstore domains and XS_RESTRICT

[Ian Jackson]

Juergen Gross writes ("Re: Xenstore domains and XS_RESTRICT"):
> Rejecting XS_RESTRICT for a non-socket connection is mandatory to
> ensure a XS_RESTRICT user on an old kernel not knowing about it can't
> drop the privilege of all other user's on that system, too.

Kernels need to proxy all commands from their users, so they should
have a table (usually a switch statement) of supported commands.
New commands are therefore unavailable until the kernel is updated.

I haven't checked the Linux xenbus chardev driver to see if it is
correct ...

Ian.

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 04/01/17 17:54, Ian Jackson wrote:
> Juergen Gross writes ("Re: Xenstore domains and XS_RESTRICT"):
>> Rejecting XS_RESTRICT for a non-socket connection is mandatory to
>> ensure a XS_RESTRICT user on an old kernel not knowing about it can't
>> drop the privilege of all other user's on that system, too.
> 
> Kernels need to proxy all commands from their users, so they should
> have a table (usually a switch statement) of supported commands.
> New commands are therefore unavailable until the kernel is updated.

Uuh, really?

I think this is true for only very few commands needing special
treatment (right now those are: start/stop transaction, register/
unregister watch). All other commands can just be passed through
without any need for special (command specific) treatment in the kernel.

XS_RESTRICT would be another command needing special treatment.

XS_DIRECTORY_PART introduced recently is a good example for a command
not needing any special kernel treatment. If the kernel would have a
positive list of commands to pass through this command would be
available on new kernels only limiting e.g. dom0 functionality
without any special need.

> I haven't checked the Linux xenbus chardev driver to see if it is
> correct ...

In your sense: no. For any non-special new Xenstore commands: yes.


Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 04/01/17 16:21, Wei Liu wrote:
> On Wed, Jan 04, 2017 at 04:05:03PM +0100, Juergen Gross wrote:
>> On 04/01/17 15:59, Wei Liu wrote:
>>> On Wed, Dec 07, 2016 at 08:44:31AM +0100, 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.
>>>>
>>>
>>> I'm slightly concerned about this non-unified model -- I would rather
>>> xenstored only needs to deal with "virtual connection", regardless of
>>> the interface (socket or kernel) they use.
>>
>> Rejecting XS_RESTRICT for a non-socket connection is mandatory to
>> ensure a XS_RESTRICT user on an old kernel not knowing about it can't
>> drop the privilege of all other user's on that system, too.
>>
> 
> This highlights the flaw in this command. IMHO it would make sense to
> invent something new other than extending something that is already
> deemed inadequate.

I think this "flaw" is present for any new Xenstore command modifying
the state of a connection for all or some following commands: kernel
and Xen versions are not in sync and so you'd need such mechanisms.

> There is no documentation about the semantics of XS_RESTRICT, which
> is another reason to not touch it.

Yes. See https://lists.xen.org/archives/html/xen-devel/2016-06/msg03487.html


Juergen


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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

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?


Juergen


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

Re: Xenstore domains and XS_RESTRICT

[Wei Liu]

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?

And sorry for the late reply, I'm still mulling over your proposal, I
will try to respond as soon as possible.

Wei.

> 
> Juergen
> 

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

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.

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


Juergen

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

Re: Xenstore domains and XS_RESTRICT

[Wei Liu]

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

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@lists.xen.org
https://lists.xen.org/xen-devel

Re: Xenstore domains and XS_RESTRICT

[Andrew Cooper]

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@lists.xen.org
https://lists.xen.org/xen-devel

Re: Xenstore domains and XS_RESTRICT

[George Dunlap]

On 18/01/17 12:37, Andrew Cooper wrote:
> 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.

I think I'd go with "Rip it out and if anyone complains, we can figure
out what to do (including puting it back in)".

 -George


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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 18/01/17 13:39, George Dunlap wrote:
> On 18/01/17 12:37, Andrew Cooper wrote:
>> 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.
> 
> I think I'd go with "Rip it out and if anyone complains, we can figure
> out what to do (including puting it back in)".

I'll post a patch to do this. In case someone is strictly against
deleting XS_RESTRICT he can still NAK the patch, right?


Juergen


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

Re: Xenstore domains and XS_RESTRICT

[Wei Liu]

On Wed, Jan 18, 2017 at 01:42:01PM +0100, Juergen Gross wrote:
> On 18/01/17 13:39, George Dunlap wrote:
> > On 18/01/17 12:37, Andrew Cooper wrote:
> >> 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.
> > 
> > I think I'd go with "Rip it out and if anyone complains, we can figure
> > out what to do (including puting it back in)".
> 
> I'll post a patch to do this. In case someone is strictly against
> deleting XS_RESTRICT he can still NAK the patch, right?
> 

Yes, that's correct.

> 
> Juergen
> 

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

Re: Xenstore domains and XS_RESTRICT

[Stefano Stabellini]

On Wed, 18 Jan 2017, 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.
> 
> > 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.

It does not work. However, QEMU based backends can be run on a separate
QEMU. Patches were submitted by IanJ and me to run 2 QEMUs per domain,
one to provide emulation, the other to provide the backends. Not sure
what happen to them, but they were more then prototypes.


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

Yes, but ioemu-stubdoms have drawbacks that make them not viable in many
scenarios. There are reasons why they are not enabled by default.
XS_RESTRICT should not replace, but complement ioemu-stubdoms. If we
remove XS_RESTRICT, what's the plan to make QEMU in Dom0 secure by
default?

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

Re: Xenstore domains and XS_RESTRICT

[Juergen Gross]

On 18/01/17 19:26, Stefano Stabellini wrote:
> On Wed, 18 Jan 2017, 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.
>>
>>> 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.
> 
> It does not work. However, QEMU based backends can be run on a separate
> QEMU. Patches were submitted by IanJ and me to run 2 QEMUs per domain,
> one to provide emulation, the other to provide the backends. Not sure
> what happen to them, but they were more then prototypes.
> 
> 
>> 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.").
> 
> Yes, but ioemu-stubdoms have drawbacks that make them not viable in many
> scenarios. There are reasons why they are not enabled by default.
> XS_RESTRICT should not replace, but complement ioemu-stubdoms. If we
> remove XS_RESTRICT, what's the plan to make QEMU in Dom0 secure by
> default?

Currently none. OTOH there is no plan how to make XS_RESTRICT work in
other cases like Xenstore domain.

We need to design a solution which has no such drawbacks. We don't have
to implement them all from beginning, but we should know how to do it.
Otherwise something like XS_RESTRICT will be the result which isn't
activated as default as it isn't working for all cases.


Juergen


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