There are now instances where internal hypervisor logic needs to make resource
allocation calls that are protected by XSM checks. The internal hypervisor logic
is represented a number of system domains which by designed are represented by
non-privileged struct domain instances. To enable these logic blocks to
function correctly but in a controlled manner, this commit introduces a pair
of privilege escalation and demotion functions that will make a system domain
privileged and then remove that privilege.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index e22d6160b5..157e57151e 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -189,6 +189,28 @@ struct xsm_operations {
 #endif
 };
 
+static always_inline int xsm_elevate_priv(struct domain *d)
+{
+    if ( is_system_domain(d) )
+    {
+        d->is_privileged = true;
+        return 0;
+    }
+
+    return -EPERM;
+}
+
+static always_inline int xsm_demote_priv(struct domain *d)
+{
+    if ( is_system_domain(d) )
+    {
+        d->is_privileged = false;
+        return 0;
+    }
+
+    return -EPERM;
+}
+
 #ifdef CONFIG_XSM
 
 extern struct xsm_operations *xsm_ops;
-- 
2.20.1

Hi Daniel,

On 31/03/2022 00:05, Daniel P. Smith wrote:
> There are now instances where internal hypervisor logic needs to make resource
> allocation calls that are protected by XSM checks. The internal hypervisor logic
> is represented a number of system domains which by designed are represented by
> non-privileged struct domain instances. To enable these logic blocks to
> function correctly but in a controlled manner, this commit introduces a pair
> of privilege escalation and demotion functions that will make a system domain
> privileged and then remove that privilege.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>   xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index e22d6160b5..157e57151e 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -189,6 +189,28 @@ struct xsm_operations {
>   #endif
>   };
>   
> +static always_inline int xsm_elevate_priv(struct domain *d)
> +{
> +    if ( is_system_domain(d) )
> +    {
> +        d->is_privileged = true;

The call for xsm_elevate_priv() cannot be nested. So I would suggest to 
check if d->is_privileged is already true and return -EBUSY in this case.

> +        return 0;
> +    }
> +
> +    return -EPERM;
> +}
> +
> +static always_inline int xsm_demote_priv(struct domain *d)
> +{
> +    if ( is_system_domain(d) )
> +    {
> +        d->is_privileged = false;
> +        return 0;
> +    }
> +
> +    return -EPERM;
> +}
> +
>   #ifdef CONFIG_XSM
>   
>   extern struct xsm_operations *xsm_ops;

Cheers,

-- 
Julien Grall

On Wed, Mar 30, 2022 at 3:05 PM Daniel P. Smith
<dpsmith@apertussolutions.com> wrote:
>
> There are now instances where internal hypervisor logic needs to make resource
> allocation calls that are protected by XSM checks. The internal hypervisor logic
> is represented a number of system domains which by designed are represented by
> non-privileged struct domain instances. To enable these logic blocks to
> function correctly but in a controlled manner, this commit introduces a pair
> of privilege escalation and demotion functions that will make a system domain
> privileged and then remove that privilege.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>  xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index e22d6160b5..157e57151e 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -189,6 +189,28 @@ struct xsm_operations {
>  #endif
>  };
>
> +static always_inline int xsm_elevate_priv(struct domain *d)
> +{
> +    if ( is_system_domain(d) )
> +    {
> +        d->is_privileged = true;
> +        return 0;
> +    }
> +
> +    return -EPERM;
> +}

These look sufficient for the default policy, but they don't seem
sufficient for Flask.  I think you need to create a new XSM hook.  For
Flask, you would want the demote hook to transition xen_boot_t ->
xen_t.  That would start xen_boot_t with privileges that are dropped
in a one-way transition.  Does that require all policies to then have
xen_boot_t and xen_t?  I guess it does unless the hook code has some
logic to skip the transition.

For the default policy, you could start by creating the system domains
as privileged and just have a single hook to drop privs.  Then you
don't have to worry about the "elevate" hook existing.  The patch 2
asserts could instead become the location of xsm_drop_privs calls to
have a clear demarcation point.  That expands the window with
privileges though.  It's a little simpler, but maybe you don't want
that.  However, it seems like you can only depriv once for the Flask
case since you want it to be one-way.

Regards,
Jason

On 3/31/22 09:16, Jason Andryuk wrote:
> On Wed, Mar 30, 2022 at 3:05 PM Daniel P. Smith
> <dpsmith@apertussolutions.com> wrote:
>>
>> There are now instances where internal hypervisor logic needs to make resource
>> allocation calls that are protected by XSM checks. The internal hypervisor logic
>> is represented a number of system domains which by designed are represented by
>> non-privileged struct domain instances. To enable these logic blocks to
>> function correctly but in a controlled manner, this commit introduces a pair
>> of privilege escalation and demotion functions that will make a system domain
>> privileged and then remove that privilege.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>  xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
>> index e22d6160b5..157e57151e 100644
>> --- a/xen/include/xsm/xsm.h
>> +++ b/xen/include/xsm/xsm.h
>> @@ -189,6 +189,28 @@ struct xsm_operations {
>>  #endif
>>  };
>>
>> +static always_inline int xsm_elevate_priv(struct domain *d)
>> +{
>> +    if ( is_system_domain(d) )
>> +    {
>> +        d->is_privileged = true;
>> +        return 0;
>> +    }
>> +
>> +    return -EPERM;
>> +}
> 
> These look sufficient for the default policy, but they don't seem
> sufficient for Flask.  I think you need to create a new XSM hook.  For
> Flask, you would want the demote hook to transition xen_boot_t ->
> xen_t.  That would start xen_boot_t with privileges that are dropped
> in a one-way transition.  Does that require all policies to then have
> xen_boot_t and xen_t?  I guess it does unless the hook code has some
> logic to skip the transition.

I am still thinking this through but my initial concern for Flask is
that I don't think we want dedicated domain transitions directly in
code. My current thinking would be to use a Kconfig to use xen_boot_t
type as the initial sid for the idle domain which would then require the
default policy to include an allowed transition from xen_boot_t to
xen_t. Then rely upon a boot domain to issue an xsm_op to do a relabel
transition for the idle domain with an assertion that the idle domain is
no longer labeled with its initial sid before Xen transitions its state
to SYS_STATE_active. The one wrinkle to this is whether I will be able
to schedule the boot domain before allowing Xen to transition into
SYS_STATE_active.

> For the default policy, you could start by creating the system domains
> as privileged and just have a single hook to drop privs.  Then you
> don't have to worry about the "elevate" hook existing.  The patch 2
> asserts could instead become the location of xsm_drop_privs calls to
> have a clear demarcation point.  That expands the window with
> privileges though.  It's a little simpler, but maybe you don't want
> that.  However, it seems like you can only depriv once for the Flask
> case since you want it to be one-way.

This does simplify the solution and since today we cannot differentiate
between hypervisor setup and hypervisor initiated domain construction
contexts, it does not run counter to what I have proposed. As for flask,
again I do not believe codifying a domain transition bound to a new XSM
op is the appropriate approach.

v/r,
dps

On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith
<dpsmith@apertussolutions.com> wrote:
>
> On 3/31/22 09:16, Jason Andryuk wrote:
> > On Wed, Mar 30, 2022 at 3:05 PM Daniel P. Smith
> > <dpsmith@apertussolutions.com> wrote:
> >>
> >> There are now instances where internal hypervisor logic needs to make resource
> >> allocation calls that are protected by XSM checks. The internal hypervisor logic
> >> is represented a number of system domains which by designed are represented by
> >> non-privileged struct domain instances. To enable these logic blocks to
> >> function correctly but in a controlled manner, this commit introduces a pair
> >> of privilege escalation and demotion functions that will make a system domain
> >> privileged and then remove that privilege.
> >>
> >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> >> ---
> >>  xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> >> index e22d6160b5..157e57151e 100644
> >> --- a/xen/include/xsm/xsm.h
> >> +++ b/xen/include/xsm/xsm.h
> >> @@ -189,6 +189,28 @@ struct xsm_operations {
> >>  #endif
> >>  };
> >>
> >> +static always_inline int xsm_elevate_priv(struct domain *d)
> >> +{
> >> +    if ( is_system_domain(d) )
> >> +    {
> >> +        d->is_privileged = true;
> >> +        return 0;
> >> +    }
> >> +
> >> +    return -EPERM;
> >> +}
> >
> > These look sufficient for the default policy, but they don't seem
> > sufficient for Flask.  I think you need to create a new XSM hook.  For
> > Flask, you would want the demote hook to transition xen_boot_t ->
> > xen_t.  That would start xen_boot_t with privileges that are dropped
> > in a one-way transition.  Does that require all policies to then have
> > xen_boot_t and xen_t?  I guess it does unless the hook code has some
> > logic to skip the transition.
>
> I am still thinking this through but my initial concern for Flask is
> that I don't think we want dedicated domain transitions directly in
> code. My current thinking would be to use a Kconfig to use xen_boot_t
> type as the initial sid for the idle domain which would then require the
> default policy to include an allowed transition from xen_boot_t to
> xen_t. Then rely upon a boot domain to issue an xsm_op to do a relabel
> transition for the idle domain with an assertion that the idle domain is
> no longer labeled with its initial sid before Xen transitions its state
> to SYS_STATE_active. The one wrinkle to this is whether I will be able
> to schedule the boot domain before allowing Xen to transition into
> SYS_STATE_active.

That is an interesting approach.  While it would work, I find it
unusual that a domain would relabel Xen.  I think Xen should be
responsible for itself and not rely on a domain for this operation.

> > For the default policy, you could start by creating the system domains
> > as privileged and just have a single hook to drop privs.  Then you
> > don't have to worry about the "elevate" hook existing.  The patch 2
> > asserts could instead become the location of xsm_drop_privs calls to
> > have a clear demarcation point.  That expands the window with
> > privileges though.  It's a little simpler, but maybe you don't want
> > that.  However, it seems like you can only depriv once for the Flask
> > case since you want it to be one-way.
>
> This does simplify the solution and since today we cannot differentiate
> between hypervisor setup and hypervisor initiated domain construction
> contexts, it does not run counter to what I have proposed. As for flask,
> again I do not believe codifying a domain transition bound to a new XSM
> op is the appropriate approach.

This hard coded domain transition does feel a little weird.  But it
seems like a natural consequence of trying to use Flask to
deprivilege.  I guess the transition could be behind a
dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
in some fashion with Flask enforcing.

Another idea: Flask could start in permissive and only transition to
enforcing at the deprivilege point.  Kinda gross, but it works without
needing a transition.

To reiterate, XSM isn't really appropriate to enforce anything
internal to Xen.  We are working around the need to go through hook
points during correct operation.  Code exec in Xen means all bets are
off.  Memory writes to Xen data mean the XSM checks can be disabled
(flip Flask to permissive) or bypassed (set d->is_privileged or change
d->ssid).  We shouldn't lose sight of this when we talk about
deprivileging the idle domain.

Regards,
Jason

On 05.04.2022 19:17, Jason Andryuk wrote:
> On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
>> On 3/31/22 09:16, Jason Andryuk wrote:
>>> For the default policy, you could start by creating the system domains
>>> as privileged and just have a single hook to drop privs.  Then you
>>> don't have to worry about the "elevate" hook existing.  The patch 2
>>> asserts could instead become the location of xsm_drop_privs calls to
>>> have a clear demarcation point.  That expands the window with
>>> privileges though.  It's a little simpler, but maybe you don't want
>>> that.  However, it seems like you can only depriv once for the Flask
>>> case since you want it to be one-way.
>>
>> This does simplify the solution and since today we cannot differentiate
>> between hypervisor setup and hypervisor initiated domain construction
>> contexts, it does not run counter to what I have proposed. As for flask,
>> again I do not believe codifying a domain transition bound to a new XSM
>> op is the appropriate approach.
> 
> This hard coded domain transition does feel a little weird.  But it
> seems like a natural consequence of trying to use Flask to
> deprivilege.  I guess the transition could be behind a
> dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
> in some fashion with Flask enforcing.
> 
> Another idea: Flask could start in permissive and only transition to
> enforcing at the deprivilege point.  Kinda gross, but it works without
> needing a transition.

I don't think that would be right. Logically such behavior ought to be
mirrored to SILO, and I'll take that for the example for being the
simpler model: Suppose an admin wants to disallow communication
between DomU-s created by Xen. Such would want enforcing when creating
those DomU-s, despite the creator (Xen) being all powerful. If the
device tree information said something different (e.g. directing for
an event channel to be established between two such DomU-s), this
should be flagged as an error, not be silently permitted.

Jan

On Wed, Apr 6, 2022 at 3:07 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.04.2022 19:17, Jason Andryuk wrote:
> > On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> >> On 3/31/22 09:16, Jason Andryuk wrote:
> >>> For the default policy, you could start by creating the system domains
> >>> as privileged and just have a single hook to drop privs.  Then you
> >>> don't have to worry about the "elevate" hook existing.  The patch 2
> >>> asserts could instead become the location of xsm_drop_privs calls to
> >>> have a clear demarcation point.  That expands the window with
> >>> privileges though.  It's a little simpler, but maybe you don't want
> >>> that.  However, it seems like you can only depriv once for the Flask
> >>> case since you want it to be one-way.
> >>
> >> This does simplify the solution and since today we cannot differentiate
> >> between hypervisor setup and hypervisor initiated domain construction
> >> contexts, it does not run counter to what I have proposed. As for flask,
> >> again I do not believe codifying a domain transition bound to a new XSM
> >> op is the appropriate approach.
> >
> > This hard coded domain transition does feel a little weird.  But it
> > seems like a natural consequence of trying to use Flask to
> > deprivilege.  I guess the transition could be behind a
> > dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
> > in some fashion with Flask enforcing.
> >
> > Another idea: Flask could start in permissive and only transition to
> > enforcing at the deprivilege point.  Kinda gross, but it works without
> > needing a transition.
>
> I don't think that would be right. Logically such behavior ought to be
> mirrored to SILO, and I'll take that for the example for being the
> simpler model: Suppose an admin wants to disallow communication
> between DomU-s created by Xen. Such would want enforcing when creating
> those DomU-s, despite the creator (Xen) being all powerful. If the
> device tree information said something different (e.g. directing for
> an event channel to be established between two such DomU-s), this
> should be flagged as an error, not be silently permitted.

Yes, you are right.  As you say, you want the policy enforced when
creating the DomU-s.

Regards,
Jason

On Wed, Apr 06, 2022 at 09:06:59AM +0200, Jan Beulich wrote:
> On 05.04.2022 19:17, Jason Andryuk wrote:
> > On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> >> On 3/31/22 09:16, Jason Andryuk wrote:
> >>> For the default policy, you could start by creating the system domains
> >>> as privileged and just have a single hook to drop privs.  Then you
> >>> don't have to worry about the "elevate" hook existing.  The patch 2
> >>> asserts could instead become the location of xsm_drop_privs calls to
> >>> have a clear demarcation point.  That expands the window with
> >>> privileges though.  It's a little simpler, but maybe you don't want
> >>> that.  However, it seems like you can only depriv once for the Flask
> >>> case since you want it to be one-way.
> >>
> >> This does simplify the solution and since today we cannot differentiate
> >> between hypervisor setup and hypervisor initiated domain construction
> >> contexts, it does not run counter to what I have proposed. As for flask,
> >> again I do not believe codifying a domain transition bound to a new XSM
> >> op is the appropriate approach.
> > 
> > This hard coded domain transition does feel a little weird.  But it
> > seems like a natural consequence of trying to use Flask to
> > deprivilege.  I guess the transition could be behind a
> > dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
> > in some fashion with Flask enforcing.
> > 
> > Another idea: Flask could start in permissive and only transition to
> > enforcing at the deprivilege point.  Kinda gross, but it works without
> > needing a transition.
> 
> I don't think that would be right. Logically such behavior ought to be
> mirrored to SILO, and I'll take that for the example for being the
> simpler model: Suppose an admin wants to disallow communication
> between DomU-s created by Xen. Such would want enforcing when creating
> those DomU-s, despite the creator (Xen) being all powerful. If the
> device tree information said something different (e.g. directing for
> an event channel to be established between two such DomU-s), this
> should be flagged as an error, not be silently permitted.

I could also see this argument the other way around: what if an admin
wants to disallow domUs freely communicating between them, but would
still like some controlled domU communication to be possible by
setting up those channels at domain creation?

Thanks, Roger.

On 06.04.2022 10:46, Roger Pau Monné wrote:
> On Wed, Apr 06, 2022 at 09:06:59AM +0200, Jan Beulich wrote:
>> On 05.04.2022 19:17, Jason Andryuk wrote:
>>> On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
>>>> On 3/31/22 09:16, Jason Andryuk wrote:
>>>>> For the default policy, you could start by creating the system domains
>>>>> as privileged and just have a single hook to drop privs.  Then you
>>>>> don't have to worry about the "elevate" hook existing.  The patch 2
>>>>> asserts could instead become the location of xsm_drop_privs calls to
>>>>> have a clear demarcation point.  That expands the window with
>>>>> privileges though.  It's a little simpler, but maybe you don't want
>>>>> that.  However, it seems like you can only depriv once for the Flask
>>>>> case since you want it to be one-way.
>>>>
>>>> This does simplify the solution and since today we cannot differentiate
>>>> between hypervisor setup and hypervisor initiated domain construction
>>>> contexts, it does not run counter to what I have proposed. As for flask,
>>>> again I do not believe codifying a domain transition bound to a new XSM
>>>> op is the appropriate approach.
>>>
>>> This hard coded domain transition does feel a little weird.  But it
>>> seems like a natural consequence of trying to use Flask to
>>> deprivilege.  I guess the transition could be behind a
>>> dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
>>> in some fashion with Flask enforcing.
>>>
>>> Another idea: Flask could start in permissive and only transition to
>>> enforcing at the deprivilege point.  Kinda gross, but it works without
>>> needing a transition.
>>
>> I don't think that would be right. Logically such behavior ought to be
>> mirrored to SILO, and I'll take that for the example for being the
>> simpler model: Suppose an admin wants to disallow communication
>> between DomU-s created by Xen. Such would want enforcing when creating
>> those DomU-s, despite the creator (Xen) being all powerful. If the
>> device tree information said something different (e.g. directing for
>> an event channel to be established between two such DomU-s), this
>> should be flagged as an error, not be silently permitted.
> 
> I could also see this argument the other way around: what if an admin
> wants to disallow domUs freely communicating between them, but would
> still like some controlled domU communication to be possible by
> setting up those channels at domain creation?

Well, imo that would require a proper (Flask) policy then, not SILO mode.

Jan

On Wed, Apr 06, 2022 at 10:48:23AM +0200, Jan Beulich wrote:
> On 06.04.2022 10:46, Roger Pau Monné wrote:
> > On Wed, Apr 06, 2022 at 09:06:59AM +0200, Jan Beulich wrote:
> >> On 05.04.2022 19:17, Jason Andryuk wrote:
> >>> On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> >>>> On 3/31/22 09:16, Jason Andryuk wrote:
> >>>>> For the default policy, you could start by creating the system domains
> >>>>> as privileged and just have a single hook to drop privs.  Then you
> >>>>> don't have to worry about the "elevate" hook existing.  The patch 2
> >>>>> asserts could instead become the location of xsm_drop_privs calls to
> >>>>> have a clear demarcation point.  That expands the window with
> >>>>> privileges though.  It's a little simpler, but maybe you don't want
> >>>>> that.  However, it seems like you can only depriv once for the Flask
> >>>>> case since you want it to be one-way.
> >>>>
> >>>> This does simplify the solution and since today we cannot differentiate
> >>>> between hypervisor setup and hypervisor initiated domain construction
> >>>> contexts, it does not run counter to what I have proposed. As for flask,
> >>>> again I do not believe codifying a domain transition bound to a new XSM
> >>>> op is the appropriate approach.
> >>>
> >>> This hard coded domain transition does feel a little weird.  But it
> >>> seems like a natural consequence of trying to use Flask to
> >>> deprivilege.  I guess the transition could be behind a
> >>> dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
> >>> in some fashion with Flask enforcing.
> >>>
> >>> Another idea: Flask could start in permissive and only transition to
> >>> enforcing at the deprivilege point.  Kinda gross, but it works without
> >>> needing a transition.
> >>
> >> I don't think that would be right. Logically such behavior ought to be
> >> mirrored to SILO, and I'll take that for the example for being the
> >> simpler model: Suppose an admin wants to disallow communication
> >> between DomU-s created by Xen. Such would want enforcing when creating
> >> those DomU-s, despite the creator (Xen) being all powerful. If the
> >> device tree information said something different (e.g. directing for
> >> an event channel to be established between two such DomU-s), this
> >> should be flagged as an error, not be silently permitted.
> > 
> > I could also see this argument the other way around: what if an admin
> > wants to disallow domUs freely communicating between them, but would
> > still like some controlled domU communication to be possible by
> > setting up those channels at domain creation?
> 
> Well, imo that would require a proper (Flask) policy then, not SILO mode.

But when creating such domains in SILO mode from dom0, dom0 would be
allowed to create and bind event channels to the created domains, even
if the domain themselves are not allowed the operation.

That's because the check in evtchn_bind_interdomain() is done against
'current' and not the domain where the event channel will be bound.
Maybe such check should instead take 3 parameters: current context
domain, domain to bind the event channel to and remote domain on the
other end of the event channel.

Thanks, Roger.

On 06.04.2022 11:09, Roger Pau Monné wrote:
> On Wed, Apr 06, 2022 at 10:48:23AM +0200, Jan Beulich wrote:
>> On 06.04.2022 10:46, Roger Pau Monné wrote:
>>> On Wed, Apr 06, 2022 at 09:06:59AM +0200, Jan Beulich wrote:
>>>> On 05.04.2022 19:17, Jason Andryuk wrote:
>>>>> On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
>>>>>> On 3/31/22 09:16, Jason Andryuk wrote:
>>>>>>> For the default policy, you could start by creating the system domains
>>>>>>> as privileged and just have a single hook to drop privs.  Then you
>>>>>>> don't have to worry about the "elevate" hook existing.  The patch 2
>>>>>>> asserts could instead become the location of xsm_drop_privs calls to
>>>>>>> have a clear demarcation point.  That expands the window with
>>>>>>> privileges though.  It's a little simpler, but maybe you don't want
>>>>>>> that.  However, it seems like you can only depriv once for the Flask
>>>>>>> case since you want it to be one-way.
>>>>>>
>>>>>> This does simplify the solution and since today we cannot differentiate
>>>>>> between hypervisor setup and hypervisor initiated domain construction
>>>>>> contexts, it does not run counter to what I have proposed. As for flask,
>>>>>> again I do not believe codifying a domain transition bound to a new XSM
>>>>>> op is the appropriate approach.
>>>>>
>>>>> This hard coded domain transition does feel a little weird.  But it
>>>>> seems like a natural consequence of trying to use Flask to
>>>>> deprivilege.  I guess the transition could be behind a
>>>>> dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
>>>>> in some fashion with Flask enforcing.
>>>>>
>>>>> Another idea: Flask could start in permissive and only transition to
>>>>> enforcing at the deprivilege point.  Kinda gross, but it works without
>>>>> needing a transition.
>>>>
>>>> I don't think that would be right. Logically such behavior ought to be
>>>> mirrored to SILO, and I'll take that for the example for being the
>>>> simpler model: Suppose an admin wants to disallow communication
>>>> between DomU-s created by Xen. Such would want enforcing when creating
>>>> those DomU-s, despite the creator (Xen) being all powerful. If the
>>>> device tree information said something different (e.g. directing for
>>>> an event channel to be established between two such DomU-s), this
>>>> should be flagged as an error, not be silently permitted.
>>>
>>> I could also see this argument the other way around: what if an admin
>>> wants to disallow domUs freely communicating between them, but would
>>> still like some controlled domU communication to be possible by
>>> setting up those channels at domain creation?
>>
>> Well, imo that would require a proper (Flask) policy then, not SILO mode.
> 
> But when creating such domains in SILO mode from dom0, dom0 would be
> allowed to create and bind event channels to the created domains, even
> if the domain themselves are not allowed the operation.
> 
> That's because the check in evtchn_bind_interdomain() is done against
> 'current' and not the domain where the event channel will be bound.

Yes and no - the check is against current, but that's because
communication is established between current ( == ld) and rd. The
function in its present shape simply can't establish a channel
between two arbitrary domains.

Jan

> Maybe such check should instead take 3 parameters: current context
> domain, domain to bind the event channel to and remote domain on the
> other end of the event channel.
> 
> Thanks, Roger.
> 

On Wed, Apr 06, 2022 at 11:16:10AM +0200, Jan Beulich wrote:
> On 06.04.2022 11:09, Roger Pau Monné wrote:
> > On Wed, Apr 06, 2022 at 10:48:23AM +0200, Jan Beulich wrote:
> >> On 06.04.2022 10:46, Roger Pau Monné wrote:
> >>> On Wed, Apr 06, 2022 at 09:06:59AM +0200, Jan Beulich wrote:
> >>>> On 05.04.2022 19:17, Jason Andryuk wrote:
> >>>>> On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> >>>>>> On 3/31/22 09:16, Jason Andryuk wrote:
> >>>>>>> For the default policy, you could start by creating the system domains
> >>>>>>> as privileged and just have a single hook to drop privs.  Then you
> >>>>>>> don't have to worry about the "elevate" hook existing.  The patch 2
> >>>>>>> asserts could instead become the location of xsm_drop_privs calls to
> >>>>>>> have a clear demarcation point.  That expands the window with
> >>>>>>> privileges though.  It's a little simpler, but maybe you don't want
> >>>>>>> that.  However, it seems like you can only depriv once for the Flask
> >>>>>>> case since you want it to be one-way.
> >>>>>>
> >>>>>> This does simplify the solution and since today we cannot differentiate
> >>>>>> between hypervisor setup and hypervisor initiated domain construction
> >>>>>> contexts, it does not run counter to what I have proposed. As for flask,
> >>>>>> again I do not believe codifying a domain transition bound to a new XSM
> >>>>>> op is the appropriate approach.
> >>>>>
> >>>>> This hard coded domain transition does feel a little weird.  But it
> >>>>> seems like a natural consequence of trying to use Flask to
> >>>>> deprivilege.  I guess the transition could be behind a
> >>>>> dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
> >>>>> in some fashion with Flask enforcing.
> >>>>>
> >>>>> Another idea: Flask could start in permissive and only transition to
> >>>>> enforcing at the deprivilege point.  Kinda gross, but it works without
> >>>>> needing a transition.
> >>>>
> >>>> I don't think that would be right. Logically such behavior ought to be
> >>>> mirrored to SILO, and I'll take that for the example for being the
> >>>> simpler model: Suppose an admin wants to disallow communication
> >>>> between DomU-s created by Xen. Such would want enforcing when creating
> >>>> those DomU-s, despite the creator (Xen) being all powerful. If the
> >>>> device tree information said something different (e.g. directing for
> >>>> an event channel to be established between two such DomU-s), this
> >>>> should be flagged as an error, not be silently permitted.
> >>>
> >>> I could also see this argument the other way around: what if an admin
> >>> wants to disallow domUs freely communicating between them, but would
> >>> still like some controlled domU communication to be possible by
> >>> setting up those channels at domain creation?
> >>
> >> Well, imo that would require a proper (Flask) policy then, not SILO mode.
> > 
> > But when creating such domains in SILO mode from dom0, dom0 would be
> > allowed to create and bind event channels to the created domains, even
> > if the domain themselves are not allowed the operation.
> > 
> > That's because the check in evtchn_bind_interdomain() is done against
> > 'current' and not the domain where the event channel will be bound.
> 
> Yes and no - the check is against current, but that's because
> communication is established between current ( == ld) and rd. The
> function in its present shape simply can't establish a channel
> between two arbitrary domains.

I've got confused with evtchn_alloc_unbound() that does take a local
and remote domains as parameters, but in that case the xsm check is
done against ld (which might not be current) and rd.

Thanks, Roger.

On 4/5/22 13:17, Jason Andryuk wrote:
> On Mon, Apr 4, 2022 at 11:34 AM Daniel P. Smith
> <dpsmith@apertussolutions.com> wrote:
>>
>> On 3/31/22 09:16, Jason Andryuk wrote:
>>> On Wed, Mar 30, 2022 at 3:05 PM Daniel P. Smith
>>> <dpsmith@apertussolutions.com> wrote:
>>>>
>>>> There are now instances where internal hypervisor logic needs to make resource
>>>> allocation calls that are protected by XSM checks. The internal hypervisor logic
>>>> is represented a number of system domains which by designed are represented by
>>>> non-privileged struct domain instances. To enable these logic blocks to
>>>> function correctly but in a controlled manner, this commit introduces a pair
>>>> of privilege escalation and demotion functions that will make a system domain
>>>> privileged and then remove that privilege.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>>> ---
>>>>  xen/include/xsm/xsm.h | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
>>>> index e22d6160b5..157e57151e 100644
>>>> --- a/xen/include/xsm/xsm.h
>>>> +++ b/xen/include/xsm/xsm.h
>>>> @@ -189,6 +189,28 @@ struct xsm_operations {
>>>>  #endif
>>>>  };
>>>>
>>>> +static always_inline int xsm_elevate_priv(struct domain *d)
>>>> +{
>>>> +    if ( is_system_domain(d) )
>>>> +    {
>>>> +        d->is_privileged = true;
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    return -EPERM;
>>>> +}
>>>
>>> These look sufficient for the default policy, but they don't seem
>>> sufficient for Flask.  I think you need to create a new XSM hook.  For
>>> Flask, you would want the demote hook to transition xen_boot_t ->
>>> xen_t.  That would start xen_boot_t with privileges that are dropped
>>> in a one-way transition.  Does that require all policies to then have
>>> xen_boot_t and xen_t?  I guess it does unless the hook code has some
>>> logic to skip the transition.
>>
>> I am still thinking this through but my initial concern for Flask is
>> that I don't think we want dedicated domain transitions directly in
>> code. My current thinking would be to use a Kconfig to use xen_boot_t
>> type as the initial sid for the idle domain which would then require the
>> default policy to include an allowed transition from xen_boot_t to
>> xen_t. Then rely upon a boot domain to issue an xsm_op to do a relabel
>> transition for the idle domain with an assertion that the idle domain is
>> no longer labeled with its initial sid before Xen transitions its state
>> to SYS_STATE_active. The one wrinkle to this is whether I will be able
>> to schedule the boot domain before allowing Xen to transition into
>> SYS_STATE_active.
> 
> That is an interesting approach.  While it would work, I find it
> unusual that a domain would relabel Xen.  I think Xen should be
> responsible for itself and not rely on a domain for this operation.

The boot domain is not a general domain as no domain can/should be
created with its domid or flask label post transition to
SYS_STATE_active. Its purpose was specifically meant to be a natural way
to push out complicated pre-execution domain configuration from having
to be in they hypervisor code. Therefore in a way it can be considered a
user provided de-privileged part of the hypervisor.

With that said, I just realized a flaw in the basis of my position. What
is the difference between codifying a check that the idle domain is not
the boot label versus codifying a transition from the boot label to the
running label? None really, both will require some knowledge that there
is a boot label and some running label. Combine with the fact that the
idle domain really shouldn't have any other label than xen_t. I will
work out how to incorporate the domain transition.

>>> For the default policy, you could start by creating the system domains
>>> as privileged and just have a single hook to drop privs.  Then you
>>> don't have to worry about the "elevate" hook existing.  The patch 2
>>> asserts could instead become the location of xsm_drop_privs calls to
>>> have a clear demarcation point.  That expands the window with
>>> privileges though.  It's a little simpler, but maybe you don't want
>>> that.  However, it seems like you can only depriv once for the Flask
>>> case since you want it to be one-way.
>>
>> This does simplify the solution and since today we cannot differentiate
>> between hypervisor setup and hypervisor initiated domain construction
>> contexts, it does not run counter to what I have proposed. As for flask,
>> again I do not believe codifying a domain transition bound to a new XSM
>> op is the appropriate approach.
> 
> This hard coded domain transition does feel a little weird.  But it
> seems like a natural consequence of trying to use Flask to
> deprivilege.  I guess the transition could be behind a
> dom0less/hyperlaunch Kconfig option.  I just don't see a way around it
> in some fashion with Flask enforcing.
> 
> Another idea: Flask could start in permissive and only transition to
> enforcing at the deprivilege point.  Kinda gross, but it works without
> needing a transition.
> 
> To reiterate, XSM isn't really appropriate to enforce anything
> internal to Xen.  We are working around the need to go through hook
> points during correct operation.  Code exec in Xen means all bets are
> off.  Memory writes to Xen data mean the XSM checks can be disabled
> (flip Flask to permissive) or bypassed (set d->is_privileged or change
> d->ssid).  We shouldn't lose sight of this when we talk about
> deprivileging the idle domain.

I would far prefer a transition than trying to reason about whether
flask should be in permissive or enforcing based on this state change in
combination of command line setting.

v/r
dps