Re: [Xen-devel] [PATCH v9.1 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general

[Daniel De Graaf]

On 10/24/2017 05:10 AM, Yi Sun wrote:

This patch renames PSR sysctl/domctl interfaces and related xsm policy to
make them be general for all resource allocation features but not only
for CAT. Then, we can resuse the interfaces for all allocation features.

Basically, it changes 'psr_cat_op' to 'psr_alloc', and remove 'CAT_' from some
macros. E.g.:
1. psr_cat_op -> psr_alloc
2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc
3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc
4. XEN_DOMCTL_PSR_CAT_SET_L3_CBM -> XEN_DOMCTL_PSR_SET_L3_CBM
5. XEN_SYSCTL_PSR_CAT_get_l3_info -> XEN_SYSCTL_PSR_get_l3_info


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH for-next 8/9] xsm: add bodge when compiling with llvm coverage support

[Daniel De Graaf]

On 10/26/2017 05:19 AM, Roger Pau Monne wrote:

llvm coverage support seems to disable some of the optimizations
needed in order to compile xsm, and the end result is that references
to __xsm_action_mismatch_detected are left in the object files.

Since coverage support cannot be used in production, introduce
__xsm_action_mismatch_detected for llvm coverage builds.

Signed-off-by: Roger Pau Monné <roger@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v3 2/7] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin

[Daniel De Graaf]

On 10/20/2017 02:14 AM, Jan Beulich wrote:

On 19.10.17 at 19:36,  wrote:

On 10/19/2017 07:58 AM, Jan Beulich wrote:

On 19.10.17 at 04:36,  wrote:

--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -516,7 +516,8 @@ static XSM_INLINE int

xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,

   static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain

*d, struct domain *t)

   {
   XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, current->domain, d) ?:
+xsm_default_action(action, current->domain, t);
   }


When all three domains are different, how does the changed
policy reflect the original "d has privilege over t" requirement?
I understand you want to relax the current condition, but this
shouldn't come at the price of granting access when access
should be denied. Nor the inverse - the current domain not
having privilege over both does also not mean d doesn't have
the necessary privilege over t.

I continue to think that you can't validly retrofit the new
intended functionality onto the existing hypercall, even if
nothing except the permission check needs to be different.

Jan


If this operation is going to be allowed at all (and I agree it has
valid use cases), then there won't be a privilege relationship between
(d) and (t) to check - they'll both be (somewhat related) domUs as far
as Xen can tell.  If this hypercall isn't used, adding a new hypercall
(subop) is the only way I'd see to do it - and that seems very redundant
as it'd need to do all the same checks except for the one about the
relationship between (d) and (t).  I don't see the reason why the
existing hypercall should deny being used for that purpose once it's
possible using other means.


One problem is, as you mention here, ...


The only possible problem that springs to mind is a restricted kernel
interface (such as the one used by QEMU in dom0 that restricts to a
single target domain) that now doesn't realize it's relaying an
operation that also requires permission over (t) after only checking
that the origin is allowed to modify (d).


... the delegation of privilege checking responsibility to a
possibly untrusted environment. Plus, as explained before,
current callers expect privilege of d over t to be validated,
which isn't happening anymore with the proposed change. If
the existing sub-op was to be modified, I think we'd need
(with c representing the current domain)
- (d over t) || ((c over d) && (c over t)) for not regressing
   the pre-existing use case,
- only (c over d) && (c over t) for not permitting something
   that isn't intended to be permitted in the new use case.
Unless the sub-op has room for adding a flag to indicate
which of the two is meant (I didn't check), I don't see a way
around adding another sub-op, no matter how similar this
would end up being.

Jan


I would say the current lack of a check for (c over t) is an oversight,
which mostly doesn't matter because the ability to modify arbitrary
memory in your target is transitive in almost any security model (c can
modify d's code to modify t, so a malicious c can compromise t anyway).
If the three domains are all different, the only way this can happen in
non-XSM is for (c) to be dom0 or for your device model to have a device
model (which I don't think is forbidden, but doubt anyone uses).

I now agree that this deserves a new subop, since this code is reached
via the stable memory_op and not just a domctl.

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

Re: [Xen-devel] [PATCH v3 2/7] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin

[Daniel De Graaf]

On 10/19/2017 08:55 PM, Zhongze Liu wrote:

2017-10-20 8:34 GMT+08:00 Zhongze Liu <blacksk...@gmail.com>:

Hi Daniel,

2017-10-20 1:36 GMT+08:00 Daniel De Graaf <dgde...@tycho.nsa.gov>:

On 10/18/2017 10:36 PM, Zhongze Liu wrote:


The original dummy xsm_map_gmfn_foregin checks if source domain has the
proper
privileges over the target domain. Under this policy, it's not allowed if
a Dom0
wants to map pages from one DomU to another, which restricts some useful
yet not
dangerous use cases of the API, such as sharing pages among DomU's by
calling
XENMEM_add_to_physmap from Dom0.

For the dummy xsm_map_gmfn_foregin, change to policy to: IFF the current
domain
has the proper privileges on (d) and (t), grant the access.

For the flask side: 1) Introduce a new av permission MMU__SHARE_MEM to
denote if
two domains can share memory through map_gmfn_foregin. 2) Change to hook
to
grant the access IFF the current domain has proper MMU privileges on (d)
and (t),
and MMU__SHARE_MEM is allowed between (d) and (t). 3) Modify the default
xen.te
to allow MMU__SHARE_MEM for normal domains that allow grant mapping/event
channels.

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blacksk...@gmail.com>

Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Cc: xen-devel@lists.xen.org
---
V3:
* Change several if statements to the GCC '... = a ?: b' extention.
* lookup the current domain in the hooks instead of passing it as an
arg
---
   tools/flask/policy/modules/xen.if   | 2 ++
   xen/include/xsm/dummy.h | 3 ++-
   xen/xsm/flask/hooks.c   | 4 +++-
   xen/xsm/flask/policy/access_vectors | 4 
   4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/flask/policy/modules/xen.if
b/tools/flask/policy/modules/xen.if
index 55437496f6..3ffd1c6239 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -127,6 +127,8 @@ define(`domain_comms', `
 domain_event_comms($1, $2)
 allow $1 $2:grant { map_read map_write copy unmap };
 allow $2 $1:grant { map_read map_write copy unmap };
+   allow $1 $2:mmu share_mem;
+   allow $2 $1:mmu share_mem;
   ')
 # domain_self_comms(domain)
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b2cd56cdc5..65e7060ad5 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -516,7 +516,8 @@ static XSM_INLINE int
xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
   static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain
*d, struct domain *t)
   {
   XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, current->domain, d) ?:
+xsm_default_action(action, current->domain, t);
   }



Same comment as below, the check between (current->domain) and (d) should
be redundant with one higher up in the call stack.  The check between
(current->domain) and (t) should remain, although this *does* result in a
relaxing of the existing permission checks on the call as Jan noted.


   static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d,
unsigned long op)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f01b4cfaaa..16103bafc9 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1199,7 +1199,9 @@ static int flask_remove_from_physmap(struct domain
*d1, struct domain *d2)
 static int flask_map_gmfn_foreign(struct domain *d, struct domain *t)
   {
-return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ |
MMU__MAP_WRITE);
+return domain_has_perm(current->domain, d, SECCLASS_MMU,
MMU__MAP_READ | MMU__MAP_WRITE) ?:
+domain_has_perm(current->domain, t, SECCLASS_MMU, MMU__MAP_READ |
MMU__MAP_WRITE) ?:
+domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);
   }



This is at least partially redundant with the higher-level permission checks
needed to get to the xenmem_add_* functions (xatp_permission_check call in
xen/common/memory.c, for example).  That check already verifies the
permission
for (current->domain) to modify (d)'s page tables.

The other two checks here look correct.


Do you mean that the checks that verify the permission for (current->domain) to
modify (d)'s page tables have already been done somewhere higher up in the
call stack so that I can eliminate them in both hooks?


Although xatp_permission_chec() does check (current->domain)'s permission over
(d), I'm not sure if this is the case for all the call paths that
would finally lead to map_gmfn_foregin().
If the answer is yes, I would ha

Re: [Xen-devel] [PATCH v3 2/7] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin

[Daniel De Graaf]

On 10/19/2017 07:58 AM, Jan Beulich wrote:

On 19.10.17 at 04:36,  wrote:

--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -516,7 +516,8 @@ static XSM_INLINE int 
xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
  static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, 
struct domain *t)
  {
  XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, current->domain, d) ?:
+xsm_default_action(action, current->domain, t);
  }


When all three domains are different, how does the changed
policy reflect the original "d has privilege over t" requirement?
I understand you want to relax the current condition, but this
shouldn't come at the price of granting access when access
should be denied. Nor the inverse - the current domain not
having privilege over both does also not mean d doesn't have
the necessary privilege over t.

I continue to think that you can't validly retrofit the new
intended functionality onto the existing hypercall, even if
nothing except the permission check needs to be different.

Jan


If this operation is going to be allowed at all (and I agree it has
valid use cases), then there won't be a privilege relationship between
(d) and (t) to check - they'll both be (somewhat related) domUs as far
as Xen can tell.  If this hypercall isn't used, adding a new hypercall
(subop) is the only way I'd see to do it - and that seems very redundant
as it'd need to do all the same checks except for the one about the
relationship between (d) and (t).  I don't see the reason why the
existing hypercall should deny being used for that purpose once it's
possible using other means.

The only possible problem that springs to mind is a restricted kernel
interface (such as the one used by QEMU in dom0 that restricts to a
single target domain) that now doesn't realize it's relaying an
operation that also requires permission over (t) after only checking
that the origin is allowed to modify (d).

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

Re: [Xen-devel] [PATCH v3 2/7] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin

[Daniel De Graaf]

On 10/18/2017 10:36 PM, Zhongze Liu wrote:

The original dummy xsm_map_gmfn_foregin checks if source domain has the proper
privileges over the target domain. Under this policy, it's not allowed if a Dom0
wants to map pages from one DomU to another, which restricts some useful yet not
dangerous use cases of the API, such as sharing pages among DomU's by calling
XENMEM_add_to_physmap from Dom0.

For the dummy xsm_map_gmfn_foregin, change to policy to: IFF the current domain
has the proper privileges on (d) and (t), grant the access.

For the flask side: 1) Introduce a new av permission MMU__SHARE_MEM to denote if
two domains can share memory through map_gmfn_foregin. 2) Change to hook to
grant the access IFF the current domain has proper MMU privileges on (d) and 
(t),
and MMU__SHARE_MEM is allowed between (d) and (t). 3) Modify the default xen.te
to allow MMU__SHARE_MEM for normal domains that allow grant mapping/event
channels.

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

[1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html

Signed-off-by: Zhongze Liu <blacksk...@gmail.com>

Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Cc: xen-devel@lists.xen.org
---
   V3:
   * Change several if statements to the GCC '... = a ?: b' extention.
   * lookup the current domain in the hooks instead of passing it as an arg
---
  tools/flask/policy/modules/xen.if   | 2 ++
  xen/include/xsm/dummy.h | 3 ++-
  xen/xsm/flask/hooks.c   | 4 +++-
  xen/xsm/flask/policy/access_vectors | 4 
  4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/flask/policy/modules/xen.if 
b/tools/flask/policy/modules/xen.if
index 55437496f6..3ffd1c6239 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -127,6 +127,8 @@ define(`domain_comms', `
domain_event_comms($1, $2)
allow $1 $2:grant { map_read map_write copy unmap };
allow $2 $1:grant { map_read map_write copy unmap };
+   allow $1 $2:mmu share_mem;
+   allow $2 $1:mmu share_mem;
  ')
  
  # domain_self_comms(domain)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b2cd56cdc5..65e7060ad5 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -516,7 +516,8 @@ static XSM_INLINE int 
xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
  static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, 
struct domain *t)
  {
  XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, current->domain, d) ?:
+xsm_default_action(action, current->domain, t);
  }


Same comment as below, the check between (current->domain) and (d) should
be redundant with one higher up in the call stack.  The check between
(current->domain) and (t) should remain, although this *does* result in a
relaxing of the existing permission checks on the call as Jan noted.


  static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, 
unsigned long op)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f01b4cfaaa..16103bafc9 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1199,7 +1199,9 @@ static int flask_remove_from_physmap(struct domain *d1, 
struct domain *d2)
  
  static int flask_map_gmfn_foreign(struct domain *d, struct domain *t)

  {
-return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE);
+return domain_has_perm(current->domain, d, SECCLASS_MMU, MMU__MAP_READ | 
MMU__MAP_WRITE) ?:
+domain_has_perm(current->domain, t, SECCLASS_MMU, MMU__MAP_READ | 
MMU__MAP_WRITE) ?:
+domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);
  }


This is at least partially redundant with the higher-level permission checks
needed to get to the xenmem_add_* functions (xatp_permission_check call in
xen/common/memory.c, for example).  That check already verifies the permission
for (current->domain) to modify (d)'s page tables.

The other two checks here look correct.

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

Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Daniel De Graaf]

On 10/17/2017 09:24 AM, Paul Durrant wrote:

Certain memory resources associated with a guest are not necessarily
present in the guest P2M.

This patch adds the boilerplate for new memory op to allow such a resource
to be priv-mapped directly, by either a PV or HVM tools domain.

NOTE: Whilst the new op is not intrinsicly specific to the x86 architecture,
   I have no means to test it on an ARM platform and so cannot verify
   that it functions correctly.

Signed-off-by: Paul Durrant <paul.durr...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v7 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general

[Daniel De Graaf]

On 10/13/2017 04:40 AM, Yi Sun wrote:

This patch renames PSR sysctl/domctl interfaces and related xsm policy to
make them be general for all resource allocation features but not only
for CAT. Then, we can resuse the interfaces for all allocation features.

Basically, it changes 'psr_cat_op' to 'psr_alloc', and remove 'CAT_' from some
macros. E.g.:
1. psr_cat_op -> psr_alloc
2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc
3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc
4. XEN_DOMCTL_PSR_CAT_SET_L3_CBM -> XEN_DOMCTL_PSR_SET_L3_CBM
5. XEN_SYSCTL_PSR_CAT_get_l3_info -> XEN_SYSCTL_PSR_get_l3_info

Signed-off-by: Yi Sun <yi.y@linux.intel.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Roger Pau Monné <roger@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v6 04/12] xen: add new domctl hypercall to set grant table resource limits

[Daniel De Graaf]

On 09/13/2017 11:46 AM, Juergen Gross wrote:

Add a domctl hypercall to set the domain's resource limits regarding
grant tables. It is accepted only as long as neither
gnttab_setup_table() has been called for the domain, nor the domain
has started to run.

Signed-off-by: Juergen Gross <jgr...@suse.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v4 6/8] xen: add new domctl hypercall to set grant table resource limits

[Daniel De Graaf]

On 09/07/2017 09:47 AM, Juergen Gross wrote:

Add a domctl hypercall to set the domain's resource limits regarding
grant tables. It is accepted only as long as neither
gnttab_setup_table() has been called for the domain, nor the domain
has started to run.

Signed-off-by: Juergen Gross 
Reviewed-by: Paul Durrant 
Reviewed-by: Wei Liu 


I just realized that this only allows dom0 to set dom0's limit. You
also need to add set_gnttab_limits to the permissions allowed by
create_domain_common in xen.if for this to be useful.

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

Re: [Xen-devel] [PATCH v4 6/8] xen: add new domctl hypercall to set grant table resource limits

[Daniel De Graaf]

On 09/07/2017 09:47 AM, Juergen Gross wrote:

Add a domctl hypercall to set the domain's resource limits regarding
grant tables. It is accepted only as long as neither
gnttab_setup_table() has been called for the domain, nor the domain
has started to run.

Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Paul Durrant <paul.durr...@citrix.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH 4/6] xsm: flask: change the interface and default policy for xsm_map_gmfn_foregin

[Daniel De Graaf]

On 08/24/2017 08:39 AM, Jan Beulich wrote:

On 24.08.17 at 13:33,  wrote:

Hi Jan,

2017-08-24 14:37 GMT+08:00 Jan Beulich :

On 24.08.17 at 02:51,  wrote:

2017-08-23 17:55 GMT+08:00 Jan Beulich :

On 22.08.17 at 20:08,  wrote:

--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -525,10 +525,12 @@ static XSM_INLINE int

xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,

  return xsm_default_action(action, d1, d2);
  }

-static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain

*d, struct domain *t)

+static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain

*cd,

+   struct domain *d, struct domain

*t)

  {
  XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, cd, d) ||
+xsm_default_action(action, cd, t);
  }


... you use "or" here and ...


This might be confusing. But think of returning 0 as "allowed", the
only condition where this
statement returns a 0 is when both calls return 0 -- so it's actually
an "and". (Think of de-morgan's law.)

But as Stefano has pointed out, I should preserve the error code.


Ah, right - the code as written suggests boolean return values,
which gives it the wrong look. You really mean ?: instead of ||.
Why do you, btw, pass in current->domain (as cd) instead of
obtaining it here (just like various other hooks do)?


This was my original plan, but I'm not very sure about this, so I turn
to Julien for help, and...
Here is part of the irc log from a discussion with Julien on
#xendevel, where Julien said:

blackskygg: I think you want to pass the current domain in
parameter, i.e having 3 domains argument.
because your solution only works when XSM is not enabled (this is
the dummy callback)
when XSM is enabled, the policy would be specificed by the administrator
he needs to be able to know which domain was doing the configuration.


in flask/hooks.c there are quite a few uses of
avc_current_has_perm() in such cases, so I would think not
handing current->domain through the hook should be fine. But
of course Daniel may tell you I'm completely wrong here.

Jan


This is really just a choice of whatever looks better.  There's a very minor
performance penalty from not calling current->domain over and over, but there
might also be a performance gain if current_has_perm is not inlined and the
call results in smaller code size.

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

Re: [Xen-devel] [PATCH] xsm: policy hooks to require an IOMMU and interrupt remapping

[Daniel De Graaf]

On 08/22/2017 04:18 AM, Jan Beulich wrote:

On 18.08.17 at 23:55, <dgde...@tycho.nsa.gov> wrote:

On 08/18/2017 05:02 PM, christopher.w.cl...@gmail.com wrote:

From: Christopher Clark <christopher.cla...@baesystems.com>

Isolation of devices passed through to domains usually requires an
active IOMMU. The existing method of requiring an IOMMU is via a Xen
boot parameter ("iommu=force") which will abort boot if an IOMMU is not
available.

More graceful degradation of behaviour when an IOMMU is absent can be
achieved by enabling XSM to perform enforcement of IOMMU requirement.

This patch enables an enforceable XSM policy to specify that an IOMMU is
required for particular domains to access devices and how capable that
IOMMU must be. This allows a Xen system to boot whilst still
ensuring that an IOMMU is active before permitting device use.

Using a XSM policy ensures that the isolation properties remain enforced
even when the large, complex toolstack software changes.

For some hardware platforms interrupt remapping is a strict requirement
for secure isolation. Not all IOMMUs provide interrupt remapping.
The XSM policy can now optionally require interrupt remapping.

The device use hooks now check whether an IOMMU is:
   * Active and securely isolating:
  -- current criteria for this is that interrupt remapping is ok
   * Active but interrupt remapping is not available
   * Not active

This patch also updates the reference XSM policy to use the new
primitives, with policy entries that do not require an active IOMMU.

Signed-off-by: Christopher Clark <christopher.cla...@baesystems.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


To be honest, for this kind of a change I would have hoped for
a Reviewed-by (by you or someone else), not just an Acked-by.
Hence I'm hesitant to put the patch in right away.

Jan


I'll keep that in mind for the future.  I have looked at this patch
in depth, so you can change that to

Reviewed-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH 4/6] xsm: flask: change the interface and default policy for xsm_map_gmfn_foregin

[Daniel De Graaf]

On 08/22/2017 10:18 PM, Zhongze Liu wrote:

Hi Stefano,

2017-08-23 3:58 GMT+08:00 Stefano Stabellini <sstabell...@kernel.org>:

On Wed, 23 Aug 2017, Zhongze Liu wrote:

The original xsm_map_gmfn_foregin policy checks if source domain has the proper
privileges over the target domain. Under this policy, it's not allowed if a Dom0
wants to map pages from one DomU to another, this restricts some useful yet not
dangerous usages of the API, such as sharing pages among DomU's by calling
XENMEM_add_to_physmap from Dom0.

Change the policy to: IIF current domain has the proper privilege on the

 ^ IFF



target domain and source domain, grant the access.

References to this xsm check have also been updated to pass the current
domain as a new parameter.

This is for the proposal "Allow setting up shared memory areas between VMs
from xl config file" (see [1]).

[1] https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03047.html

Signed-off-by: Zhongze Liu <blacksk...@gmail.com>

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: xen-devel@lists.xen.org
---
  xen/arch/arm/mm.c   | 2 +-
  xen/arch/x86/mm/p2m.c   | 2 +-
  xen/include/xsm/dummy.h | 6 --
  xen/include/xsm/xsm.h   | 7 ---
  xen/xsm/flask/hooks.c   | 6 --
  5 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a810a056d7..9ec78d8c03 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one(
  return -EINVAL;
  }

-rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
+rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od);
  if ( rc )
  {
  rcu_unlock_domain(od);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e8a57d118c..a547fd00c0 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long 
fgfn,
  if ( tdom == fdom )
  goto out;

-rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom);
+rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom);
  if ( rc )
  goto out;

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 62fcea6f04..28dbc6f2a2 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -525,10 +525,12 @@ static XSM_INLINE int 
xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
  return xsm_default_action(action, d1, d2);
  }

-static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, 
struct domain *t)
+static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *cd,
+   struct domain *d, struct domain *t)
  {
  XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, cd, d) ||
+xsm_default_action(action, cd, t);


We need to preserve the returned errors:

   rc = xsm_default_action(action, cd, d);
   if (rc) return rc;
   return xsm_default_action(action, cd, t);


OK, will correct this.






  }

  static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, 
unsigned long op)
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 60c0fd6a62..a20654a803 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -85,7 +85,7 @@ struct xsm_operations {
  int (*memory_pin_page) (struct domain *d1, struct domain *d2, struct 
page_info *page);
  int (*add_to_physmap) (struct domain *d1, struct domain *d2);
  int (*remove_from_physmap) (struct domain *d1, struct domain *d2);
-int (*map_gmfn_foreign) (struct domain *d, struct domain *t);
+int (*map_gmfn_foreign) (struct domain *cd, struct domain *d, struct 
domain *t);
  int (*claim_pages) (struct domain *d);

  int (*console_io) (struct domain *d, int cmd);
@@ -372,9 +372,10 @@ static inline int xsm_remove_from_physmap(xsm_default_t 
def, struct domain *d1,
  return xsm_ops->remove_from_physmap(d1, d2);
  }

-static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, 
struct domain *t)
+static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *cd,
+struct domain *d, struct domain *t)
  {
-return xsm_ops->map_gmfn_foreign(d, t);
+return xsm_ops->map_gmfn_foreign(cd, d, t);
  }

  static inline int xsm_claim_pages(xsm_default_t def, struct domain *d)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 91146275bb..3408b6b9e1 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1165,9 +1165,11 @@ static int flas

Re: [Xen-devel] [PATCH] xsm: policy hooks to require an IOMMU and interrupt remapping

[Daniel De Graaf]

On 08/18/2017 05:02 PM, christopher.w.cl...@gmail.com wrote:

From: Christopher Clark <christopher.cla...@baesystems.com>

Isolation of devices passed through to domains usually requires an
active IOMMU. The existing method of requiring an IOMMU is via a Xen
boot parameter ("iommu=force") which will abort boot if an IOMMU is not
available.

More graceful degradation of behaviour when an IOMMU is absent can be
achieved by enabling XSM to perform enforcement of IOMMU requirement.

This patch enables an enforceable XSM policy to specify that an IOMMU is
required for particular domains to access devices and how capable that
IOMMU must be. This allows a Xen system to boot whilst still
ensuring that an IOMMU is active before permitting device use.

Using a XSM policy ensures that the isolation properties remain enforced
even when the large, complex toolstack software changes.

For some hardware platforms interrupt remapping is a strict requirement
for secure isolation. Not all IOMMUs provide interrupt remapping.
The XSM policy can now optionally require interrupt remapping.

The device use hooks now check whether an IOMMU is:
  * Active and securely isolating:
 -- current criteria for this is that interrupt remapping is ok
  * Active but interrupt remapping is not available
  * Not active

This patch also updates the reference XSM policy to use the new
primitives, with policy entries that do not require an active IOMMU.

Signed-off-by: Christopher Clark <christopher.cla...@baesystems.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

One additional note: if this type of permission expansion needs to be
applied to more permissions based on hypervisor settings, it may be
useful to look at other solutions (such as policy booleans) to implement
this logic.  However, most of those solutions are more complicated than
necessary for a single distinction like this, and the simpler ones do
not provide the same ease of verification that this version has.

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

Re: [Xen-devel] [PATCH v3] passthrough: give XEN_DOMCTL_test_assign_device more sane semantics

[Daniel De Graaf]

On 08/16/2017 08:20 AM, Jan Beulich wrote:

So far callers of the libxc interface passed in a domain ID which was
then ignored in the hypervisor. Instead, make the hypervisor honor it
(accepting DOMID_INVALID to obtain original behavior), allowing to
query whether a device can be assigned to a particular domain.

Drop XSM's test_assign_{,dt}device hooks as no longer being
individually useful.

Signed-off-by: Jan Beulich <jbeul...@suse.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v1 02/13] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general

[Daniel De Graaf]

On 08/09/2017 03:41 AM, Yi Sun wrote:

This patch renames PSR sysctl/domctl interfaces and related xsm policy to
make them be general for all resource allocation features but not only
for CAT. Then, we can resuse the interfaces for all allocation features.

Basically, it changes 'cat' to 'alloc'. E.g.:
1. psr_cat_op -> psr_alloc_op
2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc_op
3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc_op

The sysctl/domctl version numbers are bumped.

Signed-off-by: Yi Sun <yi.y@linux.intel.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 48/52] xen: add hypercall for setting parameters at runtime

[Daniel De Graaf]

On 08/14/2017 03:08 AM, Juergen Gross wrote:

Add a sysctl hypercall to support setting parameters similar to
command line parameters, but at runtime. The parameters to set are
specified as a string, just like the boot parameters.


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 38/52] xen/xsm/flask/flask_op.c: let custom parameter parsing routines return errno

[Daniel De Graaf]

On 08/14/2017 03:08 AM, Juergen Gross wrote:

Modify the custom parameter parsing routines in:

xen/xsm/flask/flask_op.c

to indicate whether the parameter value was parsed successfully.


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH 2/4] xen/flask: Switch to using bool

[Daniel De Graaf]

On 06/28/2017 07:16 AM, Andrew Cooper wrote:

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH] xsm/flask: Fix build following "xsm: correct AVC lookups for two sysctls"

[Daniel De Graaf]

On 08/10/2017 09:17 AM, Andrew Cooper wrote:

avc_current_has_perm() takes 4 arguments, not 3.  Spotted by a Travis
randconfig run which actually turned XSM on.

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>


Whoops, looks like I sent the non-build-tested patch by accident.
Thanks for catching this!

Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

[Xen-devel] [PATCH] xsm: correct AVC lookups for two sysctls

[Daniel De Graaf]

The current code was incorrectly using SECCLASS_XEN instead of
SECCLASS_XEN2, resulting in the wrong permission being checked.

GET_CPU_LEVELLING_CAPS was checking MTRR_DEL
GET_CPU_FEATURESET was checking MTRR_READ

The default XSM policy only allowed these permissions to dom0, so this
didn't result in a security issue there.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---
 xen/xsm/flask/hooks.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 819e25d3af..57be18d6d4 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -814,10 +814,12 @@ static int flask_sysctl(int cmd)
 return domain_has_xen(current->domain, XEN__TMEM_CONTROL);
 
 case XEN_SYSCTL_get_cpu_levelling_caps:
-return domain_has_xen(current->domain, XEN2__GET_CPU_LEVELLING_CAPS);
+return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+XEN2__GET_CPU_LEVELLING_CAPS);
 
 case XEN_SYSCTL_get_cpu_featureset:
-return domain_has_xen(current->domain, XEN2__GET_CPU_FEATURESET);
+return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+XEN2__GET_CPU_FEATURESET);
 
 case XEN_SYSCTL_livepatch_op:
 return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
-- 
2.13.4


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

Re: [Xen-devel] [PATCH 48/52] xen: add hypercall for setting parameters at runtime

[Daniel De Graaf]

On 08/09/2017 03:07 AM, Juergen Gross wrote:

Add a sysctl hypercall to support setting parameters similar to
command line parameters, but at runtime. The parameters to set are
specified as a string, just like the boot parameters.


Looks good, except for one thing:


+case XEN_SYSCTL_set_parameter:
+return domain_has_xen(current->domain, XEN2__SET_PARAMETER);


This needs to use SECCLASS_XEN2, but domain_has_xen uses SECCLASS_XEN.
I will be sending a patch fixing the other two incorrect users above
in that switch statement.

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

Re: [Xen-devel] [PATCH 38/52] xen/xsm/flask/flask_op.c: let custom parameter parsing routines return errno

[Daniel De Graaf]

On 08/09/2017 03:06 AM, Juergen Gross wrote:

Modify the custom parameter parsing routines in:

xen/xsm/flask/flask_op.c

to indicate whether the parameter value was parsed successfully.


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2] passthrough: give XEN_DOMCTL_test_assign_device more sane semantics

[Daniel De Graaf]

On 06/23/2017 11:00 AM, Jan Beulich wrote:

So far callers of the libxc interface passed in a domain ID which was
then ignored in the hypervisor. Instead, make the hypervisor honor it
(accepting DOMID_INVALID to obtain original behavior), allowing to
query whether a device can be assigned to a particular domain.

Signed-off-by: Jan Beulich 
---
v2: Alter the semantics to check whether the device can be assigned to
 the passed in domain.
TBD: It's not clear to me whether the test-assign XSM hooks are still
  useful this way.


As long as the only user of this hypercall is the device assignment
path, I would remove the XSM hook for test_assign and use the
assign_device hook for both test and actual.  That way, if XSM is
actually preventing the assignment, you will get the same AVC denials as
if you tried without doing the test first.  The hook should just skip the
two checks relating to (d) if it is null.

If this test hypercall were to be used as part of a query (such as
populating a list of assignable devices by trying all PCI devices listed
via sysfs), then it would make sense to have either a different hook or
a flag in the XSM hook to silence the audit messages since you aren't
actually trying to do the assignment yet.  That change will make the
cause of the "can't assign" error harder to diagnose, however, so unless
it's being used like that I don't think it's a good idea.

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

Re: [Xen-devel] [PATCH] passthrough: give XEN_DOMCTL_test_assign_device more sane semantics

[Daniel De Graaf]

y deny device assignment based on both the bdf and
the domain.

Do you have a particular use case in mind for your alternate hypercall?


No - I'm open to any change to it which makes the currently ignored
argument no longer ignored, without breaking existing (known and
unknown) callers of the libxc wrapper. I.e. I'm in no way opposed to
make it work the way you think it was originally meant to work; it is
just that given its current use I've come to a different conclusion as
to what the original intention may have been.


So the libxc library interface is not meant to be stable.  Before I
looked at how libxl was using it, I was going to say that we should just
remove the domid argument from that function entirely, rather than
labelling it "unused".

I suggest we ask the toolstack maintainers what kind of a function they
think would be most useful, and then we can implement that.

So, Wei and Ian (and Daniel if you're around):

At the moment, xc_test_assign_device() accepts both a domid and a device
identifier.  It will return -ENOSYS if there is no IOMMU, and

The domid, however, is ignored by Xen, and there is no possibility even
of XSM/Flask paying any attention to it because the domid is not passed
to the XSM hook.

Jan thinks the color of this shed is ugly and wants to repaint it.  I
agree that the color of this shed is ugly, but think a different color
would be more suitable.  Since the function is ultimately mainly for the
benefit of toolstacks like libxl, we'd like your input:

Option 1: Make xc_test_assign_device() explicitly take only a device
identifier.  Add another xc function which takes a domain argument,
which will return true if the BDF is assigned to that particular domain,
and false otherwise.
   1a: Leave the domid argument, but add a comment specifying it as ignored
   1b: Remove the domid argument.

Option 2: Pass the domain to the XSM callback, enabling XSM / Flask
policies that can forbid specific devices from being assigned to
specific guests.

NB that neither of us has a particular requirement for the proposed
additional functionality ("Device X is assigned to domid Y" in the case
of Option 1, or "Flask policy can allow or forbid specific devices to
specific domains" in the case of Option 2).

Any preferences?

  -George


One caveat to add to option 2: the XSM permission check done by test_assign
would only cover the PCI device resource (same as assign_device).  There
are also XSM checks that occur when assigning the MMIO and IO port regions
assigned to the device (defined by the BARs), and these ranges are not
passed to the hypervisor during the test function.  A proper test_assign
function as described above would also need to check the XSM permissions
for them, which would require adding test_io{port,mem,q}_permission
functions too.

Alternatively, you could assume that the PCI device and its associated
resources all have the same label (which will be almost always be true in a
properly configured system) and just use this as an early bail out to avoid
user mistakes.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH for-4.9] xsm: fix clang 3.5 build after c47d1d

[Daniel De Graaf]

On 04/10/2017 04:41 AM, Roger Pau Monne wrote:

On Fri, Apr 07, 2017 at 08:54:59AM -0600, Jan Beulich wrote:

On 07.04.17 at 16:34, <roger@citrix.com> wrote:

The changes introduced on c47d1d broke the clang build due to undefined
references to __xsm_action_mismatch_detected, because clang hasn't optimized
the code properly.


This starts to become annoying.


Yes, tell me about it... I don't think we can blame clang for those failures
anyway. Xen is relying on optimizations to do compile time validations, and
AFAIK this is a grey area. There's nothing in the C spec that guarantees you
that those calls will be removed.

Anyway, will send a new version shortly.


If this problem expands more, I think it would be best to restrict the check to
a particular compiler or #define (as long as it's one used in the build bot);
there's no need to do this kind of check on every build as long as it's done on
occasional builds.  Alternatively, it could be done by a static analysis tool,
but I've not looked into how to do that with Coverity.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH for-4.9 v3 1/3] xsm: fix clang 3.5 build after c47d1d

[Daniel De Graaf]

On 04/10/2017 09:34 AM, Roger Pau Monne wrote:

The changes introduced on c47d1d broke the clang build due to undefined
references to __xsm_action_mismatch_detected, because clang hasn't optimized
the code properly. The following patch allows the clang build to work again,
while keeping the same functionality.

Signed-off-by: Roger Pau Monné <roger@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v6] altp2m: Introduce external-only and limited use-cases

[Daniel De Graaf]

On 04/04/2017 11:24 AM, Tamas K Lengyel wrote:

Currently setting altp2mhvm=1 in the domain configuration allows access to the
altp2m interface for both in-guest and external privileged tools. This poses
a problem for use-cases where only external access should be allowed, requiring
the user to compile Xen with XSM enabled to be able to appropriately restrict
access.

In this patch we deprecate the altp2mhvm domain configuration option and
introduce the altp2m option, which allows specifying if by default the altp2m
interface should be external-only or limited. The information is stored in
HVM_PARAM_ALTP2M which we now define with specific XEN_ALTP2M_* modes.
If external mode is selected, the XSM check is shifted to use XSM_DM_PRIV
type check, thus restricting access to the interface by the guest itself. Note
that we keep the default XSM policy untouched. Users of XSM who wish to enforce
external mode for altp2m can do so by adjusting their XSM policy directly,
as this domain config option does not override an active XSM policy.

Also, as part of this patch we adjust the hvmop handler to require
HVM_PARAM_ALTP2M to be of a type other then disabled for all ops. This has been
previously only required for get/set altp2m domain state, all other options
were gated on altp2m_enabled. Since altp2m_enabled only gets set during set
altp2m domain state, this change introduces no new requirements to the other
ops but makes it more clear that it is required for all ops.

Signed-off-by: Tamas K Lengyel <tamas.leng...@zentific.com>
Signed-off-by: Sergej Proskurin <prosku...@sec.in.tum.de>
Acked-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v5] altp2m: Allow specifying external-only use-case

[Daniel De Graaf]

On 03/22/2017 02:07 PM, Tamas K Lengyel wrote:

Currently setting altp2mhvm=1 in the domain configuration allows access to the
altp2m interface for both in-guest and external privileged tools. This poses
a problem for use-cases where only external access should be allowed, requiring
the user to compile Xen with XSM enabled to be able to appropriately restrict
access.

In this patch we deprecate the altp2mhvm domain configuration option and
introduce the altp2m option, which allows specifying if by default the altp2m
interface should be external-only. The information is stored in
HVM_PARAM_ALTP2M which we now define with specific XEN_ALTP2M_* modes.
If external mode is selected, the XSM check is shifted to use XSM_DM_PRIV
type check, thus restricting access to the interface by the guest itself. Note
that we keep the default XSM policy untouched. Users of XSM who wish to enforce
external mode for altp2m can do so by adjusting their XSM policy directly,
as this domain config option does not override an active XSM policy.

Also, as part of this patch we adjust the hvmop handler to require
HVM_PARAM_ALTP2M to be of a type other then disabled for all ops. This has been
previously only required for get/set altp2m domain state, all other options
were gated on altp2m_enabled. Since altp2m_enabled only gets set during set
altp2m domain state, this change introduces no new requirements to the other
ops but makes it more clear that it is required for all ops.

Signed-off-by: Tamas K Lengyel <tamas.leng...@zentific.com>
Signed-off-by: Sergej Proskurin <prosku...@sec.in.tum.de>


I think the XSM-enabled case using the default types should have the same
flexibility as the XSM-disabled case.  I agree that it is useful to be able
to restrict the p2m features based on policy, and I don't think that it's
useful to expand the number of XSM permissions here.  In that case, the best
way to proceed would be to require that both the domain configuration and
XSM policy must allow the action (similar to how SELinux file controls and
UNIX permissions interact).  Currently, enabling XSM effectively forces the
value of this setting to "mixed", and "limited" is impossible to use with XSM.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0

[Daniel De Graaf]

On 03/06/2017 07:31 AM, Roger Pau Monne wrote:

There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
working as expected, and vpmu.o ends up with a reference to
__xsm_action_mismatch_detected which makes the build fail:

[...]
ld-melf_x86_64_fbsd  -T xen.lds -N prelink.o  \
xen/common/symbols-dummy.o -o xen/.xen-syms.0
prelink.o: In function `xsm_default_action':
xen/include/xsm/dummy.h:80: undefined reference to 
`__xsm_action_mismatch_detected'
xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 
against undefined symbol `__xsm_action_mismatch_detected'
ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' isn't 
defined

Then doing a search in the objects files:

# find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
  'for filename; do nm "$filename" | \
  grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
xen/arch/x86/prelink.o
xen/arch/x86/cpu/vpmu.o
xen/arch/x86/cpu/built_in.o
xen/arch/x86/built_in.o

The current patch is the only way I've found to fix this so far, by simply
moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
XENPMU_* operations, instead of -EPERM when called by a privileged domain.

Signed-off-by: Roger Pau Monné <roger@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

This also looks like a good patch for backporting.  The best alternative I
can think of is to disable the mismatch detection in non-DEBUG builds, but
I'd rather not do that unless this problem expands more than it has.

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

Re: [Xen-devel] [PATCH] flask: fix build after the introduction of DMOP

[Daniel De Graaf]

On 01/25/2017 05:43 AM, Wei Liu wrote:

In 58cbc034 send_irq permission was removed but there was still
reference to it in policy file. Remove the stale reference.

And now we also need dm permission. Add that.

Signed-off-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH] [incremental] xsm/build: Further build fixes following the DMop series

[Daniel De Graaf]

On 01/25/2017 09:24 AM, Andrew Cooper wrote:

Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Daniel De Graaf <dgde...@tycho.nsa.gov>
CC: Paul Durrant <paul.durr...@citrix.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>

Might be better to merge into one single patch when committed?


Either way (combined with prior patch, original series, or alone):

Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH v4 3/8] dm_op: convert HVMOP_track_dirty_vram

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

The handle type passed to the underlying shadow and hap functions is
changed for compatibility with the new hypercall buffer.

NOTE: This patch also modifies the type of the 'nr' parameter of
  xc_hvm_track_dirty_vram() from uint64_t to uint32_t. In practice
  the value passed was always truncated to 32 bits.

Suggested-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Paul Durrant <paul.durr...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v4 7/8] dm_op: convert HVMOP_inject_trap and HVMOP_inject_msi

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

NOTE: This patch also modifies the types of the 'vector', 'type' and
  'insn_len' arguments of xc_hvm_inject_trap() from uint32_t to
  uint8_t. In practice the values passed were always truncated to
  8 bits.

Suggested-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Paul Durrant <paul.durr...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH v4 6/8] dm_op: convert HVMOP_set_mem_type

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

This patch removes the need for handling HVMOP restarts, so that
infrastructure is removed.

NOTE: This patch also modifies the type of the 'nr' argument of
  xc_hvm_set_mem_type() from uint64_t to uint32_t. In practice the
  value passed was always truncated to 32 bits.

Suggested-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Paul Durrant <paul.durr...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v4 5/8] dm_op: convert HVMOP_modified_memory

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

This patch introduces code to handle DMOP continuations.

NOTE: This patch also modifies the type of the 'nr' argument of
  xc_hvm_modified_memory() from uint64_t to uint32_t. In practice the
  value passed was always truncated to 32 bits.

Suggested-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Paul Durrant <paul.durr...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v4 4/8] dm_op: convert HVMOP_set_pci_intx_level, HVMOP_set_isa_irq_level, and...

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

... HVMOP_set_pci_link_route

These HVMOPs were exposed to guests so their definitions need to be
preserved for compatibility. This patch therefore updates
__XEN_LATEST_INTERFACE_VERSION__ to 0x00040900 and makes the HVMOP
defintions conditional on __XEN_INTERFACE_VERSION__ less than that value.

NOTE: This patch also widens the 'domain' parameter of
  xc_hvm_set_pci_intx_level() from a uint8_t to a uint16_t.

Suggested-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Paul Durrant <paul.durr...@citrix.com>
---
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

This patch (and the previous one) remove some of the permissions from
the create_domain_common macro without adding the replacement permission.
This would be a problem if the domain builder code used them, but a quick
grep seems to say that they're not actually used by the domain builder.
It looks like this ended up as a bonus improvement from this change.

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

Re: [Xen-devel] [PATCH v4 2/8] dm_op: convert HVMOP_*ioreq_server*

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

The definitions of HVM_IOREQSRV_BUFIOREQ_* have to persist as they are
already in use by callers of the libxc interface.

Suggested-by: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Paul Durrant <paul.durr...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v4 1/8] public / x86: Introduce __HYPERCALL_dm_op...

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

...as a set of hypercalls to be used by a device model.

As stated in the new docs/designs/dm_op.markdown:

"The aim of DMOP is to prevent a compromised device model from
compromising domains other then the one it is associated with. (And is
therefore likely already compromised)."

See that file for further information.

This patch simply adds the boilerplate for the hypercall.

Signed-off-by: Paul Durrant <paul.durr...@citrix.com>
Suggested-by: Ian Jackson <ian.jack...@citrix.com>
Suggested-by: Jennifer Herbert <jennifer.herb...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH] xsm: allow relevant permission during migrate and gpu-passthrough.

[Daniel De Graaf]

On 12/19/2016 11:03 PM, Doug Goldstein wrote:

On 12/19/16 10:02 AM, Doug Goldstein wrote:

On 12/14/16 3:09 PM, Daniel De Graaf wrote:

On 12/12/2016 09:00 AM, Anshul Makkar wrote:

During guest migrate allow permission to prevent
spurious page faults.
Prevents these errors:
d73: Non-privileged (73) attempt to map I/O space 

avc: denied  { set_misc_info } for domid=0 target=11
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

GPU passthrough for hvm guest:
avc:  denied  { send_irq } for domid=0 target=10
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=hvm

Signed-off-by: Anshul Makkar <anshul.mak...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>



Daniel,

Should this be backported to 4.8?



Yes, I would consider this a candidate for backporting.


FWIW, Daniel's email is bouncing. Anshul, do you want to test/confirm?


I believe this is fixed now; my email server was changed while I was gone
for the holiday and apparently the change was not tested properly.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH v6 01/12] domctl: Add XEN_DOMCTL_acpi_access

[Daniel De Graaf]

On 01/03/2017 09:04 AM, Boris Ostrovsky wrote:

This domctl will allow toolstack to read and write some
ACPI registers. It will be available to both x86 and ARM
but will be implemented first only for x86

Signed-off-by: Boris Ostrovsky <boris.ostrov...@oracle.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>



--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH] xsm: allow relevant permission during migrate and gpu-passthrough.

[Daniel De Graaf]

On 12/12/2016 09:00 AM, Anshul Makkar wrote:

During guest migrate allow permission to prevent
spurious page faults.
Prevents these errors:
d73: Non-privileged (73) attempt to map I/O space 

avc: denied  { set_misc_info } for domid=0 target=11
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

GPU passthrough for hvm guest:
avc:  denied  { send_irq } for domid=0 target=10
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=hvm

Signed-off-by: Anshul Makkar <anshul.mak...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH 07/11] docs: move vtpm from misc to man

[Daniel De Graaf]

On 12/09/2016 11:17 AM, Cédric Bosdonnat wrote:

vtpm.txt is referenced in xl.cfg man page. Convert it to pod,
move it to the man folder and update the reference.

Signed-off-by: Cédric Bosdonnat <cbosdon...@suse.com>


Since this manpage only describes Xen's vTPM implementation, and
Xen is not the only vTPM that exists in Linux (there's a Linux
kernel "vtpm_proxy" interface and another ibmvtpm module), I think
it needs be named something like "xen-vtpm".  The same applies to
patch 8 (vtpmmgr) as that manpage (and software) is Xen-specific.

The POD sources look correct, though I have not compiled & looked
at the resulting manpages.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH] Fix misleading indentation warnings

[Daniel De Graaf]

On 11/10/2016 04:23 AM, Cédric Bosdonnat wrote:

Gcc6 build reports misleading indentation as warnings. Fix a few
warnings in stubdom.

Signed-off-by: Cédric Bosdonnat <cbosdon...@suse.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH 01/10] x86/domctl: Add XEN_DOMCTL_set_avail_vcpus

[Daniel De Graaf]

On 11/06/2016 04:42 PM, Boris Ostrovsky wrote:

This domctl is called when a VCPU is hot-(un)plugged to a guest (via
'xl vcpu-set'). While this currently is only intended to be needed by
PVH guests we will call this domctl for all (x86) guests for consistency.

Signed-off-by: Boris Ostrovsky <boris.ostrov...@oracle.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

[Xen-devel] [PATCH] xsm: add missing permissions discovered in testing

[Daniel De Graaf]

Add two missing allow rules:

1. Device model domain construction uses getvcpucontext, discovered by
Andrew Cooper in an (apparently) unrelated bisection.

2. When a domain is destroyed with a device passthrough active, the
calls to remove_{irq,ioport,iomem} can be made by the hypervisor itself
(which results in an XSM check with the source xen_t).  It does not make
sense to deny these permissions; no domain should be using xen_t, and
forbidding the hypervisor from performing cleanup is not useful.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
---
 tools/flask/policy/modules/xen.if | 2 +-
 tools/flask/policy/modules/xen.te | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/modules/xen.if 
b/tools/flask/policy/modules/xen.if
index d83f031..eb646f5 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -49,7 +49,7 @@ define(`create_domain_common', `
allow $1 $2:domain { create max_vcpus setdomainmaxmem setaddrsize
getdomaininfo hypercall setvcpucontext getscheduler
getvcpuinfo getaddrsize getaffinity setaffinity
-   settime setdomainhandle };
+   settime setdomainhandle getvcpucontext };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
psr_cmt_op psr_cat_op soft_reset };
diff --git a/tools/flask/policy/modules/xen.te 
b/tools/flask/policy/modules/xen.te
index b52edc2..0cff2df 100644
--- a/tools/flask/policy/modules/xen.te
+++ b/tools/flask/policy/modules/xen.te
@@ -49,6 +49,10 @@ type ioport_t, resource_type;
 type iomem_t, resource_type;
 type device_t, resource_type;
 
+# Domain destruction can result in some access checks for actions performed by
+# the hypervisor.  These should always be allowed.
+allow xen_t resource_type : resource { remove_irq remove_ioport remove_iomem };
+
 

 #
 # Policy constraints
-- 
2.7.4


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

Re: [Xen-devel] [PATCH for-4.8] flask: build policy in different locations

[Daniel De Graaf]

On 10/28/2016 11:17 AM, Wei Liu wrote:

The flask policy can be build twice -- one for hypervisor and one for
tools.

Before this patch, everything is built inside tools/flask/policy
directory.  It is possible to have a race to write to the same output
file when running parallel builds.

Prepend output file names with FLASK_BUILD_DIR. Hypervisor and tools
build will set that variable to different directories, so that we can
be safe from races.

Adjust other bits of the build system as needed.

Signed-off-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

Pulling the definition of POLICY_FILENAME out of Makefile.common might
remove the need for the cmp||cp line in the xen-side Makefile, but that
probably belongs in another patch.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH] flask: add gcov_op check

[Daniel De Graaf]

On 10/13/2016 10:37 AM, Wei Liu wrote:

Signed-off-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH] XSM: drop pointless uses of __FUNCTION__

[Daniel De Graaf]

On 08/24/2016 04:06 AM, Jan Beulich wrote:

Non-debugging message text should be (and is in the cases here)
distinguishable without also logging function names.

Signed-off-by: Jan Beulich <jbeul...@suse.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v3 36/38] altp2m: Allow specifying external-only use-case

[Daniel De Graaf]

On 08/16/2016 06:17 PM, Sergej Proskurin wrote:

From: Tamas K Lengyel <tamas.leng...@zentific.com>

Currently setting altp2mhvm=1 in the domain configuration allows access to the
altp2m interface for both in-guest and external privileged tools. This poses
a problem for use-cases where only external access should be allowed, requiring
the user to compile Xen with XSM enabled to be able to appropriately restrict
access.

In this patch we deprecate the altp2mhvm domain configuration option and
introduce the altp2m option, which allows specifying if by default the altp2m
interface should be external-only. The information is stored in
HVM_PARAM_ALTP2M which we now define with specific XEN_ALTP2M_* modes.
If external_only mode is selected, the XSM check is shifted to use XSM_DM_PRIV
type check, thus restricting access to the interface by the guest itself. Note
that we keep the default XSM policy untouched. Users of XSM who wish to enforce
external_only mode for altp2m can do so by adjusting their XSM policy directly,
as this domain config option does not override an active XSM policy.

Also, as part of this patch we adjust the hvmop handler to require
HVM_PARAM_ALTP2M to be of a type other then disabled for all ops. This has been
previously only required for get/set altp2m domain state, all other options
were gated on altp2m_enabled. Since altp2m_enabled only gets set during set
altp2m domain state, this change introduces no new requirements to the other
ops but makes it more clear that it is required for all ops.

Signed-off-by: Tamas K Lengyel <tamas.leng...@zentific.com>
Signed-off-by: Sergej Proskurin <prosku...@sec.in.tum.de>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2] domctl: relax getdomaininfo permissions

[Daniel De Graaf]

On 08/11/2016 07:33 AM, Jan Beulich wrote:

On 05.08.16 at 13:20, <jbeul...@suse.com> wrote:


Daniel,

I've only now realized that I forgot to Cc you on this v2.

Jan


Qemu needs access to this for the domain it controls, both due to it
being used by xc_domain_memory_mapping() (which qemu calls) and the
explicit use in hw/xenpv/xen_domainbuild.c:xen_domain_poll(). Extend
permissions to that of any "ordinary" domctl: A domain controlling the
targeted domain can invoke this operation for that target domain (which
is being achieved by no longer passing NULL to xsm_domctl()).

This at once avoids a for_each_domain() loop when the ID of an
existing domain gets passed in.

Reported-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeul...@suse.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

[...]

I know there had been an alternative patch suggestion, but that one
doesn't seem have seen a formal submission so far, so here is my
original proposal.

I wonder what good the duplication of the returned domain ID does: I'm
tempted to remove the one in the command-specific structure. Does
anyone have insight into why it was done that way?

I further wonder why we have XSM_OTHER: The respective conversion into
other XSM_* values in xsm/dummy.h could as well move into the callers,
making intentions more obvious when looking at the actual code.


The XSM_* values are not actually present in the XSM hook functions, so
they have to be a static value per function.  Otherwise, the dummy XSM
module won't have enough information to make the same decision as the
inlined dummy.h version does.

An alternate solution would be to add an explicit action parameter to
the hooks that currently use XSM_OTHER, but that mostly just moves the
conversion switch statement around and adds a pointless computation in
the case when the parameter is not used.


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

Re: [Xen-devel] [PATCH v2 2/2] x86/altp2m: allow specifying external-only use-case

[Daniel De Graaf]

On 08/11/2016 10:51 AM, Jan Beulich wrote:

On 11.08.16 at 16:37, <tamas.leng...@zentific.com> wrote:

On Aug 11, 2016 06:02, "Jan Beulich" <jbeul...@suse.com> wrote:



On 10.08.16 at 17:00, <tamas.leng...@zentific.com> wrote:

@@ -5238,18 +5238,19 @@ static int do_altp2m_op(
 goto out;
 }

-if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
+if ( !d->arch.hvm_domain.params[HVM_PARAM_ALTP2M] )
+{
+rc = -EINVAL;
+goto out;
+}
+
+if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d,
+d->arch.hvm_domain.params[HVM_PARAM_ALTP2M])) )


I'm sorry that this didn't occur to me on v1 already, but is there
really a need for passing this extra argument, when the callee
could - if it cared in the first place - read the value itself?


I'm not sure if it's ok to have xsm poke around in arch specific parts like
this. We are adding this hvm param for ARM in another series but still..


Daniel, what's your opinion?

Jan


XSM does have some required arch-specific knowledge already (x86 IO port
labeling, in particular), so it's really a style question.  I'd prefer the
form with the value passed in so that it's clearer what the XSM check is
inspecting to determine what to do, especially in this case where it changes
what permissions are actually being enforced (in the non-FLASK case).

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH] xsm: don't require configuring tools to build xen xsm blob

[Daniel De Graaf]

On 07/25/2016 11:22 AM, Wei Liu wrote:

Starting from 08cffe66 ("xsm: add a default policy to .init.data") we
can attach a xsm policy blob to hypervisor. To build that policy blob
now hypervisor build system needs to enter tools directory.

The expectation for hypervisor and tools build systems is different. We
don't want xen build system to depend on configure but we want tools
build system to. That commit broke this expectation because it required
users to run configure before building hypervisor. This broke ARM build
because ARM developers normally build hypervisor and tools separately
(and possibly on different platforms). It can also break x86 if
developers don't run configure before building hypervisor with XSM on.

To fix it, move major part of tools/flask/policy/Makefile into
Makefile.common and create tools only Makefile to include that common
Makefile. Hypervisor Makefile will use Makefile.common to build xsm
policy.

Signed-off-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

Thanks for fixing this; I intended the build to remain separate but
never actually de-configured a build tree to test.

Using git-send-email -C would make the patch email clearer.

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

Re: [Xen-devel] [PATCH] XSM-docs: Flask operates on domain types and not on individual domain. Updated the documentation to reflect this.

[Daniel De Graaf]

On 07/19/2016 11:21 AM, anshul makkar wrote:

On 19/07/16 14:30, Doug Goldstein wrote:

On 7/19/16 4:05 AM, Anshul Makkar wrote:

Signed-off-by: Anshul Makkar <anshul.mak...@citrix.com>
---
  * Resending the patch due to incomplete subject in the previous patch.

  docs/misc/xsm-flask.txt | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
---
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 62f15dd..bf8bb6e 100644
  Some examples of what FLASK can do:
- - Prevent two domains from communicating via event channels or grants
- - Control which domains can use device passthrough (and which devices)
+ - Prevent two domains types from communicating via event channels or grants
+ - Control which type of domains can use device passthrough (and which devices)


I disagree with this snippet. This is an example of what you can do with
FLASK. You can use flask to do those two actions. Adding the word
"types" in there takes it from being a concrete example to being more
ambiguous.

"Prevent domains belonging to different types to communicate via event channels or 
grants". Does this sounds better.

I think that its important to use the word "type" so that user doesn't get a 
wrong impression that the policy is per domain, while in actual its per type.


I think it would be clearer to leave the examples as is, but add a sentence to 
the
following paragraph about how the policy is written based on types.

For the other changes, I agree Doug's rewording is a bit clearer than the 
original.

--
Daniel De Graaf
National Security Agency

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

[Xen-devel] [PATCH v6 2/2] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

The XSM policy is not moved out of tools because that remains the
primary location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---

No changes from v5.

 Config.mk   |  6 ++
 INSTALL | 10 --
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/common/Kconfig  | 20 
 xen/include/xsm/xsm.h   |  5 +
 xen/xsm/flask/.gitignore|  1 +
 xen/xsm/flask/Makefile  | 13 -
 xen/xsm/flask/gen-policy.py | 23 +++
 xen/xsm/xsm_core.c  |  8 
 10 files changed, 107 insertions(+), 25 deletions(-)
 create mode 100644 xen/xsm/flask/.gitignore
 create mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/Config.mk b/Config.mk
index 723e129..01316ae 100644
--- a/Config.mk
+++ b/Config.mk
@@ -147,6 +147,12 @@ export XEN_HAS_BUILD_ID=y
 build_id_linker := --build-id=sha1
 endif
 
+ifndef XEN_HAS_CHECKPOLICY
+CHECKPOLICY ?= checkpolicy
+XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && 
echo y || echo n)
+export XEN_HAS_CHECKPOLICY
+endif
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
diff --git a/INSTALL b/INSTALL
index 616a67a..9759354 100644
--- a/INSTALL
+++ b/INSTALL
@@ -269,10 +269,16 @@ Building the python tools may fail unless certain options 
are passed to
 setup.py. Config.mk contains additional info how to use this variable.
 PYTHON_PREFIX_ARG=
 
-The hypervisor may be build with XSM/Flask support, which can be changed
+The hypervisor may be built with XSM/Flask support, which can be changed
 by running:
 make -C xen menuconfig
-and enabling XSM/Flask in the 'Common Features' menu.
+and enabling XSM/Flask in the 'Common Features' menu.  A security policy
+is required to use XSM/Flask; if the SELinux policy compiler is
+available, the policy from tools can be included in the hypervisor.
+This option is enabled by default if XSM is enabled and the compiler
+(checkpolicy) is found.  The location of this executable can be set
+using the environment variable.
+CHECKPOLICY=
 
 Do a build for coverage.
 coverage=y
diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 2a088ca..5500242 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -712,13 +712,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@

[Xen-devel] [PATCH v6 1/2] xsm: rework policy_buffer globals

[Daniel De Graaf]

This makes the buffers function parameters instead of globals, in
preparation for adding alternate locations for the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---

Changes since v5:
 - Adjusted __init annotation placement
 - Removed unneeded cast to char*

 xen/include/xsm/xsm.h| 13 ++---
 xen/xsm/flask/hooks.c|  2 +-
 xen/xsm/flask/include/security.h |  2 +-
 xen/xsm/flask/ss/policydb.h  |  2 +-
 xen/xsm/flask/ss/services.c  |  2 +-
 xen/xsm/xsm_core.c   | 17 +++--
 xen/xsm/xsm_policy.c | 21 ++---
 7 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4b8843d..e83dca2 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -43,9 +43,6 @@ enum xsm_default {
 };
 typedef enum xsm_default xsm_default_t;
 
-extern char *policy_buffer;
-extern u32 policy_size;
-
 struct xsm_operations {
 void (*security_domaininfo) (struct domain *d,
 struct xen_domctl_getdomaininfo *info);
@@ -740,12 +737,14 @@ extern int xsm_multiboot_init(unsigned long *module_map,
   void *(*bootstrap_map)(const module_t *));
 extern int xsm_multiboot_policy_init(unsigned long *module_map,
  const multiboot_info_t *mbi,
- void *(*bootstrap_map)(const module_t *));
+ void *(*bootstrap_map)(const module_t *),
+ void **policy_buffer,
+ size_t *policy_size);
 #endif
 
 #ifdef CONFIG_HAS_DEVICE_TREE
 extern int xsm_dt_init(void);
-extern int xsm_dt_policy_init(void);
+extern int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
 extern bool has_xsm_magic(paddr_t);
 #endif
 
@@ -755,9 +754,9 @@ extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
 
 #ifdef CONFIG_FLASK
-extern void flask_init(void);
+extern void flask_init(const void *policy_buffer, size_t policy_size);
 #else
-static inline void flask_init(void)
+static inline void flask_init(const void *policy_buffer, size_t policy_size)
 {
 }
 #endif
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 2692a6f..ec6f5b4 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1815,7 +1815,7 @@ static struct xsm_operations flask_ops = {
 .xen_version = flask_xen_version,
 };
 
-__init void flask_init(void)
+void __init flask_init(const void *policy_buffer, size_t policy_size)
 {
 int ret = -ENOENT;
 
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index 1da020d..ec8b442 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -52,7 +52,7 @@ enum flask_bootparam_t {
 extern enum flask_bootparam_t flask_bootparam;
 extern int flask_mls_enabled;
 
-int security_load_policy(void * data, size_t len);
+int security_load_policy(const void *data, size_t len);
 
 struct av_decision {
 u32 allowed;
diff --git a/xen/xsm/flask/ss/policydb.h b/xen/xsm/flask/ss/policydb.h
index 238a042..d3b409a 100644
--- a/xen/xsm/flask/ss/policydb.h
+++ b/xen/xsm/flask/ss/policydb.h
@@ -277,7 +277,7 @@ extern int policydb_read(struct policydb *p, void *fp);
 #define TARGET_XEN_OLD 0
 
 struct policy_file {
-char *data;
+const char *data;
 size_t len;
 };
 
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index 86f94c9..b2c5c44 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1353,7 +1353,7 @@ static int security_preserve_bools(struct policydb *p);
  * This function will flush the access vector cache after
  * loading the new policy.
  */
-int security_load_policy(void *data, size_t len)
+int security_load_policy(const void *data, size_t len)
 {
 struct policydb oldpolicydb, newpolicydb;
 struct sidtab oldsidtab, newsidtab;
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 8df1a3c..3d132be 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -36,7 +36,7 @@ static inline int verify(struct xsm_operations *ops)
 return 0;
 }
 
-static int __init xsm_core_init(void)
+static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 {
 if ( verify(_xsm_ops) )
 {
@@ -46,7 +46,7 @@ static int __init xsm_core_init(void)
 }
 
 xsm_ops = _xsm_ops;
-flask_init();
+flask_init(policy_buffer, policy_size);
 
 return 0;
 }
@@ -57,12 +57,15 @@ int __init xsm_multiboot_init(unsigned long *module_map,
   void *(*bootstrap_map)(const module_t *))
 {
 int ret = 0;
+void *policy_buffer = NULL;
+size_t policy_size = 0;
 
 printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
 if (

Re: [Xen-devel] [PATCH] XSM-Policy: allow source domain access to setpodtarget for ballooning.

[Daniel De Graaf]

On 07/13/2016 08:59 AM, Anshul Makkar wrote:

Access to setpodtarget is required by dom0 to set the balloon targets for
domU. The patch gives source domain (dom0) access to set this target for
domU and resolve the following permission denied error message during
ballooning :
avc:  denied  { setpodtarget } for domid=0 target=9
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

Signed-off-by: Anshul Makkar <anshul.mak...@citrix.com>


This seems to indicate that getpodtarget should also be added to the list.

Either as-is or with getpodtarget also added,
Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] default XSM policy for PCI passthrough for unlabeled resources.

[Daniel De Graaf]

On 07/06/2016 12:19 PM, anshul makkar wrote:

On 06/07/16 16:59, Daniel De Graaf wrote:

On 07/06/2016 11:34 AM, anshul makkar wrote:

Hi,


It allows the resource to be added and removed by the source domain to
target domain, but its use by target domain is blocked.


This rule only mandates the use of resource_type for resource types.  If
you are creating a new resource type, follow the example in nic_dev.te.

Agreed, but inherently it means that "use" of any unlabeled resource be it irq, 
ioport or iomem or nic_dev is restricted.


Restricted how?  The fallback types have the resource_type attribute.

Neverallow rules are actually not present in the binary policy; they act as
compile-time assertions in the policy build.




The resource can be used only if it has been labeled using
flask-label-pci command which needs to be rerun after every boot and
after every policy reload.


Yes; this gives the most control over what resources can be delegated.
Policy reloads are supposed to be rare (on a production system) and you
already need special boot scripts (or parameters) to set up the device
for passthrough, so this can be added there.  However, I agree this can
be more work than a "default" FLASK policy should require.

Try adding a module with the following rules, which should allow domU to
use unlabeled devices:

use_device(domU_t, irq_t)
use_device(domU_t, ioport_t)
use_device(domU_t, iomem_t)
use_device(domU_t, device_t)

Yes, it does work , but I have added these in delegate_device to make it 
restrict to the case where there is delegation.


This prevents using delegate_devices without allowing access to unlabeled
devices.  If you think this should be a macro, I would suggest making a new
one named something like "delegate_unlabeled_devices".

--
Daniel De Graaf
National Security Agency

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

[Xen-devel] [PATCH v5 1/2] xsm: rework policy_buffer globals

[Daniel De Graaf]

This makes the buffers function parameters instead of globals, in
preparation for adding alternate locations for the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---

This patch is new in v5.

 xen/include/xsm/xsm.h| 13 ++---
 xen/xsm/flask/hooks.c|  2 +-
 xen/xsm/flask/include/security.h |  2 +-
 xen/xsm/flask/ss/policydb.h  |  2 +-
 xen/xsm/flask/ss/services.c  |  2 +-
 xen/xsm/xsm_core.c   | 17 +++--
 xen/xsm/xsm_policy.c | 21 ++---
 7 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4b8843d..e83dca2 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -43,9 +43,6 @@ enum xsm_default {
 };
 typedef enum xsm_default xsm_default_t;
 
-extern char *policy_buffer;
-extern u32 policy_size;
-
 struct xsm_operations {
 void (*security_domaininfo) (struct domain *d,
 struct xen_domctl_getdomaininfo *info);
@@ -740,12 +737,14 @@ extern int xsm_multiboot_init(unsigned long *module_map,
   void *(*bootstrap_map)(const module_t *));
 extern int xsm_multiboot_policy_init(unsigned long *module_map,
  const multiboot_info_t *mbi,
- void *(*bootstrap_map)(const module_t *));
+ void *(*bootstrap_map)(const module_t *),
+ void **policy_buffer,
+ size_t *policy_size);
 #endif
 
 #ifdef CONFIG_HAS_DEVICE_TREE
 extern int xsm_dt_init(void);
-extern int xsm_dt_policy_init(void);
+extern int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
 extern bool has_xsm_magic(paddr_t);
 #endif
 
@@ -755,9 +754,9 @@ extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
 
 #ifdef CONFIG_FLASK
-extern void flask_init(void);
+extern void flask_init(const void *policy_buffer, size_t policy_size);
 #else
-static inline void flask_init(void)
+static inline void flask_init(const void *policy_buffer, size_t policy_size)
 {
 }
 #endif
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 2692a6f..3555907 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1815,7 +1815,7 @@ static struct xsm_operations flask_ops = {
 .xen_version = flask_xen_version,
 };
 
-__init void flask_init(void)
+__init void flask_init(const void *policy_buffer, size_t policy_size)
 {
 int ret = -ENOENT;
 
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index 1da020d..ec8b442 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -52,7 +52,7 @@ enum flask_bootparam_t {
 extern enum flask_bootparam_t flask_bootparam;
 extern int flask_mls_enabled;
 
-int security_load_policy(void * data, size_t len);
+int security_load_policy(const void *data, size_t len);
 
 struct av_decision {
 u32 allowed;
diff --git a/xen/xsm/flask/ss/policydb.h b/xen/xsm/flask/ss/policydb.h
index 238a042..d3b409a 100644
--- a/xen/xsm/flask/ss/policydb.h
+++ b/xen/xsm/flask/ss/policydb.h
@@ -277,7 +277,7 @@ extern int policydb_read(struct policydb *p, void *fp);
 #define TARGET_XEN_OLD 0
 
 struct policy_file {
-char *data;
+const char *data;
 size_t len;
 };
 
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index 86f94c9..b2c5c44 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1353,7 +1353,7 @@ static int security_preserve_bools(struct policydb *p);
  * This function will flush the access vector cache after
  * loading the new policy.
  */
-int security_load_policy(void *data, size_t len)
+int security_load_policy(const void *data, size_t len)
 {
 struct policydb oldpolicydb, newpolicydb;
 struct sidtab oldsidtab, newsidtab;
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 8df1a3c..3d132be 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -36,7 +36,7 @@ static inline int verify(struct xsm_operations *ops)
 return 0;
 }
 
-static int __init xsm_core_init(void)
+static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 {
 if ( verify(_xsm_ops) )
 {
@@ -46,7 +46,7 @@ static int __init xsm_core_init(void)
 }
 
 xsm_ops = _xsm_ops;
-flask_init();
+flask_init(policy_buffer, policy_size);
 
 return 0;
 }
@@ -57,12 +57,15 @@ int __init xsm_multiboot_init(unsigned long *module_map,
   void *(*bootstrap_map)(const module_t *))
 {
 int ret = 0;
+void *policy_buffer = NULL;
+size_t policy_size = 0;
 
 printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
 if ( XSM_MAGIC )
 {
-ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
+ret = xsm_multiboot_p

[Xen-devel] [PATCH v5 2/2] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

The XSM policy is not moved out of tools because that remains the
primary location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---

Changes since v4:
 - Fixed clean target in xsm/flask/Makefile
 - Dropped now-unneeded const-dropping cast of policy_buffer

 Config.mk   |  6 ++
 INSTALL | 10 --
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/common/Kconfig  | 20 
 xen/include/xsm/xsm.h   |  5 +
 xen/xsm/flask/.gitignore|  1 +
 xen/xsm/flask/Makefile  | 13 -
 xen/xsm/flask/gen-policy.py | 23 +++
 xen/xsm/xsm_core.c  |  8 
 10 files changed, 107 insertions(+), 25 deletions(-)
 create mode 100644 xen/xsm/flask/.gitignore
 create mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/Config.mk b/Config.mk
index 723e129..01316ae 100644
--- a/Config.mk
+++ b/Config.mk
@@ -147,6 +147,12 @@ export XEN_HAS_BUILD_ID=y
 build_id_linker := --build-id=sha1
 endif
 
+ifndef XEN_HAS_CHECKPOLICY
+CHECKPOLICY ?= checkpolicy
+XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && 
echo y || echo n)
+export XEN_HAS_CHECKPOLICY
+endif
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
diff --git a/INSTALL b/INSTALL
index 616a67a..9759354 100644
--- a/INSTALL
+++ b/INSTALL
@@ -269,10 +269,16 @@ Building the python tools may fail unless certain options 
are passed to
 setup.py. Config.mk contains additional info how to use this variable.
 PYTHON_PREFIX_ARG=
 
-The hypervisor may be build with XSM/Flask support, which can be changed
+The hypervisor may be built with XSM/Flask support, which can be changed
 by running:
 make -C xen menuconfig
-and enabling XSM/Flask in the 'Common Features' menu.
+and enabling XSM/Flask in the 'Common Features' menu.  A security policy
+is required to use XSM/Flask; if the SELinux policy compiler is
+available, the policy from tools can be included in the hypervisor.
+This option is enabled by default if XSM is enabled and the compiler
+(checkpolicy) is found.  The location of this executable can be set
+using the environment variable.
+CHECKPOLICY=
 
 Do a build for coverage.
 coverage=y
diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 2a088ca..5500242 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -712,13 +712,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 100644
--- a/docs/misc/

Re: [Xen-devel] [PATCH] XSM/policy: Allow the source domain access to settime and setdomainhandle domctls while creating domain.

[Daniel De Graaf]

On 07/07/2016 09:45 AM, anshul.makkar.anshul.mak...@citrix.com wrote:

From: Anshul Makkar <anshul.mak...@citrix.com>

This patch resolves the following permission denied scenarios while creating
new domU :
avc:  denied  { setdomainhandle } for domid=0 target=1
scontext=system_u:system_r:dom0_t tcontext=system_u:system_r:domU_t 
tclass=domain

avc:  denied  { settime } for domid=0 target=1 scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

Signed-off-by: Anshul Makkar <anshul.mak...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v4] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 07/07/2016 06:30 AM, Jan Beulich wrote:

On 05.07.16 at 19:44, <dgde...@tycho.nsa.gov> wrote:

--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -762,6 +762,13 @@ static inline void flask_init(void)
 }
 #endif

+#ifdef CONFIG_XSM_POLICY
+extern const unsigned char xsm_init_policy[];
+extern const int xsm_init_policy_size;


unsigned int or size_t please.


--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,17 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
 $(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+obj-$(CONFIG_XSM_POLICY) += policy.o
+
+POLICY_SRC := $(XEN_ROOT)/tools/flask/policy/xenpolicy-$(XEN_FULLVERSION)
+
+policy.bin: FORCE
+   $(MAKE) -C $(XEN_ROOT)/tools/flask/policy
+   cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
+
+policy.c: policy.bin gen-policy.py
+   $(PYTHON) gen-policy.py < $< > $@
+
 .PHONY: clean
 clean::
rm -f $(ALL_H_FILES) *.o $(DEPS)


I suppose the clean target then also needs adjustment?


Yes, it does.


+static inline void xsm_policy_init(void)
+{
+#ifdef CONFIG_XSM_POLICY
+if ( policy_size == 0 )
+{
+policy_buffer = (char*)xsm_init_policy;


Can't xsm_init_policy by of type const char[] then, avoiding the need
for a cast (you certainly shouldn't be casting away constness)? If not,
besides adding the const please also add a blank before the *.


The policy_buffer global cannot be a const char* because it is passed to
xfree() below (only in ARM); the cast would only be moved.  The buffer is
never modified, if that's what you are asking.

The reason that xsm_init_policy is unsigned is to avoid compiler warnings
resulting from assigning values such as 0xF3 to a signed character.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] default XSM policy for PCI passthrough for unlabeled resources.

[Daniel De Graaf]

On 07/06/2016 11:34 AM, anshul makkar wrote:

Hi,

Default XSM policy doesn't allow the use of unlabeled PCI resources that have 
been passed through to target domain.

xen.te
# Resources must be declared using . resource_type
neverallow * ~resource_type:resource use;

It allows the resource to be added and removed by the source domain to target 
domain, but its use by target domain is blocked.


This rule only mandates the use of resource_type for resource types.  If you 
are creating a new resource type, follow the example in nic_dev.te.


The resource can be used only if it has been labeled using flask-label-pci 
command which needs to be rerun after every boot and after every policy reload.


Yes; this gives the most control over what resources can be delegated.  Policy reloads 
are supposed to be rare (on a production system) and you already need special boot 
scripts (or parameters) to set up the device for passthrough, so this can be added there. 
 However, I agree this can be more work than a "default" FLASK policy should 
require.


The above approach reduces the flexibility and necessitates admin intervention to give 
passthrough rights after host has booted. Also, in general if I want to allow all domUs 
to have PCI passthough access to all PCI device, I have no other way apart from disabling 
the "neverallow" rule.


Try adding a module with the following rules, which should allow domU to use 
unlabeled devices:

use_device(domU_t, irq_t)
use_device(domU_t, ioport_t)
use_device(domU_t, iomem_t)
use_device(domU_t, device_t)

If this works, that module could be added to the default policy.


Given that what we ship is just a sample default policy for reference which is expected 
to be permissive in most of the scenarios so that it doesn't affect the basic 
functionalities, is this "neverallow" rule needed ?

Thanks
Anshul Makkar


The neverallow rules are just there to ensure that the attributes are being 
used correctly.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH v2 10/11] hvmctl: convert HVMOP_*ioreq_server*

[Daniel De Graaf]

On 06/24/2016 06:34 AM, Jan Beulich wrote:

Note that we can't adjust HVM_IOREQSRV_BUFIOREQ_* to properly obey
name space rules, as these constants as in use by callers of the libxc
interface.

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Paul Durrant <paul.durr...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 09/11] hvmctl: convert HVMOP_inject_msi

[Daniel De Graaf]

On 06/24/2016 06:33 AM, Jan Beulich wrote:

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>

Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 08/11] hvmctl: convert HVMOP_inject_trap

[Daniel De Graaf]

On 06/24/2016 06:32 AM, Jan Beulich wrote:

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 07/11] hvmctl: convert HVMOP_set_mem_type

[Daniel De Graaf]

On 06/24/2016 06:32 AM, Jan Beulich wrote:

This allows elimination of the (ab)use of the high operation number
bits for encoding continuations.

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 06/11] hvmctl: convert HVMOP_modified_memory

[Daniel De Graaf]

On 06/24/2016 06:31 AM, Jan Beulich wrote:

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 05/11] hvmctl: convert HVMOP_track_dirty_vram

[Daniel De Graaf]

On 06/24/2016 06:31 AM, Jan Beulich wrote:

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 04/11] hvmctl: convert HVMOP_set_pci_link_route

[Daniel De Graaf]

On 06/24/2016 06:30 AM, Jan Beulich wrote:

Note that this retains the hvmop interface definitions as those had
(wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 03/11] hvmctl: convert HVMOP_set_isa_irq_level

[Daniel De Graaf]

On 06/24/2016 06:29 AM, Jan Beulich wrote:

Note that this retains the hvmop interface definitions as those had
(wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

Re: [Xen-devel] [PATCH v2 01/11] public / x86: introduce hvmctl hypercall

[Daniel De Graaf]

On 06/24/2016 06:28 AM, Jan Beulich wrote:

... as a means to replace all HVMOP_* which a domain can't issue on
itself (i.e. intended for use by only the control domain or device
model).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


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

Re: [Xen-devel] [PATCH v2 2/5] xen/console: allow log level threshold adjustments

[Daniel De Graaf]

On 07/04/2016 11:13 AM, Wei Liu wrote:

... from serial console and via sysctl so that one doesn't always need
to reboot to see more / less messages.

Note that upper thresholds driven from the serial console are sticky,
i.e. while they get adjusted upwards when the lower threshold would
otherwise end up above the upper one, they don't get adjusted when
reducing the lower one. Full flexibility is available only via the
sysctl interface.

Signed-off-by: Jan Beulich <jbeul...@suse.com>

Rework the sysctl interface to pass in / out strings directly. Provide
some helper functions to transform from log level numbers to strings and
vice verse. Lower and upper bounds are checked. Add XSM hook.

Signed-off-by: Wei Liu <wei.l...@citrix.com>


Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

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

[Xen-devel] [PATCH v4] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

The XSM policy is not moved out of tools because that remains the
primary location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---

Changes from v3:
 - Make default Kconfig value depend on the presence of checkpolicy
 - Use proper __initconst label on generated data
 - Place generated symbols in xsm.h to enforce type matching

 Config.mk   |  6 ++
 INSTALL | 10 --
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/common/Kconfig  | 20 
 xen/include/xsm/xsm.h   |  7 +++
 xen/xsm/flask/.gitignore|  1 +
 xen/xsm/flask/Makefile  | 11 +++
 xen/xsm/flask/gen-policy.py | 23 +++
 xen/xsm/xsm_core.c  | 15 ++-
 10 files changed, 114 insertions(+), 25 deletions(-)
 create mode 100644 xen/xsm/flask/.gitignore
 create mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/Config.mk b/Config.mk
index 723e129..01316ae 100644
--- a/Config.mk
+++ b/Config.mk
@@ -147,6 +147,12 @@ export XEN_HAS_BUILD_ID=y
 build_id_linker := --build-id=sha1
 endif
 
+ifndef XEN_HAS_CHECKPOLICY
+CHECKPOLICY ?= checkpolicy
+XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && 
echo y || echo n)
+export XEN_HAS_CHECKPOLICY
+endif
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
diff --git a/INSTALL b/INSTALL
index 616a67a..9759354 100644
--- a/INSTALL
+++ b/INSTALL
@@ -269,10 +269,16 @@ Building the python tools may fail unless certain options 
are passed to
 setup.py. Config.mk contains additional info how to use this variable.
 PYTHON_PREFIX_ARG=
 
-The hypervisor may be build with XSM/Flask support, which can be changed
+The hypervisor may be built with XSM/Flask support, which can be changed
 by running:
 make -C xen menuconfig
-and enabling XSM/Flask in the 'Common Features' menu.
+and enabling XSM/Flask in the 'Common Features' menu.  A security policy
+is required to use XSM/Flask; if the SELinux policy compiler is
+available, the policy from tools can be included in the hypervisor.
+This option is enabled by default if XSM is enabled and the compiler
+(checkpolicy) is found.  The location of this executable can be set
+using the environment variable.
+CHECKPOLICY=
 
 Do a build for coverage.
 coverage=y
diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 2a088ca..5500242 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -712,13 +712,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 1006

Re: [Xen-devel] [PATCH v3] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 06/30/2016 09:45 AM, Konrad Rzeszutek Wilk wrote:

On Wed, Jun 29, 2016 at 11:09:01AM -0400, Daniel De Graaf wrote:

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

The XSM policy is not moved out of tools because that remains the
primary location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---

Changes from v2 (dropped acks and reviewed-by):
 - Drop linker script changes, use python binary-to-C file script
 - Make the config option always include the policy if selected
 - Note the new conditional dependency on checkpolicy in INSTALL


I liked the previous patch of putting in it in __init section.
Is that something this patch could do? Ah, n/m. I see that
the python script generates the binary with __init!

Secondly I was wondering why the suggestion I had - which was to check
of the 'checkpolicy' availability - and if not found - then
hide the Kconfig option was not mentioned?


At the moment, since FLASK is the only XSM implementation and it is not
enabled by default, if someone enables XSM they will need to generate a
security policy at some point: either during the hypervisor build or in
the tools build.  Otherwise, they will need to disable FLASK at runtime
or (worst case) run without a policy at all, which means all domains can
act as dom0.  If you plan to disable FLASK at runtime, it is better to
compile without XSM enabled so you don't pay the cost of the function
calls to the XSM hooks on (almost) every hypercall.

Otherwise, I was planning to just change the default from always "y" to
"y" if checkpolicy was found and "n" if not.  This would end up running
"checkpolicy -h" on every (recursive) Make invocation if placed in the
top-level Config.mk - I'm unsure if that would be a problem or not.


.. snip...

+sys.stdout.write("\n};\nconst int __init xsm_init_policy_size = %d;\n" % 
policy_size)
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 8df1a3c..93c7d43 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -36,6 +36,24 @@ static inline int verify(struct xsm_operations *ops)
 return 0;
 }

+#ifdef CONFIG_XSM_POLICY
+extern char xsm_init_policy[];



+extern int xsm_init_policy_size;
+#else
+#define xsm_init_policy 0
+#endif
+
+static void __init xsm_policy_init(void)
+{
+#ifdef CONFIG_XSM_POLICY
+if ( policy_size == 0 )
+{
+policy_buffer = xsm_init_policy;
+policy_size = xsm_init_policy_size;
+}
+#endif
+}
+


This all looks like it could go in a header file?


Sure, I could move it to xsm.h.  I thought it might be clearer to have the
assignment and the xfree() check in the same file.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] FW: vTPM detaching issue

[Daniel De Graaf]

On 06/29/2016 03:51 AM, Xu, Quan wrote:

On June 15, 2016 7:49 PM, < wei.l...@citrix.com > wrote:

On Tue, Jun 14, 2016 at 12:32:19PM +0200, Andrea Genuise wrote:
[...]

libxl: debug: libxl_aoutils.c:88:xswait_timeout_callback: backend
/local/domain/748/backend/vtpm/749/0/state (hoping for state change to

6):

xswait timeout (path=/local/domain/748/backend/vtpm/749/0/state)
libxl: debug: libxl_event.c:677:libxl__ev_xswatch_deregister: watch
w=0x1c4c1f0 wpath=/local/domain/748/backend/vtpm/749/0/state

token=3/0:

deregister slotnum=3
libxl: debug: libxl_event.c:867:devstate_callback: backend
/local/domain/748/backend/vtpm/749/0/state wanted state 6  timed out


This. The toolstack is waiting for the state to change to 6. But that never
happened.


libxl: debug: libxl_event.c:691:libxl__ev_xswatch_deregister: watch
w=0x1c4c1f0: deregister unregistered
libxl: debug: libxl_device.c:937:device_backend_callback: calling
device_backend_cleanup
libxl: debug: libxl_event.c:691:libxl__ev_xswatch_deregister: watch
w=0x1c4c1f0: deregister unregistered
libxl: debug: libxl_device.c:943:device_backend_callback: Timeout
reached, initiating forced remove


I think this is due to interaction between frontend and backend, but I'm not an
expert on vtpm so I don't have further comment.



Daniel,  are you following this issue?  --Quan


I am, but I haven't had a chance to look at what is happening with the state
changes.  There are few, if any, use cases that actually need the ability to
remove a vTPM without destroying the client domain (or the driver domain),
so I don't think this ever got tested.  I am guessing that the minios and/or
Linux driver is missing a state change step.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

The XSM policy is not moved out of tools because that remains the
primary location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---

Changes from v2 (dropped acks and reviewed-by):
 - Drop linker script changes, use python binary-to-C file script
 - Make the config option always include the policy if selected
 - Note the new conditional dependency on checkpolicy in INSTALL


 INSTALL |  6 +-
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/common/Kconfig  | 16 
 xen/xsm/flask/.gitignore|  1 +
 xen/xsm/flask/Makefile  | 11 +++
 xen/xsm/flask/gen-policy.py | 19 +++
 xen/xsm/xsm_core.c  | 22 +-
 8 files changed, 97 insertions(+), 24 deletions(-)
 create mode 100644 xen/xsm/flask/.gitignore
 create mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/INSTALL b/INSTALL
index 616a67a..703cfe7 100644
--- a/INSTALL
+++ b/INSTALL
@@ -272,7 +272,11 @@ PYTHON_PREFIX_ARG=
 The hypervisor may be build with XSM/Flask support, which can be changed
 by running:
 make -C xen menuconfig
-and enabling XSM/Flask in the 'Common Features' menu.
+and enabling XSM/Flask in the 'Common Features' menu.  By default, this
+selection will also enable compiling a built-in policy, which requires
+the SELinux policy compiler (checkpolicy) to build.  The location of
+this executable can be set using its environment variable.
+CHECKPOLICY=
 
 Do a build for coverage.
 coverage=y
diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 2a088ca..5500242 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -712,13 +712,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -141,21 +141,21 @@ only type enforcement is used and the user and role are 
set to system_u and
 system_r for all domains.
 
 The FLASK security framework is mostly configured using a security policy file.
-This policy file is not normally generated during the Xen build process because
-it relies on the SELinux compiler "checkpolicy"; run
-
-   make -C tools/flask/policy
-
-to compile the example policy included with Xen. The policy is generated from
-definition files under this directory. Most changes to security policy will
-involve creating or modifying modules found in tools/flask/policy/modules/.  
The
-modules.conf file there defines what modules are enabled and has short
-descriptions of each module.
-
-The XSM policy file needs to be copied to /boot and loaded as a module by grub.
-The exact position of the module does not matter as long as it is after the Xen
-kernel; it is normally placed eith

Re: [Xen-devel] [xen-unstable test] 96330: regressions - trouble: blocked/broken/fail/pass

[Daniel De Graaf]

On 06/29/2016 06:58 AM, Ian Jackson wrote:

Jan Beulich writes ("Re: [Xen-devel] [xen-unstable test] 96330: regressions - 
trouble: blocked/broken/fail/pass"):

On 28.06.16 at 21:16, <osstest-ad...@xenproject.org> wrote:
This latter one is

objcopy -S -I binary -O elf64-little --rename-section=.data=.init.xsm_policy 
policy.bin policy.o

...

:policy.o: Invalid bfd target
Makefile:45: recipe for target 'policy.o' failed
make[5]: Leaving directory 
'/home/osstest/build.96330.build-armhf-xsm/xen/xen/xsm/flask'
make[5]: *** [policy.o] Error 1

which looks to be the not really suitable use of elf64-little in
commit 08cffe6696 ("xsm: add a default policy to .init.data").
Since it's not immediately clear how to fix this preferably
without ugly ifdef-ery in the makefile, I think we better revert
this for now. Opinions?


Yes, I think reverting it for now is the right answer.


That's fine; I am planning on sending a v3 of this patch that drops
the use of objcopy for a python script converting the policy to an
array in a .c file.  This also eliminates the linker script changes.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 06/24/2016 01:46 PM, Konrad Rzeszutek Wilk wrote:

I can remove the HAS_CHECKPOLICY check completely and make the call to
checkpolicy only conditional on the Kconfig option.  I think this is
less complicated than stopping the compile one step above the invocation
of checkpolicy, and probably just as informative (and better, if the
detection heuristic ever breaks).


I actually like the way you have it - with the checkpolicy check determining
whether the Kconfig option for XSM is shown or not.


Is that possible?  That's not what I have; the check I have only determines
if the Kconfig option does anything or not, it is still visible regardless.


Totally!

See 95111a94f0168699d5154c7a25bd33865559e2c xsplice: Stacking build-id 
dependency checking.

Thanks.


Ah, I hadn't considered setting the variable in the top-level Config.mk.
If I were to add the HAS_CHECKPOLICY check there, I think it would make
sense to have it adjust the default value of CONFIG_XSM_POLICY, but
not hide the option.  If someone deliberately enables the option, then
having the compile error show up is less confusing than the current
method where it gets enabled when only selecting XSM.

Anyway, since checkpolicy is required to make use of FLASK, anyone who
currently enables XSM is going to need to install it at some point: either
in the hypervisor compile for the built-in policy or the tools compile for
the bootloader- or dom0-provided policy.  Having the error show up sooner
is not all that much of a problem.  This would change if XSM were to be
enabled by default, because I would then expect "xsm enabled, flask disabled"
to become a more common case - and that does not require a policy.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 06/24/2016 01:40 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 01:34:29PM -0400, Daniel De Graaf wrote:

On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:

Hello Daniel,

Please try to CC relevant maintainers on your patch. I would have missed it
if Andrew did not ping me on IRC.

On 20/06/16 15:04, Daniel De Graaf wrote:

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>


For ARM bits:

Acked-by: Julien Grall <julien.gr...@arm.com>

Although, I one a question below.

[...]


diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 12fc3a9..eefd37c 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
$(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+ifeq ($(CONFIG_XSM_POLICY),y)
+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || echo 
n)
+
+obj-$(HAS_CHECKPOLICY) += policy.o


I would have expect a warning (if not an error) here to tell the user that
checkpolicy is not available. Otherwise it may take some time to the user to
understand why the policy is not loaded/present. Because if you enable XSM,
you don't necessarily check which other options have been enabled by
default.


Good point! And we should probably update the INSTALL document too to mention
that you need checkpoint tool!


The dependency on checkpolicy is called out in the Kconfig item that
enables this option.  Are you suggesting I should add a mention below
the instructions on running menuconfig for XSM in INSTALL?


No, I just thinking that the INSTALL has a list of packages one needs.
And it may be good to have checkpolicy on it.



I can remove the HAS_CHECKPOLICY check completely and make the call to
checkpolicy only conditional on the Kconfig option.  I think this is
less complicated than stopping the compile one step above the invocation
of checkpolicy, and probably just as informative (and better, if the
detection heuristic ever breaks).


I actually like the way you have it - with the checkpolicy check determining
whether the Kconfig option for XSM is shown or not.


Is that possible?  That's not what I have; the check I have only determines
if the Kconfig option does anything or not, it is still visible regardless.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:

Hello Daniel,

Please try to CC relevant maintainers on your patch. I would have missed it
if Andrew did not ping me on IRC.

On 20/06/16 15:04, Daniel De Graaf wrote:

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>


For ARM bits:

Acked-by: Julien Grall <julien.gr...@arm.com>

Although, I one a question below.

[...]


diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 12fc3a9..eefd37c 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
 $(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+ifeq ($(CONFIG_XSM_POLICY),y)
+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || echo 
n)
+
+obj-$(HAS_CHECKPOLICY) += policy.o


I would have expect a warning (if not an error) here to tell the user that
checkpolicy is not available. Otherwise it may take some time to the user to
understand why the policy is not loaded/present. Because if you enable XSM,
you don't necessarily check which other options have been enabled by
default.


Good point! And we should probably update the INSTALL document too to mention
that you need checkpoint tool!


The dependency on checkpolicy is called out in the Kconfig item that
enables this option.  Are you suggesting I should add a mention below
the instructions on running menuconfig for XSM in INSTALL?

I can remove the HAS_CHECKPOLICY check completely and make the call to
checkpolicy only conditional on the Kconfig option.  I think this is
less complicated than stopping the compile one step above the invocation
of checkpolicy, and probably just as informative (and better, if the
detection heuristic ever breaks).




+endif


Regards,

--
Julien Grall





--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall"

[Daniel De Graaf]

On 06/23/2016 11:22 AM, Marek Marczykowski-Górecki wrote:

On Thu, Jun 23, 2016 at 11:00:42AM -0400, Daniel De Graaf wrote:

On 06/23/2016 09:25 AM, Marek Marczykowski-Górecki wrote:
[...]

Ok, after drawing a flowchart of the control in this function after your
change, on a piece of paper, this case looks fine. But depending on how
the domain was found (explicit loop or rcu_lock_domain_by_id), different
locks are held, which makes it harder to follow what is going on.

Crazy idea: how about making the code easy/easier to read instead of
obfuscating it even more? XEN_DOMCTL_getdomaininfo semantic is
convolved enough. How about this version (2 patches):

[...]

xen: allow XEN_DOMCTL_getdomaininfo for device model

Allow device model domain to get info about its target domain.
It is used during PCI passthrough setup (xc_domain_memory_mapping
checks for guest being auto-translated). While it happens in stubdomain,
it failed, breaking PCI passthrough in such setup.

While it is possible to workaround this at toolstack side, it seems
logical to allow device model to get information about its target
domain.

The problem was exposed by c428c9f "tools/libxl: handle the iomem
parameter with the memory_mapping hcall".

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 xen/include/xsm/dummy.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 406cd18..70a1633 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -128,7 +128,10 @@ static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG struct 
domain *d, int cmd)
 case XEN_DOMCTL_unbind_pt_irq:
 return xsm_default_action(XSM_DM_PRIV, current->domain, d);
 case XEN_DOMCTL_getdomaininfo:
-return xsm_default_action(XSM_XS_PRIV, current->domain, d);
+if (current->domain->target)
+return xsm_default_action(XSM_DM_PRIV, current->domain, d);
+else
+return xsm_default_action(XSM_XS_PRIV, current->domain, d);
 default:
 return xsm_default_action(XSM_PRIV, current->domain, d);
 }


I would prefer testing for the xenstore flag instead of testing for the
target field.  It ends up being the same thing in reality, since nobody
sane would make the xenstore also a device model (and not also dom0).

  case XEN_DOMCTL_getdomaininfo:
  if ( src->is_xenstore )
  return 0;
  return xsm_default_action(XSM_DM_PRIV, current->domain, d);

This makes it clear that xenstore is the special case, and removes the
need for the one-off XSM_XS_PRIV constant.


This was my initial idea, but I don't really understand the comment
about link-time verification if the behaviour is the same for xsm not
compiled vs disabled. But if skipping xsm_default_action here doesn't
break this magic, I'm for it.


That magic is just for the parameter to the XSM hook: in this case, it's
the XSM_OTHER in xsm_domctl that is being verified.  There is no magic
in xsm_default_action.


Updated patch (with removal of XSM_XS_PRIV):

xen: allow XEN_DOMCTL_getdomaininfo for device model domains

Allow device model domain to get info about its target domain.
It is used during PCI passthrough setup (xc_domain_memory_mapping
checks for guest being auto-translated). While it happens in stubdomain,
it failed, breaking PCI passthrough in such setup.

While it is possible to workaround this at toolstack side, it seems
logical to allow device model to get information about its target
domain.

Also, since this was the only usage of XSM_XS_PRIV, which now gets
handled inline, drop it.

The problem was exposed by c428c9f "tools/libxl: handle the iomem
parameter with the memory_mapping hcall".

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>


This would be a good patch to include the corresponding change to the
XSM policy (tweaked from Jan's email):


--- a/tools/flask/policy/modules/xen/xen.if
+++ b/tools/flask/policy/modules/xen/xen.if
@@ -148,7 +148,7 @@ define(`device_model', `
create_channel($2, $1, $2_channel)
allow $1 $2_channel:event create;

-   allow $1 $2_target:domain shutdown;
+   allow $1 $2_target:domain { getdomaininfo shutdown };
allow $1 $2_target:mmu { map_read map_write adjust physmap target_hack 
};
allow $1 $2_target:hvm { getparam setparam trackdirtyvram hvmctl 
irqlevel pciroute pcilevel cacheattr send_irq };
 ')


With that included, or elsewhere in the series:

Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>


---
 xen/include/xsm/dummy.h | 8 +++-
 xen/include/xsm/xsm.h   | 1 -
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 406cd18..2768861 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -71,10 +71,6 @@ static always_inline int xsm_default_action(

Re: [Xen-devel] PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall"

[Daniel De Graaf]

On 06/23/2016 09:25 AM, Marek Marczykowski-Górecki wrote:
[...]

Ok, after drawing a flowchart of the control in this function after your
change, on a piece of paper, this case looks fine. But depending on how
the domain was found (explicit loop or rcu_lock_domain_by_id), different
locks are held, which makes it harder to follow what is going on.

Crazy idea: how about making the code easy/easier to read instead of
obfuscating it even more? XEN_DOMCTL_getdomaininfo semantic is
convolved enough. How about this version (2 patches):

[...]

xen: allow XEN_DOMCTL_getdomaininfo for device model

Allow device model domain to get info about its target domain.
It is used during PCI passthrough setup (xc_domain_memory_mapping
checks for guest being auto-translated). While it happens in stubdomain,
it failed, breaking PCI passthrough in such setup.

While it is possible to workaround this at toolstack side, it seems
logical to allow device model to get information about its target
domain.

The problem was exposed by c428c9f "tools/libxl: handle the iomem
parameter with the memory_mapping hcall".

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 xen/include/xsm/dummy.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 406cd18..70a1633 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -128,7 +128,10 @@ static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG struct 
domain *d, int cmd)
 case XEN_DOMCTL_unbind_pt_irq:
 return xsm_default_action(XSM_DM_PRIV, current->domain, d);
 case XEN_DOMCTL_getdomaininfo:
-return xsm_default_action(XSM_XS_PRIV, current->domain, d);
+if (current->domain->target)
+return xsm_default_action(XSM_DM_PRIV, current->domain, d);
+else
+return xsm_default_action(XSM_XS_PRIV, current->domain, d);
 default:
 return xsm_default_action(XSM_PRIV, current->domain, d);
 }


I would prefer testing for the xenstore flag instead of testing for the
target field.  It ends up being the same thing in reality, since nobody
sane would make the xenstore also a device model (and not also dom0).

  case XEN_DOMCTL_getdomaininfo:
  if ( src->is_xenstore )
  return 0;
  return xsm_default_action(XSM_DM_PRIV, current->domain, d);

This makes it clear that xenstore is the special case, and removes the
need for the one-off XSM_XS_PRIV constant.  


--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall"

[Daniel De Graaf]

On 06/23/2016 04:39 AM, Jan Beulich wrote:

On 23.06.16 at 10:32, <jbeul...@suse.com> wrote:

On 22.06.16 at 20:24, <dgde...@tycho.nsa.gov> wrote:

Either method works, and I agree allowing DM to invoke this domctl is both
useful and not going to introduce problems.  The getdomaininfo permission
will also need to be added to the device_model macro in xen.if.


What exactly this last sentence means I need to add I'm not sure
about.


Perhaps this?

--- unstable.orig/tools/flask/policy/policy/modules/xen/xen.if
+++ unstable/tools/flask/policy/policy/modules/xen/xen.if
@@ -148,7 +148,7 @@ define(`device_model', `
create_channel($2, $1, $2_channel)
allow $1 $2_channel:event create;

-   allow $1 $2_target:domain shutdown;
+   allow $1 $2_target:domain { getdomaininfo shutdown };
allow $1 $2_target:mmu { map_read map_write adjust physmap target_hack 
};
allow $1 $2_target:hvm { getparam setparam trackdirtyvram hvmctl 
irqlevel pciroute pcilevel cacheattr send_irq };
 ')

Jan


Yes, that is what I meant.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall"

[Daniel De Graaf]

On 06/22/2016 11:23 AM, Jan Beulich wrote:

On 22.06.16 at 16:13, <marma...@invisiblethingslab.com> wrote:

On Wed, Jun 22, 2016 at 07:50:09AM -0600, Jan Beulich wrote:

On 22.06.16 at 15:03, <marma...@invisiblethingslab.com> wrote:

I've finally found what was causing long standing issue of not working
PCI passthrough for HVM domains with qemu in stubdomain (only - without
the other one in dom0). It looks to be this patch:


http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=c428c9f162895cb3473f

ab26d23ffbf41a6f293d;hp=dcccaf806a92eabb95929a67c344ac1e9ead6257

It calls xc_domain_getinfo from xc_domain_memory_mapping (to check if
the target domain is auto-translated), but xc_domain_getinfo fails with
EPERM in stubdomain.

What would be the best solution for this? Allowing xc_domain_getinfo
from stubdomain in xen/include/xsm/dummy.h? Currently it is uses policy
XSM_XS_PRIV in unstable and just XSM_PRIV in 4.6 - so, maybe have some
combination of XSM_XS_PRIV and XSM_DM_PRIV? Or maybe fix this by
removing xc_domain_getinfo call in xc_domain_memory_mapping, possibly
implementing the logic from that commit solely in libxl?


Once we fixed the quirky behavior of the current implementation
(allowing information to be returned for other than the requested
domain), I see no reason why this couldn't become XSM_DM_PRIV.


Can you explain this more? Is this fix backported to 4.6 and/or 4.4?


Which fix? I talked of one to be made.


But let's ask Daniel explicitly. And in that context I then also wonder
whether the xsm_getdomaininfo() invocation shouldn't be limited to
the respective sysctl.


Actually getdomaininfo is handled in two places in xsm/dummy.h:
 - xsm_getdomaininfo (which does nothing when XSM is disabled)
 - xsm_domctl (which enforce actual policy)

Also reading commit message of XSM_XS_PRIV introduction, it may be
useful to be able to just check if given domain is alive, without
getting all the information returned by XEN_DOMCTL_getdomaininfo. I find
this useful also for any other inter-domain communication (for example
libxenvchan connection).

But for now, XEN_DOMCTL_getdomaininfo should be allowed either when
device-model domain is asking about its target domain, or calling domain
is xenstore domain/privileged domain. Right?


Yes, that's what I think too.


 How to combine those
types? Change XSM_XS_PRIV to XSM_XS_DM_PRIV (it looks like the only
usage of XSM_XS_PRIV)?


Daniel?

Jan


Changing the definition of XSM_XS_PRIV seems like the best solution, since
this is the only use.  I don't think it matters if the constant is renamed
to XSM_XS_DM_PRIV or not.  In fact, since the constant isn't ever used by a
caller, it could be removed and the actual logic placed in the switch
statement - that way it's clear this is a special case for getdomaininfo
instead of attempting to make this generic.

Either method works, and I agree allowing DM to invoke this domctl is both
useful and not going to introduce problems.  The getdomaininfo permission
will also need to be added to the device_model macro in xen.if.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 13/17 v3] xen: move FLASK entry under XSM in Kconfig

[Daniel De Graaf]

Since enabling XSM is required to enable FLASK, place the option for
FLASK below the one for XSM.  In addition, since it does not make sense
to enable XSM without any XSM providers, and FLASK is the only XSM
provider, hide the option to disable FLASK under EXPERT.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---

Changes from v2: use def_bool/prompt instead of def_bool/bool so that it
  matches the other "if EXPERT" items in this file

 xen/common/Kconfig | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index cd59574..faee3ec 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -11,24 +11,6 @@ config COMPAT
 config CORE_PARKING
bool
 
-config FLASK
-   bool "FLux Advanced Security Kernel support"
-   default y
-   depends on XSM
-   ---help---
- Enables the FLASK (FLux Advanced Security Kernel) support which
- provides a mandatory access control framework by which security
- enforcement, isolation, and auditing can be achieved with fine
- granular control via a security policy.
-
- If unsure, say N.
-
-config FLASK_AVC_STATS
-   def_bool y
-   depends on FLASK
-   ---help---
- Maintain statistics on the access vector cache
-
 # Select HAS_DEVICE_TREE if device tree is supported
 config HAS_DEVICE_TREE
bool
@@ -137,6 +119,25 @@ config XSM
 
  If unsure, say N.
 
+config FLASK
+   def_bool y
+   prompt "FLux Advanced Security Kernel support" if EXPERT = "y"
+   depends on XSM
+   ---help---
+ Enables FLASK (FLux Advanced Security Kernel) as the access control
+ mechanism used by the XSM framework.  This provides a mandatory access
+ control framework by which security enforcement, isolation, and
+ auditing can be achieved with fine granular control via a security
+ policy.
+
+ If unsure, say Y.
+
+config FLASK_AVC_STATS
+   def_bool y
+   depends on FLASK
+   ---help---
+ Maintain statistics on the access vector cache
+
 # Enable schedulers
 menu "Schedulers"
visible if EXPERT = "y"
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17] xen: move FLASK entry under XSM in Kconfig

[Daniel De Graaf]

On 06/20/2016 10:46 AM, Doug Goldstein wrote:

On 6/20/16 9:04 AM, Daniel De Graaf wrote:

Since enabling XSM is required to enable FLASK, place the option for
FLASK below the one for XSM.  In addition, since it does not make sense
to enable XSM without any XSM providers, and FLASK is the only XSM
provider, hide the option to disable FLASK under EXPERT.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---



@@ -137,6 +119,25 @@ config XSM

  If unsure, say N.

+config FLASK
+   def_bool y
+   bool "FLux Advanced Security Kernel support" if EXPERT = "y"


Ok. Here's the real review. I think you want the prompt to be optional
if EXPERT is enabled then I think you need to use "prompt" instead of
"bool". You've already got this set to a bool with the "def_bool" line.


OK.  This version also apparently works, since I tested it, but if
prompt is the preferred keyword I'll change it.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 10/17] flask: remove xen_flask_userlist operation

[Daniel De Graaf]

On 06/20/2016 10:35 AM, Doug Goldstein wrote:

On 6/20/16 9:04 AM, Daniel De Graaf wrote:

This operation has no known users, and is primarily useful when an MLS
policy is in use (which has never been shipped with Xen).  In addition,
the information it provides does not actually depend on hypervisor
state (only on the XSM policy), so an application that needs it could
compute the results without needing to involve the hypervisor.



So if I read this language correctly. Removing this does not affect
someone being able to build a MLS policy at a later date right?


Correct; that support is still there.  This hypercall was used to
compute a list of reachable security contexts for a given user, which
is trivial in a non-MLS policy but more complex when one is being
used.  This computation makes more sense on Linux (where creating
new contexts via "exec" is common) than on Xen (where normally a
domain cannot create another).

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Daniel De Graaf]

On 06/20/2016 10:35 AM, Andrew Cooper wrote:

On 20/06/16 15:27, Doug Goldstein wrote:

On 6/20/16 9:04 AM, Daniel De Graaf wrote:

These permissions were initially split because they were in separate
domctls, but this split is very unlikely to actually provide security
benefits: it would require a carefully contrived situation for a domain
to both need access to one type of CPU register and also need to be
prohibited from accessing another type.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>

I'm a:

Reviewed-by: Doug Goldstein <car...@cardoe.com>

But I'd like to see Andrew Cooper's R-b or comments as well.



I agree.  I can't see a plausible usecase for an entity being entitled
to read vcpu content, but not to modify it.

Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>


That's not exactly what this patch does: the get and set permissions
are still split, but unified across the different types of registers.
Where previously there were 6 permissions, now there are 2.

A use case where you would be entitled to read but not modify is a
monitoring domain (remote virus scanner, for example) which needs
read access to scan but does not do remediation itself.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 02/11] hvmctl: convert HVMOP_set_pci_intx_level

[Daniel De Graaf]

On 06/20/2016 08:53 AM, Jan Beulich wrote:

Note that this adds validation of the "domain" interface structure
field, which previously got ignored.

Note further that this retains the hvmop interface definitions as those
had (wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
TBD: xen/xsm/flask/policy/access_vectors says "also needs hvmctl", but
 I don't see how this has been done so far. With the change here,
 doing two checks in flask_hvm_control() (the generic one always
 and a specific one if needed) would of course be simple, but it's
 unclear how subsequently added sub-ops should then be dealt with
 (which don't have a similar remark).


I am not sure why that remark is there: it seems like it refers to an
overall check in the HVM operation hypercall, which does not exist.

There is no reason to have an operation protected by two different
access checks, so I think that both the previous and patched code
are correct and the "also needs hvmctl" comment should be removed.
With that, Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 05/17] flask/policy: xenstore stubdom policy

[Daniel De Graaf]

This adds the xenstore_t type to the example policy for use by a
xenstore stub domain; see the init-xenstore-domain tool for how this
type needs to be used.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
 tools/flask/policy/modules/modules.conf |  3 +++
 tools/flask/policy/modules/xenstore.te  | 24 
 2 files changed, 27 insertions(+)
 create mode 100644 tools/flask/policy/modules/xenstore.te

diff --git a/tools/flask/policy/modules/modules.conf 
b/tools/flask/policy/modules/modules.conf
index 9aac6a0..6dba0a3 100644
--- a/tools/flask/policy/modules/modules.conf
+++ b/tools/flask/policy/modules/modules.conf
@@ -33,6 +33,9 @@ nomigrate = on
 # Example device policy.  Also see policy/device_contexts.
 nic_dev = on
 
+# Xenstore stub domain (see init-xenstore-domain).
+xenstore = on
+
 # This allows any domain type to be created using the system_r role.  When it 
is
 # disabled, domains not using the default types (dom0_t, domU_t, dm_dom_t) must
 # use another role (such as vm_r from the vm_role module below).
diff --git a/tools/flask/policy/modules/xenstore.te 
b/tools/flask/policy/modules/xenstore.te
new file mode 100644
index 000..519566a
--- /dev/null
+++ b/tools/flask/policy/modules/xenstore.te
@@ -0,0 +1,24 @@
+
+#
+# Xenstore stubdomain
+#
+
+declare_singleton_domain(xenstore_t)
+create_domain(dom0_t, xenstore_t)
+manage_domain(dom0_t, xenstore_t)
+
+# Xenstore requires the global VIRQ for domain destroy operations
+allow dom0_t xenstore_t:domain set_virq_handler;
+# Current xenstore stubdom uses the hypervisor console, not "xl console"
+allow xenstore_t xen_t:xen writeconsole;
+# Xenstore queries domaininfo on all domains
+allow xenstore_t domain_type:domain getdomaininfo;
+
+# As a shortcut, the following 3 rules are used instead of adding a 
domain_comms
+# rule between xenstore_t and every domain type that talks to xenstore
+create_channel(xenstore_t, domain_type, xenstore_t_channel)
+allow event_type xenstore_t: event bind;
+allow xenstore_t domain_type:grant { map_read map_write unmap };
+
+# Xenstore is a utility domain, so it should use the system role
+role system_r types xenstore_t;
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 02/17] flask/policy: split out rules for system_r

[Daniel De Graaf]

When the all_system_role module is enabled, any domain type can be
created using the system_r role, which was the default.  When it is
disabled, domains not using the default types (dom0_t and domU_t) must
use another role such as vm_r.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
 tools/flask/policy/modules/all_system_role.te |  8 
 tools/flask/policy/modules/domU.te|  3 +++
 tools/flask/policy/modules/modules.conf   |  5 +
 tools/flask/policy/modules/xen.te | 11 +++
 4 files changed, 19 insertions(+), 8 deletions(-)
 create mode 100644 tools/flask/policy/modules/all_system_role.te

diff --git a/tools/flask/policy/modules/all_system_role.te 
b/tools/flask/policy/modules/all_system_role.te
new file mode 100644
index 000..74f870f
--- /dev/null
+++ b/tools/flask/policy/modules/all_system_role.te
@@ -0,0 +1,8 @@
+# Allow all domains to use system_r so that systems that are not using the
+# user/role separation feature will work properly.
+role system_r types domain_type;
+
+# The vm role is used as part of user separation.  Allow all domain types to 
use
+# this role except dom0.
+role vm_r;
+role vm_r types { domain_type -dom0_t };
diff --git a/tools/flask/policy/modules/domU.te 
b/tools/flask/policy/modules/domU.te
index ca5eecd..b77df29 100644
--- a/tools/flask/policy/modules/domU.te
+++ b/tools/flask/policy/modules/domU.te
@@ -23,3 +23,6 @@ make_device_model(dom0_t, dm_dom_t, domU_t)
 
 # This is required for PCI (or other device) passthrough
 delegate_devices(dom0_t, domU_t)
+
+# Both of these domain types can be created using the default (system) role
+role system_r types { domU_t dm_dom_t };
diff --git a/tools/flask/policy/modules/modules.conf 
b/tools/flask/policy/modules/modules.conf
index dba4b40..d875dbf 100644
--- a/tools/flask/policy/modules/modules.conf
+++ b/tools/flask/policy/modules/modules.conf
@@ -32,3 +32,8 @@ nomigrate = on
 
 # Example device policy.  Also see policy/device_contexts.
 nic_dev = on
+
+# This allows any domain type to be created using the system_r role.  When it 
is
+# disabled, domains not using the default types (dom0_t and domU_t) must use
+# another role (such as vm_r) from the vm_role module.
+all_system_role = on
diff --git a/tools/flask/policy/modules/xen.te 
b/tools/flask/policy/modules/xen.te
index 3ee5e75..f374dc5 100644
--- a/tools/flask/policy/modules/xen.te
+++ b/tools/flask/policy/modules/xen.te
@@ -78,12 +78,7 @@ neverallow * ~event_type:event { create send status };
 # The object role (object_r) is used for devices, resources, and event 
channels;
 # it does not need to be defined here and should not be used for domains.
 
-# The system role is used for utility domains and pseudo-domains
+# The system role is used for utility domains and pseudo-domains.  If roles are
+# not being used for separation, all domains can use the system role.
 role system_r;
-role system_r types { xen_type domain_type };
-# If you want to prevent domUs from being placed in system_r:
-##role system_r types { xen_type dom0_t };
-
-# The vm role is used for customer virtual machines
-role vm_r;
-role vm_r types { domain_type -dom0_t };
+role system_r types { xen_type dom0_t };
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 07/17] flask: unify {get, set}vcpucontext permissions

[Daniel De Graaf]

These permissions were initially split because they were in separate
domctls, but this split is very unlikely to actually provide security
benefits: it would require a carefully contrived situation for a domain
to both need access to one type of CPU register and also need to be
prohibited from accessing another type.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---
 tools/flask/policy/modules/dom0.te  |  1 -
 tools/flask/policy/modules/xen.if   |  7 +++
 xen/xsm/flask/hooks.c   | 20 ++--
 xen/xsm/flask/policy/access_vectors | 16 ++--
 4 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index ef6a986..d228b24 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -34,7 +34,6 @@ allow dom0_t dom0_t:domain {
setvcpucontext max_vcpus setaffinity getaffinity getscheduler
getdomaininfo getvcpuinfo getvcpucontext setdomainmaxmem setdomainhandle
setdebugging hypercall settime setaddrsize getaddrsize trigger
-   getextvcpucontext setextvcpucontext getvcpuextstate setvcpuextstate
getpodtarget setpodtarget set_misc_info set_virq_handler
 };
 allow dom0_t dom0_t:domain2 {
diff --git a/tools/flask/policy/modules/xen.if 
b/tools/flask/policy/modules/xen.if
index 00d1bbb..fd96303 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -47,9 +47,8 @@ define(`declare_build_label', `
 
 define(`create_domain_common', `
allow $1 $2:domain { create max_vcpus setdomainmaxmem setaddrsize
-   getdomaininfo hypercall setvcpucontext setextvcpucontext
-   getscheduler getvcpuinfo getvcpuextstate getaddrsize
-   getaffinity setaffinity setvcpuextstate };
+   getdomaininfo hypercall setvcpucontext getscheduler
+   getvcpuinfo getaddrsize getaffinity setaffinity };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
psr_cmt_op psr_cat_op soft_reset };
@@ -94,7 +93,7 @@ define(`migrate_domain_out', `
allow $1 domxen_t:mmu map_read;
allow $1 $2:hvm { gethvmc getparam irqlevel };
allow $1 $2:mmu { stat pageinfo map_read };
-   allow $1 $2:domain { getaddrsize getvcpucontext getextvcpucontext 
getvcpuextstate pause destroy };
+   allow $1 $2:domain { getaddrsize getvcpucontext pause destroy };
allow $1 $2:domain2 gettsc;
allow $1 $2:shadow { enable disable logdirty };
 ')
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 20d46c8..a8d45e7 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -630,10 +630,16 @@ static int flask_domctl(struct domain *d, int cmd)
 case XEN_DOMCTL_setdomainhandle:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETDOMAINHANDLE);
 
+case XEN_DOMCTL_set_ext_vcpucontext:
+case XEN_DOMCTL_set_vcpu_msrs:
 case XEN_DOMCTL_setvcpucontext:
+case XEN_DOMCTL_setvcpuextstate:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETVCPUCONTEXT);
 
+case XEN_DOMCTL_get_ext_vcpucontext:
+case XEN_DOMCTL_get_vcpu_msrs:
 case XEN_DOMCTL_getvcpucontext:
+case XEN_DOMCTL_getvcpuextstate:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETVCPUCONTEXT);
 
 case XEN_DOMCTL_getvcpuinfo:
@@ -675,20 +681,6 @@ static int flask_domctl(struct domain *d, int cmd)
 case XEN_DOMCTL_pin_mem_cacheattr:
 return current_has_perm(d, SECCLASS_HVM, HVM__CACHEATTR);
 
-case XEN_DOMCTL_set_ext_vcpucontext:
-case XEN_DOMCTL_set_vcpu_msrs:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETEXTVCPUCONTEXT);
-
-case XEN_DOMCTL_get_ext_vcpucontext:
-case XEN_DOMCTL_get_vcpu_msrs:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETEXTVCPUCONTEXT);
-
-case XEN_DOMCTL_setvcpuextstate:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETVCPUEXTSTATE);
-
-case XEN_DOMCTL_getvcpuextstate:
-return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETVCPUEXTSTATE);
-
 case XEN_DOMCTL_sendtrigger:
 return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__TRIGGER);
 
diff --git a/xen/xsm/flask/policy/access_vectors 
b/xen/xsm/flask/policy/access_vectors
index 3d29042..7e69ede 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -111,6 +111,9 @@ class xen2
 class domain
 {
 # XEN_DOMCTL_setvcpucontext
+# XEN_DOMCTL_setvcpuextstate
+# XEN_DOMCTL_set_ext_vcpucontext
+# XEN_DOMCTL_set_vcpu_msrs
 setvcpucontext
 # XEN_DOMCTL_pausedomain
 pause
@@ -142,6 +145,9 @@ class domain
 # XEN_DOMCTL_getvcpuinfo
 getvcpuinfo
 # XEN_DOMCTL_ge

[Xen-devel] [PATCH 12/17] xen/xsm: remove .xsm_initcall.init section

[Daniel De Graaf]

Since FLASK is the only implementation of XSM hooks in Xen, using an
iterated initcall dispatch for setup is overly complex.  Change this to
a direct function call to a globally visible function; if additional XSM
hooks are added in the future, a switching mechanism will be needed
regardless, and that can be placed in xsm_core.c.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---
 xen/arch/arm/xen.lds.S |  5 -
 xen/arch/x86/xen.lds.S |  5 -
 xen/include/xsm/xsm.h  | 16 
 xen/xsm/flask/hooks.c  |  4 +---
 xen/xsm/xsm_core.c | 13 +
 5 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 76982b2..8320381 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -162,11 +162,6 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
   } :text
-  .xsm_initcall.init : {
-   __xsm_initcall_start = .;
-   *(.xsm_initcall.init)
-   __xsm_initcall_end = .;
-  } :text
   __init_end_efi = .;
   . = ALIGN(STACK_SIZE);
   __init_end = .;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index a43b29d..dcbb8fe 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -190,11 +190,6 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
   } :text
-  .xsm_initcall.init : {
-   __xsm_initcall_start = .;
-   *(.xsm_initcall.init)
-   __xsm_initcall_end = .;
-  } :text
   . = ALIGN(PAGE_SIZE);
   __init_end = .;
 
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 8ed8ee5..0d525ec 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -46,14 +46,6 @@ typedef enum xsm_default xsm_default_t;
 extern char *policy_buffer;
 extern u32 policy_size;
 
-typedef void (*xsm_initcall_t)(void);
-
-extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
-
-#define xsm_initcall(fn) \
-static xsm_initcall_t __initcall_##fn \
-__used_section(".xsm_initcall.init") = fn
-
 struct xsm_operations {
 void (*security_domaininfo) (struct domain *d,
 struct xen_domctl_getdomaininfo *info);
@@ -763,6 +755,14 @@ extern int unregister_xsm(struct xsm_operations *ops);
 extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
 
+#ifdef CONFIG_FLASK
+extern void flask_init(void);
+#else
+static inline void flask_init(void)
+{
+}
+#endif
+
 #else /* CONFIG_XSM */
 
 #include 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 543406b..d632b0e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1817,7 +1817,7 @@ static struct xsm_operations flask_ops = {
 .xen_version = flask_xen_version,
 };
 
-static __init void flask_init(void)
+__init void flask_init(void)
 {
 int ret = -ENOENT;
 
@@ -1860,8 +1860,6 @@ static __init void flask_init(void)
 printk(XENLOG_INFO "Flask:  Starting in permissive mode.\n");
 }
 
-xsm_initcall(flask_init);
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 634ec98..3487742 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -36,17 +36,6 @@ static inline int verify(struct xsm_operations *ops)
 return 0;
 }
 
-static void __init do_xsm_initcalls(void)
-{
-xsm_initcall_t *call;
-call = __xsm_initcall_start;
-while ( call < __xsm_initcall_end )
-{
-(*call) ();
-call++;
-}
-}
-
 static int __init xsm_core_init(void)
 {
 if ( verify(_xsm_ops) )
@@ -57,7 +46,7 @@ static int __init xsm_core_init(void)
 }
 
 xsm_ops = _xsm_ops;
-do_xsm_initcalls();
+flask_init();
 
 return 0;
 }
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 06/17] flask/policy: remove unused example

[Daniel De Graaf]

The access vectors defined here have never been used by xenstore.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
 tools/flask/policy/policy/access_vectors   | 23 ++-
 tools/flask/policy/policy/security_classes |  1 -
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/tools/flask/policy/policy/access_vectors 
b/tools/flask/policy/policy/access_vectors
index 4fd61f1..d9c69c0 100644
--- a/tools/flask/policy/policy/access_vectors
+++ b/tools/flask/policy/policy/access_vectors
@@ -1,24 +1,5 @@
 # Locally defined access vectors
 #
-# Define access vectors for the security classes defined in security_classes
+# Define access vectors for the security classes defined in security_classes.
+# Access vectors defined in this file should not be used by the hypervisor.
 #
-
-# Note: this is an example; the xenstore daemon provided with Xen does
-# not yet include XSM support, and the exact permissions may be defined
-# differently if such support is added.
-class xenstore {
-   # read from keys owned by the target domain (if permissions allow)
-   read
-   # write to keys owned by the target domain (if permissions allow)
-   write
-   # change permissions of a key owned by the target domain
-   chmod
-   # change the owner of a key which was owned by the target domain
-   chown_from
-   # change the owner of a key to the target domain
-   chown_to
-   # access a key owned by the target domain without permission
-   override
-   # introduce a domain
-   introduce
-}
diff --git a/tools/flask/policy/policy/security_classes 
b/tools/flask/policy/policy/security_classes
index 56595e8..0f0f9f3 100644
--- a/tools/flask/policy/policy/security_classes
+++ b/tools/flask/policy/policy/security_classes
@@ -5,4 +5,3 @@
 # security policy.
 #
 # Access vectors for these classes must be defined in the access_vectors file.
-class xenstore
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 15/17] xsm: clean up unregistration

[Daniel De Graaf]

The only possible value of original_ops was _xsm_ops, and
unregister_xsm was never used.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---
 xen/include/xsm/xsm.h|  1 -
 xen/xsm/flask/flask_op.c |  4 +---
 xen/xsm/flask/hooks.c|  3 ---
 xen/xsm/xsm_core.c   | 16 
 4 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 0d525ec..4b8843d 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -750,7 +750,6 @@ extern bool has_xsm_magic(paddr_t);
 #endif
 
 extern int register_xsm(struct xsm_operations *ops);
-extern int unregister_xsm(struct xsm_operations *ops);
 
 extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 3ad4bdc..719c2d7 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -58,8 +58,6 @@ static int flask_security_make_bools(void);
 
 extern int ss_initialized;
 
-extern struct xsm_operations *original_ops;
-
 static void __init parse_flask_param(char *s)
 {
 if ( !strcmp(s, "enforcing") )
@@ -243,7 +241,7 @@ static int flask_disable(void)
 flask_disabled = 1;
 
 /* Reset xsm_ops to the original module. */
-xsm_ops = original_ops;
+xsm_ops = _xsm_ops;
 
 return 0;
 }
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index d632b0e..2692a6f 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -35,8 +35,6 @@
 #include 
 #include 
 
-struct xsm_operations *original_ops = NULL;
-
 static u32 domain_sid(struct domain *dom)
 {
 struct domain_security_struct *dsec = dom->ssid;
@@ -1842,7 +1840,6 @@ __init void flask_init(void)
 
 avc_init();
 
-original_ops = xsm_ops;
 if ( register_xsm(_ops) )
 panic("Flask: Unable to register with XSM");
 
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 78d881b..8df1a3c 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -144,22 +144,6 @@ int __init register_xsm(struct xsm_operations *ops)
 return 0;
 }
 
-
-int unregister_xsm(struct xsm_operations *ops)
-{
-if ( ops != xsm_ops )
-{
-printk("%s: trying to unregister "
-   "a security_opts structure that is not "
-   "registered, failing.\n", __FUNCTION__);
-return -EINVAL;
-}
-
-xsm_ops = _xsm_ops;
-
-return 0;
-}
-
 #endif
 
 long do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 04/17] flask/policy: remove unused support for binary modules

[Daniel De Graaf]

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
 .../policy/policy/support/loadable_module.spt  | 166 -
 tools/flask/policy/policy/support/misc_macros.spt  |   2 +
 2 files changed, 2 insertions(+), 166 deletions(-)
 delete mode 100644 tools/flask/policy/policy/support/loadable_module.spt

diff --git a/tools/flask/policy/policy/support/loadable_module.spt 
b/tools/flask/policy/policy/support/loadable_module.spt
deleted file mode 100644
index de48b3b..000
--- a/tools/flask/policy/policy/support/loadable_module.spt
+++ /dev/null
@@ -1,166 +0,0 @@
-
-#
-# Macros for switching between source policy
-# and loadable policy module support
-#
-
-##
-#
-# For adding the module statement
-#
-define(`policy_module',`
-   ifdef(`self_contained_policy',`',`
-   module $1 $2;
-
-   require {
-   role system_r;
-   all_kernel_class_perms
-   }
-   ')
-')
-
-##
-#
-# For use in interfaces, to optionally insert a require block
-#
-define(`gen_require',`
-   ifdef(`self_contained_policy',`',`
-   define(`in_gen_require_block')
-   require {
-   $1
-   }
-   undefine(`in_gen_require_block')
-   ')
-')
-
-##
-#
-# In the future interfaces should be in loadable modules
-#
-# template(name,rules)
-#
-define(`template',`
-   `define(`$1',`
-# begin $1(dollarsstar)
-   $2
-# end $1(dollarsstar)
-   '')
-')
-
-# helper function, since m4 wont expand macros
-# if a line is a comment (#):
-define(`policy_m4_comment',`dnl
-# $2 depth: $1
-')dnl
-
-##
-#
-# In the future interfaces should be in loadable modules
-#
-# interface(name,rules)
-#
-define(`interface',`
-   `define(`$1',`
-
-   define(`policy_temp',incr(policy_call_depth))
-   pushdef(`policy_call_depth',policy_temp)
-   undefine(`policy_temp')
-
-   policy_m4_comment(policy_call_depth,begin `$1'(dollarsstar))
-
-   $2
-
-   define(`policy_temp',decr(policy_call_depth))
-   pushdef(`policy_call_depth',policy_temp)
-   undefine(`policy_temp')
-
-   policy_m4_comment(policy_call_depth,end `$1'(dollarsstar))
-
-   '')
-')
-
-define(`policy_call_depth',0)
-
-##
-#
-# Optional policy handling
-#
-define(`optional_policy',`
-   ifdef(`self_contained_policy',`
-   ifdef(`$1',`$2',`$3')
-   ',`
-   optional {
-   $2
-   ifelse(`$3',`',`',`
-   } else {
-   $3
-   ')
-   }
-   ')
-')
-
-##
-#
-# Determine if we should use the default
-# tunable value as specified by the policy
-# or if the override value should be used
-#
-define(`dflt_or_overr',`ifdef(`$1',$1,$2)')
-
-##
-#
-# Extract booleans out of an expression.
-# This needs to be reworked so expressions
-# with parentheses can work.
-
-define(`delcare_required_symbols',`
-ifelse(regexp($1, `\w'), -1, `', `dnl
-bool regexp($1, `\(\w+\)', `\1');
-delcare_required_symbols(regexp($1, `\w+\(.*\)', `\1'))dnl
-') dnl
-')
-
-##
-#
-# Tunable declaration
-#
-define(`gen_tunable',`
-   ifdef(`self_contained_policy',`
-   bool $1 dflt_or_overr(`$1'_conf,$2);
-   ',`
-   # loadable module tunable
-   # declaration will go here
-   # instead of bool when
-   # loadable modules support
-   # tunables
-   bool $1 dflt_or_overr(`$1'_conf,$2);
-   ')
-')
-
-##
-#
-# Tunable policy handling
-#
-define(`tunable_policy',`
-   ifdef(`self_contained_policy',`
-   if (`$1') {
-   $2
-   } else {
-   $3
-   }
-   ',`
-   # structure for tunables
-   # will go here instead of a
-   # conditional when loadable
-   # modules support tunables
-   gen_require(`
-   delcare_required_symbols(`$1')
-   ')
-
-   if (`$1') {
-   $2
-   } else {
-   $3
-   }
-   ')
-')
diff --git a/tools/flask/policy/policy/support/misc_macros.spt 
b/tools/flask/policy/policy/support/misc_macros.spt
index 344f5c4..3116db9 100644
--- a/tools/flask/policy/policy/support/misc_macros.spt
+++ b/tools/flask/policy/policy/support/misc_macros.spt
@@ -61,6 +61,8 @@ define(`gen_all_users',`')
 #
 define(`gen_context',`$1`'ifdef

[Xen-devel] [PATCH v2 00/17] XSM/FLASK updates for 4.8

[Daniel De Graaf]

Changes from v1:
 - Change c->context and c->sid from arrays to fields when shrinking
 - Keep struct xen_flask_userlist in headers, but guard it with #ifs
 - Split off Kconfig changes into their own patches
 - Add patch 16 (AVC_STATS in Kconfig)
 - Prevent free() of static data in xsm_dt_init

FLASK policy updates:
 [PATCH 01/17] flask/policy: split into modules
 [PATCH 02/17] flask/policy: split out rules for system_r
 [PATCH 03/17] flask/policy: move user definitions and constraints
 [PATCH 04/17] flask/policy: remove unused support for binary modules
 [PATCH 05/17] flask/policy: xenstore stubdom policy
 [PATCH 06/17] flask/policy: remove unused example

Hypervisor updates to the FLASK security server:
 [PATCH 07/17] flask: unify {get,set}vcpucontext permissions
 [PATCH 08/17] flask: remove unused secondary context in ocontext
 [PATCH 09/17] flask: remove unused AVC callback functions
 [PATCH 10/17] flask: remove xen_flask_userlist operation
 [PATCH 11/17] flask: improve unknown permission handling

Hypervisor updates to the XSM framework (and its config):
 [PATCH 12/17] xen/xsm: remove .xsm_initcall.init section
 [PATCH 13/17] xen: fix FLASK dependency in Kconfig
 [PATCH 14/17] xsm: annotate setup functions with __init
 [PATCH 15/17] xsm: clean up unregistration
 [PATCH 16/17] xen: Make FLASK_AVC_STATS kconfig option visible
 [PATCH 17/17] xsm: add a default policy to .init.data

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/arch/arm/xen.lds.S  |  4 
 xen/arch/x86/xen.lds.S  |  5 +
 xen/common/Kconfig  | 17 +
 xen/xsm/flask/Makefile  | 17 +
 xen/xsm/xsm_core.c  | 15 ++-
 7 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index fed732c..c85d1dc 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -704,13 +704,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -141,21 +141,21 @@ only type enforcement is used and the user and role are 
set to system_u and
 system_r for all domains.
 
 The FLASK security framework is mostly configured using a security policy file.
-This policy file is not normally generated during the Xen build process because
-it relies on the SELinux compiler "checkpolicy"; run
-
-   make -C tools/flask/policy
-
-to compile the example policy included with Xen. The policy is generated from
-definition files under this directory. Most changes to security policy will
-involve creating or modifying modules found in tools/flask/policy/modules/.  
The
-modules.conf file there defines what modules are enabled and has short
-descriptions of each module.
-
-The XSM policy file needs to be copied to /boot and loaded as a module by grub.
-The exact position of the module does not matter as long as it is after the Xen
-kernel; it is normally placed either just above the dom0 kernel or at the end.
-Once dom0 is running, the policy can be reloaded using "xl loadpolicy".
+It relies on the SELinux compiler "checkpolicy"; if this is available, the
+policy will be compiled as part of the tools build.  If hypervisor support for 
a
+built-in policy is enabled ("Compile Xen with a built-in security policy"), the
+policy will be built during the hypervisor build.
+
+The policy is generated from definition files in tools/flask/policy.  Most
+changes to security policy will involve creating or modifying modules found in
+tools/flask/policy/modules/.  The modules.conf file there defines what modules
+are enabled and has short descriptions of each module.
+
+If not using the built-in policy, the XSM policy file needs to be copied to
+/boot a