diff mbox series

[v3,2/7] xsm: remove the ability to disable flask

Message ID 20210805140644.357-3-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series xsm: refactoring xsm hooks | expand

Commit Message

Daniel P. Smith Aug. 5, 2021, 2:06 p.m. UTC
On Linux when SELinux is put into permissive mode the descretionary access
controls are still in place. Whereas for Xen when the enforcing state of flask
is set to permissive, all operations for all domains would succeed, i.e. it
does not fall back to the default access controls. To provide a means to mimic
a similar but not equivalent behavior, a flask op is present to allow a
one-time switch back to the default access controls, aka the "dummy policy".

This patch removes this flask op to enforce a consistent XSM usage model that a
reboot of Xen is required to change the XSM policy module in use.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/include/public/xsm/flask_op.h |  2 +-
 xen/xsm/flask/flask_op.c          | 30 ------------------------------
 2 files changed, 1 insertion(+), 31 deletions(-)

Comments

Jan Beulich Aug. 25, 2021, 3:22 p.m. UTC | #1
On 05.08.2021 16:06, Daniel P. Smith wrote:
> On Linux when SELinux is put into permissive mode the descretionary access
> controls are still in place. Whereas for Xen when the enforcing state of flask
> is set to permissive, all operations for all domains would succeed, i.e. it
> does not fall back to the default access controls. To provide a means to mimic
> a similar but not equivalent behavior, a flask op is present to allow a
> one-time switch back to the default access controls, aka the "dummy policy".
> 
> This patch removes this flask op to enforce a consistent XSM usage model that a
> reboot of Xen is required to change the XSM policy module in use.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

The primary reason you remove this is - aiui - that with alternatives
patching there's technically not really a way back (would need to re-
patch every patched location, or every hook would need to check whether
state changed to disabled and if so chain on to the dummy function).
This became sufficiently clear to me only when looking at the next
patch. It would be nice if description also said why the change is
needed. As it stands to me the description reads at best like something
that people could have different views on (and initially I didn't mean
to reply here, for not being convinced of the removal of functionality
in the common case).

Jan
Daniel P. Smith Aug. 27, 2021, 1:42 p.m. UTC | #2
On 8/25/21 11:22 AM, Jan Beulich wrote:
> On 05.08.2021 16:06, Daniel P. Smith wrote:
>> On Linux when SELinux is put into permissive mode the descretionary access
>> controls are still in place. Whereas for Xen when the enforcing state of flask
>> is set to permissive, all operations for all domains would succeed, i.e. it
>> does not fall back to the default access controls. To provide a means to mimic
>> a similar but not equivalent behavior, a flask op is present to allow a
>> one-time switch back to the default access controls, aka the "dummy policy".
>>
>> This patch removes this flask op to enforce a consistent XSM usage model that a
>> reboot of Xen is required to change the XSM policy module in use.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> 
> The primary reason you remove this is - aiui - that with alternatives
> patching there's technically not really a way back (would need to re-
> patch every patched location, or every hook would need to check whether
> state changed to disabled and if so chain on to the dummy function).
> This became sufficiently clear to me only when looking at the next
> patch. It would be nice if description also said why the change is
> needed. As it stands to me the description reads at best like something
> that people could have different views on (and initially I didn't mean
> to reply here, for not being convinced of the removal of functionality
> in the common case).
> 
> Jan
> 

Ack, I can expand further.

v/r,
dps
diff mbox series

Patch

diff --git a/xen/include/public/xsm/flask_op.h b/xen/include/public/xsm/flask_op.h
index 16af7bc22f..b41dd6dac8 100644
--- a/xen/include/public/xsm/flask_op.h
+++ b/xen/include/public/xsm/flask_op.h
@@ -188,7 +188,7 @@  struct xen_flask_op {
 #define FLASK_SETBOOL           12
 #define FLASK_COMMITBOOLS       13
 #define FLASK_MLS               14
-#define FLASK_DISABLE           15
+#define FLASK_DISABLE           15 /* No longer implemented */
 #define FLASK_GETAVC_THRESHOLD  16
 #define FLASK_SETAVC_THRESHOLD  17
 #define FLASK_AVC_HASHSTATS     18
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 01e52138a1..f41c025391 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -223,32 +223,6 @@  static int flask_security_sid(struct xen_flask_sid_context *arg)
 
 #ifndef COMPAT
 
-static int flask_disable(void)
-{
-    static int flask_disabled = 0;
-
-    if ( ss_initialized )
-    {
-        /* Not permitted after initial policy load. */
-        return -EINVAL;
-    }
-
-    if ( flask_disabled )
-    {
-        /* Only do this once. */
-        return -EINVAL;
-    }
-
-    printk("Flask:  Disabled at runtime.\n");
-
-    flask_disabled = 1;
-
-    /* Reset xsm_ops to the original module. */
-    xsm_ops = &dummy_xsm_ops;
-
-    return 0;
-}
-
 static int flask_security_setavc_threshold(struct xen_flask_setavc_threshold *arg)
 {
     int rv = 0;
@@ -698,10 +672,6 @@  ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
         rv = flask_mls_enabled;
         break;    
 
-    case FLASK_DISABLE:
-        rv = flask_disable();
-        break;
-
     case FLASK_GETAVC_THRESHOLD:
         rv = avc_cache_threshold;
         break;