[PATCH] selinux: add a new warn_on_audited debug flag to selinuxfs

* [PATCH] selinux: add a new warn_on_audited debug flag to selinuxfs
@ 2022-08-08 14:49 Ondrej Mosnacek
  2022-08-16  3:31 ` Paul Moore
  0 siblings, 1 reply; 3+ messages in thread
From: Ondrej Mosnacek @ 2022-08-08 14:49 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux

When debugging SELinux denials, it is often helpful to know which part
of kernel code triggered the denial. Thus, this patch adds a new
/sys/fs/selinux/warn_on_audited flag that, when set to 1, will cause any
audited AVC event to log a WARNING to the kernel console, which
naturally comes with a kernel stack trace.

While the same can be achieved via the "avc:selinux_audited" kernel
tracepoint and the perf tool, that approach has several practical
disadvantages:
1. It requires perf to be installed on the machine.
2. It requires kernel debug symbols to be available when decoding the
   stack trace.
3. It requires a perf process to be running in the background.
4. The stack traces can only be obtained at the end, after the perf
   process is terminated, not live during the capture. (Though this may
   be solved by writing a custom tool on top of libtraceevent.)

Thus, providing a simple native knob for this in selinuxfs is still
valuable.

The warn_on_audited flag is always set to 0 on boot and is expected to
be set to 1 only temporarily by system administrator in order to debug
SELinux denials. It is not intended to be used on production systems.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 security/selinux/avc.c              |  6 +++
 security/selinux/ima.c              | 11 +++++-
 security/selinux/include/security.h | 11 ++++++
 security/selinux/selinuxfs.c        | 61 +++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 9a43af0ebd7d..0e615c9e8e79 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -736,6 +736,12 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a)
 		audit_log_format(ab, " permissive=%u", sad->result ? 0 : 1);
 
 	trace_selinux_audited(sad, scontext, tcontext, tclass);
+
+	WARN(warn_on_audited_get(sad->state),
+	     "SELinux:  AV: requested=0x%x denied=0x%x audited=0x%x result=%d scontext=%s tcontext=%s tclass=%s",
+	     sad->requested, sad->denied, sad->audited, sad->result,
+	     scontext, tcontext, tclass);
+
 	kfree(tcontext);
 	kfree(scontext);
 
diff --git a/security/selinux/ima.c b/security/selinux/ima.c
index a915b89d55b0..506e880040f5 100644
--- a/security/selinux/ima.c
+++ b/security/selinux/ima.c
@@ -26,7 +26,10 @@ static char *selinux_ima_collect_state(struct selinux_state *state)
 	char *buf;
 	int buf_len, len, i, rc;
 
-	buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
+	buf_len = strlen("initialized=0;"
+			 "enforcing=0;"
+			 "checkreqprot=0;"
+			 "warn_on_audited=0;") + 1;
 
 	len = strlen(on);
 	for (i = 0; i < __POLICYDB_CAP_MAX; i++)
@@ -54,6 +57,12 @@ static char *selinux_ima_collect_state(struct selinux_state *state)
 	rc = strlcat(buf, checkreqprot_get(state) ? on : off, buf_len);
 	WARN_ON(rc >= buf_len);
 
+	rc = strlcat(buf, "warn_on_audited", buf_len);
+	WARN_ON(rc >= buf_len);
+
+	rc = strlcat(buf, warn_on_audited_get(state) ? on : off, buf_len);
+	WARN_ON(rc >= buf_len);
+
 	for (i = 0; i < __POLICYDB_CAP_MAX; i++) {
 		rc = strlcat(buf, selinux_policycap_names[i], buf_len);
 		WARN_ON(rc >= buf_len);
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 393aff41d3ef..bb1f0507edb6 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -97,6 +97,7 @@ struct selinux_state {
 	bool enforcing;
 #endif
 	bool checkreqprot;
+	bool warn_on_audited;
 	bool initialized;
 	bool policycap[__POLICYDB_CAP_MAX];
 
@@ -157,6 +158,16 @@ static inline void checkreqprot_set(struct selinux_state *state, bool value)
 	WRITE_ONCE(state->checkreqprot, value);
 }
 
+static inline bool warn_on_audited_get(const struct selinux_state *state)
+{
+	return READ_ONCE(state->warn_on_audited);
+}
+
+static inline void warn_on_audited_set(struct selinux_state *state, bool value)
+{
+	WRITE_ONCE(state->warn_on_audited, value);
+}
+
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 static inline bool selinux_disabled(struct selinux_state *state)
 {
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 8fcdd494af27..c3774ba39cf8 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -64,6 +64,7 @@ enum sel_inos {
 	SEL_STATUS,	/* export current status using mmap() */
 	SEL_POLICY,	/* allow userspace to read the in kernel policy */
 	SEL_VALIDATE_TRANS, /* compute validatetrans decision */
+	SEL_WARN_ON_AUDITED, /* whether to WARN() when a denial is audited */
 	SEL_INO_NEXT,	/* The next inode number to use */
 };
 
@@ -857,6 +858,64 @@ static const struct file_operations sel_transition_ops = {
 	.llseek		= generic_file_llseek,
 };
 
+static ssize_t sel_read_warn_on_audited(struct file *filp, char __user *buf,
+				     size_t count, loff_t *ppos)
+{
+	struct selinux_fs_info *fsi = file_inode(filp)->i_sb->s_fs_info;
+	char tmpbuf[TMPBUFLEN];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, TMPBUFLEN, "%u",
+			   warn_on_audited_get(fsi->state));
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static ssize_t sel_write_warn_on_audited(struct file *file, const char __user *buf,
+				      size_t count, loff_t *ppos)
+{
+	struct selinux_fs_info *fsi = file_inode(file)->i_sb->s_fs_info;
+	char *page;
+	ssize_t length;
+	unsigned int new_value;
+
+	length = avc_has_perm(&selinux_state,
+			      current_sid(), SECINITSID_SECURITY,
+			      SECCLASS_SECURITY, SECURITY__SETSECPARAM,
+			      NULL);
+	if (length)
+		return length;
+
+	if (count >= PAGE_SIZE)
+		return -ENOMEM;
+
+	/* No partial writes. */
+	if (*ppos != 0)
+		return -EINVAL;
+
+	page = memdup_user_nul(buf, count);
+	if (IS_ERR(page))
+		return PTR_ERR(page);
+
+	length = -EINVAL;
+	if (sscanf(page, "%u", &new_value) != 1)
+		goto out;
+
+	warn_on_audited_set(fsi->state, !!new_value);
+	length = count;
+
+	selinux_ima_measure_state(fsi->state);
+
+out:
+	kfree(page);
+	return length;
+}
+
+static const struct file_operations sel_warn_on_audited_ops = {
+	.read		= sel_read_warn_on_audited,
+	.write		= sel_write_warn_on_audited,
+	.llseek		= generic_file_llseek,
+};
+
 /*
  * Remaining nodes use transaction based IO methods like nfsd/nfsctl.c
  */
@@ -2084,6 +2143,8 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
 		[SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
 		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops,
 					S_IWUGO},
+		[SEL_WARN_ON_AUDITED] = {"warn_on_audited", &sel_warn_on_audited_ops,
+					 S_IRUGO|S_IWUSR},
 		/* last one */ {""}
 	};
 
-- 
2.37.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread