1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
|
From d2ecc1f231b90d4e54394e25a9aef9be42c0d196 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu, 8 Aug 2024 13:44:56 +0200
Subject: [PATCH 03/35] XSM/domctl: Fix permission checks on
XEN_DOMCTL_createdomain
The XSM checks for XEN_DOMCTL_createdomain are problematic. There's a split
between xsm_domctl() called early, and flask_domain_create() called quite late
during domain construction.
All XSM implementations except Flask have a simple IS_PRIV check in
xsm_domctl(), and operate as expected when an unprivileged domain tries to
make a hypercall.
Flask however foregoes any action in xsm_domctl() and defers everything,
including the simple "is the caller permitted to create a domain" check, to
flask_domain_create().
As a consequence, when XSM Flask is active, and irrespective of the policy
loaded, all domains irrespective of privilege can:
* Mutate the global 'rover' variable, used to track the next free domid.
Therefore, all domains can cause a domid wraparound, and combined with a
voluntary reboot, choose their own domid.
* Cause a reasonable amount of a domain to be constructed before ultimately
failing for permission reasons, including the use of settings outside of
supported limits.
In order to remediate this, pass the ssidref into xsm_domctl() and at least
check that the calling domain privileged enough to create domains.
Take the opportunity to also fix the sign of the cmd parameter to be unsigned.
This issue has not been assigned an XSA, because Flask is experimental and not
security supported.
Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
master commit: ee32b9b29af449d38aad0a1b3a81aaae586f5ea7
master date: 2024-07-30 17:42:17 +0100
---
xen/arch/x86/mm/paging.c | 2 +-
xen/common/domctl.c | 4 +++-
xen/include/xsm/dummy.h | 2 +-
xen/include/xsm/xsm.h | 7 ++++---
xen/xsm/flask/hooks.c | 14 ++++++++++++--
5 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index bca320fffa..dd47bde5ce 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -767,7 +767,7 @@ long do_paging_domctl_cont(
if ( d == NULL )
return -ESRCH;
- ret = xsm_domctl(XSM_OTHER, d, op.cmd);
+ ret = xsm_domctl(XSM_OTHER, d, op.cmd, 0 /* SSIDref not applicable */);
if ( !ret )
{
if ( domctl_lock_acquire() )
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 2c0331bb05..ea16b75910 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -322,7 +322,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
break;
}
- ret = xsm_domctl(XSM_OTHER, d, op->cmd);
+ ret = xsm_domctl(XSM_OTHER, d, op->cmd,
+ /* SSIDRef only applicable for cmd == createdomain */
+ op->u.createdomain.ssidref);
if ( ret )
goto domctl_out_unlock_domonly;
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 00d2cbebf2..7956f27a29 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -162,7 +162,7 @@ static XSM_INLINE int cf_check xsm_set_target(
}
static XSM_INLINE int cf_check xsm_domctl(
- XSM_DEFAULT_ARG struct domain *d, int cmd)
+ XSM_DEFAULT_ARG struct domain *d, unsigned int cmd, uint32_t ssidref)
{
XSM_ASSERT_ACTION(XSM_OTHER);
switch ( cmd )
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 8dad03fd3d..627c0d2731 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -60,7 +60,7 @@ struct xsm_ops {
int (*domctl_scheduler_op)(struct domain *d, int op);
int (*sysctl_scheduler_op)(int op);
int (*set_target)(struct domain *d, struct domain *e);
- int (*domctl)(struct domain *d, int cmd);
+ int (*domctl)(struct domain *d, unsigned int cmd, uint32_t ssidref);
int (*sysctl)(int cmd);
int (*readconsole)(uint32_t clear);
@@ -248,9 +248,10 @@ static inline int xsm_set_target(
return alternative_call(xsm_ops.set_target, d, e);
}
-static inline int xsm_domctl(xsm_default_t def, struct domain *d, int cmd)
+static inline int xsm_domctl(xsm_default_t def, struct domain *d,
+ unsigned int cmd, uint32_t ssidref)
{
- return alternative_call(xsm_ops.domctl, d, cmd);
+ return alternative_call(xsm_ops.domctl, d, cmd, ssidref);
}
static inline int xsm_sysctl(xsm_default_t def, int cmd)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 5e88c71b8e..278ad38c2a 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -663,12 +663,22 @@ static int cf_check flask_set_target(struct domain *d, struct domain *t)
return rc;
}
-static int cf_check flask_domctl(struct domain *d, int cmd)
+static int cf_check flask_domctl(struct domain *d, unsigned int cmd,
+ uint32_t ssidref)
{
switch ( cmd )
{
- /* These have individual XSM hooks (common/domctl.c) */
case XEN_DOMCTL_createdomain:
+ /*
+ * There is a later hook too, but at this early point simply check
+ * that the calling domain is privileged enough to create a domain.
+ *
+ * Note that d is NULL because we haven't even allocated memory for it
+ * this early in XEN_DOMCTL_createdomain.
+ */
+ return avc_current_has_perm(ssidref, SECCLASS_DOMAIN, DOMAIN__CREATE, NULL);
+
+ /* These have individual XSM hooks (common/domctl.c) */
case XEN_DOMCTL_getdomaininfo:
case XEN_DOMCTL_scheduler_op:
case XEN_DOMCTL_irq_permission:
--
2.46.1
|
GitWeb