[PATCH v10 0/3] Adds starting the idle domain privileged

Daniel P. Smith posted 3 patches 1 year, 8 months ago
Failed in applying to current master (apply log)
tools/flask/policy/modules/dom0.te     |  3 ++
tools/flask/policy/modules/domU.te     |  3 ++
tools/flask/policy/modules/xen.if      |  7 +++
tools/flask/policy/modules/xen.te      |  1 +
tools/flask/policy/policy/initial_sids |  1 +
xen/arch/arm/setup.c                   |  3 ++
xen/arch/x86/setup.c                   |  4 ++
xen/common/sched/core.c                |  7 ++-
xen/include/xsm/dummy.h                | 17 +++++++
xen/include/xsm/xsm.h                  |  6 +++
xen/xsm/dummy.c                        |  1 +
xen/xsm/flask/hooks.c                  | 67 ++++++++++++++++++++------
xen/xsm/flask/policy/initial_sids      |  1 +
13 files changed, 104 insertions(+), 17 deletions(-)
[PATCH v10 0/3] Adds starting the idle domain privileged
Posted by Daniel P. Smith 1 year, 8 months ago
This series makes it so that the idle domain is started privileged under the
default policy, which the SILO policy inherits, and under the flask policy. It
then introduces a new one-way XSM hook, xsm_transition_running, that is hooked
by an XSM policy to transition the idle domain to its running privilege level.

Patch 3 is an important one, as first it addresses the issue raised under an
RFC late last year by Jason Andryuk regarding the awkward entanglement of
flask_domain_alloc_security() and flask_domain_create(). Second, it helps
articulate why it is that the hypervisor should go through the access control
checks, even when it is doing the action itself. The issue at hand is not that
the hypervisor could be influenced to go around these check. The issue is these
checks provides a configurable way to express the execution flow that the
hypervisor should enforce. Specifically with this change, it is now possible
for an owner of a dom0less or hyperlaunch system to express a policy where the
hypervisor will enforce that no dom0 will be constructed, regardless of what
boot construction details were provided to it. Likewise, an owner that does not
want to see dom0less or hyperlaunch to be used can enforce that the hypervisor
will only construct a dom0 domain. This can all be accomplished without the
need to rebuild the hypervisor with these features enabled or disabled.

Changes in v10:
- rewrote patch 3 commit message
- fixed typos in patch 3
- reworked logic in flask_domain_create() to be simpler and not result in
  changing the domain security struct before the access check fails

Changes in v9:
- added missing Rb/Tb to patch 1
- corrected the flask policy macro in patch 2 to allow domain create
- added patch 3 to address allowing the hypervisor create more than 1 domain

Changes in v8:
- adjusted panic messages in arm and x86 setup.c to be less than 80cols
- fixed comment line that went over 80col
- added line in patch #1 commit message to clarify the need is for domain
  creation

Changes in v7:
- adjusted error message in default and flask xsm_set_system_active hooks
- merged panic messages in arm and x86 setup.c to a single line

Changes in v6:
- readded the setting of is_privileged in flask_set_system_active()
- clarified comment on is_privileged in flask_set_system_active()
- added ASSERT on is_privileged and self_sid in flask_set_system_active()
- fixed err code returned on Arm for xsm_set_system_active() panic message

Changes in v5:
- dropped setting is_privileged in flask_set_system_active()
- added err code returned by xsm_set_system_active() to panic message

Changes in v4:
- reworded patch 1 commit messaged
- fixed whitespace to coding style
- fixed comment to coding style

Changes in v3:
- renamed *_transition_running() to *_set_system_active()
- changed the XSM hook set_system_active() from void to int return
- added ASSERT check for the expected privilege level each XSM policy expected
- replaced a check against is_privileged in each arch with checking the return
  value from the call to xsm_set_system_active()

Changes in v2:
- renamed flask_domain_runtime_security() to flask_transition_running()
- added the missed assignment of self_sid

Daniel P. Smith (3):
  xsm: create idle domain privileged and demote after setup
  flask: implement xsm_set_system_active
  xsm: refactor flask sid alloc and domain check

 tools/flask/policy/modules/dom0.te     |  3 ++
 tools/flask/policy/modules/domU.te     |  3 ++
 tools/flask/policy/modules/xen.if      |  7 +++
 tools/flask/policy/modules/xen.te      |  1 +
 tools/flask/policy/policy/initial_sids |  1 +
 xen/arch/arm/setup.c                   |  3 ++
 xen/arch/x86/setup.c                   |  4 ++
 xen/common/sched/core.c                |  7 ++-
 xen/include/xsm/dummy.h                | 17 +++++++
 xen/include/xsm/xsm.h                  |  6 +++
 xen/xsm/dummy.c                        |  1 +
 xen/xsm/flask/hooks.c                  | 67 ++++++++++++++++++++------
 xen/xsm/flask/policy/initial_sids      |  1 +
 13 files changed, 104 insertions(+), 17 deletions(-)

-- 
2.20.1
Re: [PATCH v10 0/3] Adds starting the idle domain privileged
Posted by Jan Beulich 1 year, 8 months ago
On 01.08.2022 20:49, Daniel P. Smith wrote:
> This series makes it so that the idle domain is started privileged under the
> default policy, which the SILO policy inherits, and under the flask policy. It
> then introduces a new one-way XSM hook, xsm_transition_running, that is hooked
> by an XSM policy to transition the idle domain to its running privilege level.
> 
> Patch 3 is an important one, as first it addresses the issue raised under an
> RFC late last year by Jason Andryuk regarding the awkward entanglement of
> flask_domain_alloc_security() and flask_domain_create(). Second, it helps
> articulate why it is that the hypervisor should go through the access control
> checks, even when it is doing the action itself. The issue at hand is not that
> the hypervisor could be influenced to go around these check. The issue is these
> checks provides a configurable way to express the execution flow that the
> hypervisor should enforce. Specifically with this change, it is now possible
> for an owner of a dom0less or hyperlaunch system to express a policy where the
> hypervisor will enforce that no dom0 will be constructed, regardless of what
> boot construction details were provided to it. Likewise, an owner that does not
> want to see dom0less or hyperlaunch to be used can enforce that the hypervisor
> will only construct a dom0 domain. This can all be accomplished without the
> need to rebuild the hypervisor with these features enabled or disabled.
> 
> Changes in v10:
> - rewrote patch 3 commit message
> - fixed typos in patch 3
> - reworked logic in flask_domain_create() to be simpler and not result in
>   changing the domain security struct before the access check fails
> 
> Changes in v9:
> - added missing Rb/Tb to patch 1
> - corrected the flask policy macro in patch 2 to allow domain create
> - added patch 3 to address allowing the hypervisor create more than 1 domain
> 
> Changes in v8:
> - adjusted panic messages in arm and x86 setup.c to be less than 80cols
> - fixed comment line that went over 80col
> - added line in patch #1 commit message to clarify the need is for domain
>   creation
> 
> Changes in v7:
> - adjusted error message in default and flask xsm_set_system_active hooks
> - merged panic messages in arm and x86 setup.c to a single line
> 
> Changes in v6:
> - readded the setting of is_privileged in flask_set_system_active()
> - clarified comment on is_privileged in flask_set_system_active()
> - added ASSERT on is_privileged and self_sid in flask_set_system_active()
> - fixed err code returned on Arm for xsm_set_system_active() panic message
> 
> Changes in v5:
> - dropped setting is_privileged in flask_set_system_active()
> - added err code returned by xsm_set_system_active() to panic message
> 
> Changes in v4:
> - reworded patch 1 commit messaged
> - fixed whitespace to coding style
> - fixed comment to coding style
> 
> Changes in v3:
> - renamed *_transition_running() to *_set_system_active()
> - changed the XSM hook set_system_active() from void to int return
> - added ASSERT check for the expected privilege level each XSM policy expected
> - replaced a check against is_privileged in each arch with checking the return
>   value from the call to xsm_set_system_active()
> 
> Changes in v2:
> - renamed flask_domain_runtime_security() to flask_transition_running()
> - added the missed assignment of self_sid
> 
> Daniel P. Smith (3):
>   xsm: create idle domain privileged and demote after setup
>   flask: implement xsm_set_system_active

Against what tree is this series? These two patches look to be in staging
already (and they have been there for almost a month), so if there are
incremental changes to make, please send incremental patches. Otherwise
please clarify whether ...

>   xsm: refactor flask sid alloc and domain check

... this change alone was meant to be (re)submitted.

Jan
Re: [PATCH v10 0/3] Adds starting the idle domain privileged
Posted by Daniel P. Smith 1 year, 8 months ago
On 8/2/22 02:15, Jan Beulich wrote:
> On 01.08.2022 20:49, Daniel P. Smith wrote:
>> This series makes it so that the idle domain is started privileged under the
>> default policy, which the SILO policy inherits, and under the flask policy. It
>> then introduces a new one-way XSM hook, xsm_transition_running, that is hooked
>> by an XSM policy to transition the idle domain to its running privilege level.
>>
>> Patch 3 is an important one, as first it addresses the issue raised under an
>> RFC late last year by Jason Andryuk regarding the awkward entanglement of
>> flask_domain_alloc_security() and flask_domain_create(). Second, it helps
>> articulate why it is that the hypervisor should go through the access control
>> checks, even when it is doing the action itself. The issue at hand is not that
>> the hypervisor could be influenced to go around these check. The issue is these
>> checks provides a configurable way to express the execution flow that the
>> hypervisor should enforce. Specifically with this change, it is now possible
>> for an owner of a dom0less or hyperlaunch system to express a policy where the
>> hypervisor will enforce that no dom0 will be constructed, regardless of what
>> boot construction details were provided to it. Likewise, an owner that does not
>> want to see dom0less or hyperlaunch to be used can enforce that the hypervisor
>> will only construct a dom0 domain. This can all be accomplished without the
>> need to rebuild the hypervisor with these features enabled or disabled.
>>
>> Changes in v10:
>> - rewrote patch 3 commit message
>> - fixed typos in patch 3
>> - reworked logic in flask_domain_create() to be simpler and not result in
>>   changing the domain security struct before the access check fails
>>
>> Changes in v9:
>> - added missing Rb/Tb to patch 1
>> - corrected the flask policy macro in patch 2 to allow domain create
>> - added patch 3 to address allowing the hypervisor create more than 1 domain
>>
>> Changes in v8:
>> - adjusted panic messages in arm and x86 setup.c to be less than 80cols
>> - fixed comment line that went over 80col
>> - added line in patch #1 commit message to clarify the need is for domain
>>   creation
>>
>> Changes in v7:
>> - adjusted error message in default and flask xsm_set_system_active hooks
>> - merged panic messages in arm and x86 setup.c to a single line
>>
>> Changes in v6:
>> - readded the setting of is_privileged in flask_set_system_active()
>> - clarified comment on is_privileged in flask_set_system_active()
>> - added ASSERT on is_privileged and self_sid in flask_set_system_active()
>> - fixed err code returned on Arm for xsm_set_system_active() panic message
>>
>> Changes in v5:
>> - dropped setting is_privileged in flask_set_system_active()
>> - added err code returned by xsm_set_system_active() to panic message
>>
>> Changes in v4:
>> - reworded patch 1 commit messaged
>> - fixed whitespace to coding style
>> - fixed comment to coding style
>>
>> Changes in v3:
>> - renamed *_transition_running() to *_set_system_active()
>> - changed the XSM hook set_system_active() from void to int return
>> - added ASSERT check for the expected privilege level each XSM policy expected
>> - replaced a check against is_privileged in each arch with checking the return
>>   value from the call to xsm_set_system_active()
>>
>> Changes in v2:
>> - renamed flask_domain_runtime_security() to flask_transition_running()
>> - added the missed assignment of self_sid
>>
>> Daniel P. Smith (3):
>>   xsm: create idle domain privileged and demote after setup
>>   flask: implement xsm_set_system_active
> 
> Against what tree is this series? These two patches look to be in staging
> already (and they have been there for almost a month), so if there are
> incremental changes to make, please send incremental patches. Otherwise
> please clarify whether ...

No changes in the first two patches, just forgot to rebase on stable. I
will resend patch 3 standalone, which turns out to be good, as there is
one more adjustment I feel is needed after looking back at hyperlaunch
test cases.

v/r,
dps