[PATCH v26 14/25] LSM: Specify which LSM to display

[PATCH v26 14/25] LSM: Specify which LSM to display

[Casey Schaufler]

Create a new entry "interface_lsm" in the procfs attr directory for
controlling which LSM security information is displayed for a
process. A process can only read or write its own display value.

The name of an active LSM that supplies hooks for
human readable data may be written to "interface_lsm" to set the
value. The name of the LSM currently in use can be read from
"interface_lsm". At this point there can only be one LSM capable
of display active. A helper function lsm_task_ilsm() is
provided to get the interface lsm slot for a task_struct.

Setting the "interface_lsm" requires that all security modules using
setprocattr hooks allow the action. Each security module is
responsible for defining its policy.

AppArmor hook provided by John Johansen <john.johansen@canonical.com>
SELinux hook provided by Stephen Smalley <stephen.smalley.work@gmail.com>

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-api@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
 .../ABI/testing/procfs-attr-lsm_display       |  22 +++
 Documentation/security/lsm.rst                |  14 ++
 fs/proc/base.c                                |   1 +
 include/linux/lsm_hooks.h                     |  17 ++
 security/apparmor/include/apparmor.h          |   3 +-
 security/apparmor/lsm.c                       |  32 ++++
 security/security.c                           | 166 ++++++++++++++++--
 security/selinux/hooks.c                      |  11 ++
 security/selinux/include/classmap.h           |   2 +-
 security/smack/smack_lsm.c                    |   7 +
 10 files changed, 256 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/ABI/testing/procfs-attr-lsm_display

diff --git a/Documentation/ABI/testing/procfs-attr-lsm_display b/Documentation/ABI/testing/procfs-attr-lsm_display
new file mode 100644
index 000000000000..0f60005c235c
--- /dev/null
+++ b/Documentation/ABI/testing/procfs-attr-lsm_display
@@ -0,0 +1,22 @@
+What:		/proc/*/attr/lsm_display
+Contact:	linux-security-module@vger.kernel.org,
+Description:	The name of the Linux security module (LSM) that will
+		provide information in the /proc/*/attr/current,
+		/proc/*/attr/prev and /proc/*/attr/exec interfaces.
+		The details of permissions required to read from
+		this interface are dependent on the LSMs active on the
+		system.
+		A process cannot write to this interface unless it
+		refers to itself.
+		The other details of permissions required to write to
+		this interface are dependent on the LSMs active on the
+		system.
+		The format of the data used by this interface is a
+		text string identifying the name of an LSM. The values
+		accepted are:
+			selinux		- the SELinux LSM
+			smack		- the Smack LSM
+			apparmor	- The AppArmor LSM
+		By convention the LSM names are lower case and do not
+		contain special characters.
+Users:		LSM user-space
diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst
index 6a2a2e973080..b77b4a540391 100644
--- a/Documentation/security/lsm.rst
+++ b/Documentation/security/lsm.rst
@@ -129,3 +129,17 @@ to identify it as the first security module to be registered.
 The capabilities security module does not use the general security
 blobs, unlike other modules. The reasons are historical and are
 based on overhead, complexity and performance concerns.
+
+LSM External Interfaces
+=======================
+
+The LSM infrastructure does not generally provide external interfaces.
+The individual security modules provide what external interfaces they
+require.
+
+The file ``/sys/kernel/security/lsm`` provides a comma
+separated list of the active security modules.
+
+The file ``/proc/pid/attr/interface_lsm`` contains the name of the security
+module for which the ``/proc/pid/attr/current`` interface will
+apply. This interface can be written to.
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 3851bfcdba56..10de522f3112 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2807,6 +2807,7 @@ static const struct pid_entry attr_dir_stuff[] = {
 	ATTR(NULL, "fscreate",		0666),
 	ATTR(NULL, "keycreate",		0666),
 	ATTR(NULL, "sockcreate",	0666),
+	ATTR(NULL, "interface_lsm",	0666),
 #ifdef CONFIG_SECURITY_SMACK
 	DIR("smack",			0555,
 	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c61a16f0a5bc..d2c4bc94d47f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1686,4 +1686,21 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
 
 extern int lsm_inode_alloc(struct inode *inode);
 
+/**
+ * lsm_task_ilsm - the "interface_lsm" for this task
+ * @task: The task to report on
+ *
+ * Returns the task's interface LSM slot.
+ */
+static inline int lsm_task_ilsm(struct task_struct *task)
+{
+#ifdef CONFIG_SECURITY
+	int *ilsm = task->security;
+
+	if (ilsm)
+		return *ilsm;
+#endif
+	return LSMBLOB_INVALID;
+}
+
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
index 1fbabdb565a8..b1622fcb4394 100644
--- a/security/apparmor/include/apparmor.h
+++ b/security/apparmor/include/apparmor.h
@@ -28,8 +28,9 @@
 #define AA_CLASS_SIGNAL		10
 #define AA_CLASS_NET		14
 #define AA_CLASS_LABEL		16
+#define AA_CLASS_DISPLAY_LSM	17
 
-#define AA_CLASS_LAST		AA_CLASS_LABEL
+#define AA_CLASS_LAST		AA_CLASS_DISPLAY_LSM
 
 /* Control parameters settable through module/boot flags */
 extern enum audit_mode aa_g_audit;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 392e25940d1f..4237536106aa 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -621,6 +621,25 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
 	return error;
 }
 
+
+static int profile_interface_lsm(struct aa_profile *profile,
+				 struct common_audit_data *sa)
+{
+	struct aa_perms perms = { };
+	unsigned int state;
+
+	state = PROFILE_MEDIATES(profile, AA_CLASS_DISPLAY_LSM);
+	if (state) {
+		aa_compute_perms(profile->policy.dfa, state, &perms);
+		aa_apply_modes_to_perms(profile, &perms);
+		aad(sa)->label = &profile->label;
+
+		return aa_check_perms(profile, &perms, AA_MAY_WRITE, sa, NULL);
+	}
+
+	return 0;
+}
+
 static int apparmor_setprocattr(const char *name, void *value,
 				size_t size)
 {
@@ -632,6 +651,19 @@ static int apparmor_setprocattr(const char *name, void *value,
 	if (size == 0)
 		return -EINVAL;
 
+	/* LSM infrastructure does actual setting of interface_lsm if allowed */
+	if (!strcmp(name, "interface_lsm")) {
+		struct aa_profile *profile;
+		struct aa_label *label;
+
+		aad(&sa)->info = "set interface lsm";
+		label = begin_current_label_crit_section();
+		error = fn_for_each_confined(label, profile,
+					profile_interface_lsm(profile, &sa));
+		end_current_label_crit_section(label);
+		return error;
+	}
+
 	/* AppArmor requires that the buffer must be null terminated atm */
 	if (args[size - 1] != '\0') {
 		/* null terminate */
diff --git a/security/security.c b/security/security.c
index f5407a85641e..1ce125c01782 100644
--- a/security/security.c
+++ b/security/security.c
@@ -77,7 +77,16 @@ static struct kmem_cache *lsm_file_cache;
 static struct kmem_cache *lsm_inode_cache;
 
 char *lsm_names;
-static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
+
+/*
+ * The task blob includes the "interface_lsm" slot used for
+ * chosing which module presents contexts.
+ * Using a long to avoid potential alignment issues with
+ * module assigned task blobs.
+ */
+static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
+	.lbs_task = sizeof(long),
+};
 
 /* Boot-time LSM user choice */
 static __initdata const char *chosen_lsm_order;
@@ -669,6 +678,8 @@ int lsm_inode_alloc(struct inode *inode)
  */
 static int lsm_task_alloc(struct task_struct *task)
 {
+	int *ilsm;
+
 	if (blob_sizes.lbs_task == 0) {
 		task->security = NULL;
 		return 0;
@@ -677,6 +688,15 @@ static int lsm_task_alloc(struct task_struct *task)
 	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
 	if (task->security == NULL)
 		return -ENOMEM;
+
+	/*
+	 * The start of the task blob contains the "interface" LSM slot number.
+	 * Start with it set to the invalid slot number, indicating that the
+	 * default first registered LSM be displayed.
+	 */
+	ilsm = task->security;
+	*ilsm = LSMBLOB_INVALID;
+
 	return 0;
 }
 
@@ -1732,14 +1752,26 @@ int security_file_open(struct file *file)
 
 int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
 {
+	int *oilsm = current->security;
+	int *nilsm;
 	int rc = lsm_task_alloc(task);
 
-	if (rc)
+	if (unlikely(rc))
 		return rc;
+
 	rc = call_int_hook(task_alloc, 0, task, clone_flags);
-	if (unlikely(rc))
+	if (unlikely(rc)) {
 		security_task_free(task);
-	return rc;
+		return rc;
+	}
+
+	if (oilsm) {
+		nilsm = task->security;
+		if (nilsm)
+			*nilsm = *oilsm;
+	}
+
+	return 0;
 }
 
 void security_task_free(struct task_struct *task)
@@ -2171,23 +2203,110 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
 				char **value)
 {
 	struct security_hook_list *hp;
+	int ilsm = lsm_task_ilsm(current);
+	int slot = 0;
+
+	if (!strcmp(name, "interface_lsm")) {
+		/*
+		 * lsm_slot will be 0 if there are no displaying modules.
+		 */
+		if (lsm_slot == 0)
+			return -EINVAL;
+
+		/*
+		 * Only allow getting the current process' interface_lsm.
+		 * There are too few reasons to get another process'
+		 * interface_lsm and too many LSM policy issues.
+		 */
+		if (current != p)
+			return -EINVAL;
+
+		ilsm = lsm_task_ilsm(p);
+		if (ilsm != LSMBLOB_INVALID)
+			slot = ilsm;
+		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
+		if (*value)
+			return strlen(*value);
+		return -ENOMEM;
+	}
 
 	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
 		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
 			continue;
+		if (lsm == NULL && ilsm != LSMBLOB_INVALID &&
+		    ilsm != hp->lsmid->slot)
+			continue;
 		return hp->hook.getprocattr(p, name, value);
 	}
 	return LSM_RET_DEFAULT(getprocattr);
 }
 
+/**
+ * security_setprocattr - Set process attributes via /proc
+ * @lsm: name of module involved, or NULL
+ * @name: name of the attribute
+ * @value: value to set the attribute to
+ * @size: size of the value
+ *
+ * Set the process attribute for the specified security module
+ * to the specified value. Note that this can only be used to set
+ * the process attributes for the current, or "self" process.
+ * The /proc code has already done this check.
+ *
+ * Returns 0 on success, an appropriate code otherwise.
+ */
 int security_setprocattr(const char *lsm, const char *name, void *value,
 			 size_t size)
 {
 	struct security_hook_list *hp;
+	char *termed;
+	char *copy;
+	int *ilsm = current->security;
+	int rc = -EINVAL;
+	int slot = 0;
+
+	if (!strcmp(name, "interface_lsm")) {
+		/*
+		 * Change the "interface_lsm" value only if all the security
+		 * modules that support setting a procattr allow it.
+		 * It is assumed that all such security modules will be
+		 * cooperative.
+		 */
+		if (size == 0)
+			return -EINVAL;
+
+		hlist_for_each_entry(hp, &security_hook_heads.setprocattr,
+				     list) {
+			rc = hp->hook.setprocattr(name, value, size);
+			if (rc < 0)
+				return rc;
+		}
+
+		rc = -EINVAL;
+
+		copy = kmemdup_nul(value, size, GFP_KERNEL);
+		if (copy == NULL)
+			return -ENOMEM;
+
+		termed = strsep(&copy, " \n");
+
+		for (slot = 0; slot < lsm_slot; slot++)
+			if (!strcmp(termed, lsm_slotlist[slot]->lsm)) {
+				*ilsm = lsm_slotlist[slot]->slot;
+				rc = size;
+				break;
+			}
+
+		kfree(termed);
+		return rc;
+	}
 
 	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
 		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
 			continue;
+		if (lsm == NULL && *ilsm != LSMBLOB_INVALID &&
+		    *ilsm != hp->lsmid->slot)
+			continue;
 		return hp->hook.setprocattr(name, value, size);
 	}
 	return LSM_RET_DEFAULT(setprocattr);
@@ -2207,15 +2326,15 @@ EXPORT_SYMBOL(security_ismaclabel);
 int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
 {
 	struct security_hook_list *hp;
-	int rc;
+	int ilsm = lsm_task_ilsm(current);
 
 	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
 		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
 			continue;
-		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
-					      secdata, seclen);
-		if (rc != LSM_RET_DEFAULT(secid_to_secctx))
-			return rc;
+		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
+			return hp->hook.secid_to_secctx(
+					blob->secid[hp->lsmid->slot],
+					secdata, seclen);
 	}
 
 	return LSM_RET_DEFAULT(secid_to_secctx);
@@ -2226,16 +2345,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
 			     struct lsmblob *blob)
 {
 	struct security_hook_list *hp;
-	int rc;
+	int ilsm = lsm_task_ilsm(current);
 
 	lsmblob_init(blob, 0);
 	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
 		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
 			continue;
-		rc = hp->hook.secctx_to_secid(secdata, seclen,
-					      &blob->secid[hp->lsmid->slot]);
-		if (rc != 0)
-			return rc;
+		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
+			return hp->hook.secctx_to_secid(secdata, seclen,
+						&blob->secid[hp->lsmid->slot]);
 	}
 	return 0;
 }
@@ -2243,7 +2361,14 @@ EXPORT_SYMBOL(security_secctx_to_secid);
 
 void security_release_secctx(char *secdata, u32 seclen)
 {
-	call_void_hook(release_secctx, secdata, seclen);
+	struct security_hook_list *hp;
+	int ilsm = lsm_task_ilsm(current);
+
+	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
+		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot) {
+			hp->hook.release_secctx(secdata, seclen);
+			return;
+		}
 }
 EXPORT_SYMBOL(security_release_secctx);
 
@@ -2384,8 +2509,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
 int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 				      int __user *optlen, unsigned len)
 {
-	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
-				optval, optlen, len);
+	int ilsm = lsm_task_ilsm(current);
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
+			     list)
+		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
+			return hp->hook.socket_getpeersec_stream(sock, optval,
+								 optlen, len);
+	return -ENOPROTOOPT;
 }
 
 int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0133b142e938..dba867721336 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6510,6 +6510,17 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
 	/*
 	 * Basic control over ability to set these attributes at all.
 	 */
+
+	/*
+	 * For setting interface_lsm, we only perform a permission check;
+	 * the actual update to the interface_lsm value is handled by the
+	 * LSM framework.
+	 */
+	if (!strcmp(name, "interface_lsm"))
+		return avc_has_perm(&selinux_state,
+				    mysid, mysid, SECCLASS_PROCESS2,
+				    PROCESS2__SETDISPLAY, NULL);
+
 	if (!strcmp(name, "exec"))
 		error = avc_has_perm(&selinux_state,
 				     mysid, mysid, SECCLASS_PROCESS,
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 62d19bccf3de..8f4b0dd6dd78 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -53,7 +53,7 @@ struct security_class_mapping secclass_map[] = {
 	    "execmem", "execstack", "execheap", "setkeycreate",
 	    "setsockcreate", "getrlimit", NULL } },
 	{ "process2",
-	  { "nnp_transition", "nosuid_transition", NULL } },
+	  { "nnp_transition", "nosuid_transition", "setdisplay", NULL } },
 	{ "system",
 	  { "ipc_info", "syslog_read", "syslog_mod",
 	    "syslog_console", "module_request", "module_load", NULL } },
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5c10ad27be37..7aa7ea38f627 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3508,6 +3508,13 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
 	struct smack_known_list_elem *sklep;
 	int rc;
 
+	/*
+	 * Allow the /proc/.../attr/current and SO_PEERSEC "interface_lsm"
+	 * to be reset at will.
+	 */
+	if (strcmp(name, "interface_lsm") == 0)
+		return 0;
+
 	if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
 		return -EPERM;
 
-- 
2.29.2

[PATCH v26 24/25] LSM: Add /proc attr entry for full LSM context

[Casey Schaufler]

Add an entry /proc/.../attr/context which displays the full
process security "context" in compound format:
        lsm1\0value\0lsm2\0value\0...
This entry is not writable.

A security module may decide that its policy does not allow
this information to be displayed. In this case none of the
information will be displayed.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-api@vger.kernel.org
Cc: linux-doc@vger.kernel.org
---
 Documentation/ABI/testing/procfs-attr-context | 14 ++++
 Documentation/security/lsm.rst                | 14 ++++
 fs/proc/base.c                                |  1 +
 include/linux/lsm_hooks.h                     |  6 ++
 security/apparmor/include/procattr.h          |  2 +-
 security/apparmor/lsm.c                       |  8 +-
 security/apparmor/procattr.c                  | 22 +++---
 security/security.c                           | 79 +++++++++++++++++++
 security/selinux/hooks.c                      |  2 +-
 security/smack/smack_lsm.c                    |  2 +-
 10 files changed, 135 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/ABI/testing/procfs-attr-context

diff --git a/Documentation/ABI/testing/procfs-attr-context b/Documentation/ABI/testing/procfs-attr-context
new file mode 100644
index 000000000000..40da1c397c30
--- /dev/null
+++ b/Documentation/ABI/testing/procfs-attr-context
@@ -0,0 +1,14 @@
+What:		/proc/*/attr/context
+Contact:	linux-security-module@vger.kernel.org,
+Description:	The current security information used by all Linux
+		security module (LSMs) that are active on the system.
+		The details of permissions required to read from
+		this interface and hence obtain the security state
+		of the task identified is dependent on the LSMs that
+		are active on the system.
+		A process cannot write to this interface.
+		The data provided by this interface will have the form:
+			lsm_name\0lsm_data\0[lsm_name\0lsm_data\0]...
+		where lsm_name is the name of the LSM and the following
+		lsm_data is the process data for that LSM.
+Users:		LSM user-space
diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst
index b77b4a540391..070225ae6ceb 100644
--- a/Documentation/security/lsm.rst
+++ b/Documentation/security/lsm.rst
@@ -143,3 +143,17 @@ separated list of the active security modules.
 The file ``/proc/pid/attr/interface_lsm`` contains the name of the security
 module for which the ``/proc/pid/attr/current`` interface will
 apply. This interface can be written to.
+
+The infrastructure does provide an interface for the special
+case where multiple security modules provide a process context.
+This is provided in compound context format.
+
+-  `lsm\0value\0lsm\0value\0`
+
+The `lsm` and `value` fields are NUL-terminated bytestrings.
+Each field may contain whitespace or non-printable characters.
+The NUL bytes are included in the size of a compound context.
+The context ``Bell\0Secret\0Biba\0Loose\0`` has a size of 23.
+
+The file ``/proc/pid/attr/context`` provides the security
+context of the identified process.
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 10de522f3112..23ebfc35435c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2808,6 +2808,7 @@ static const struct pid_entry attr_dir_stuff[] = {
 	ATTR(NULL, "keycreate",		0666),
 	ATTR(NULL, "sockcreate",	0666),
 	ATTR(NULL, "interface_lsm",	0666),
+	ATTR(NULL, "context",		0444),
 #ifdef CONFIG_SECURITY_SMACK
 	DIR("smack",			0555,
 	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d2c4bc94d47f..f6ffe8b069e2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1397,6 +1397,12 @@
  *	@pages contains the number of pages.
  *	Return 0 if permission is granted.
  *
+ * @getprocattr:
+ *	Provide the named process attribute for display in special files in
+ *	the /proc/.../attr directory.  Attribute naming and the data displayed
+ *	is at the discretion of the security modules.  The exception is the
+ *	"context" attribute, which will contain the security context of the
+ *	task as a nul terminated text string without trailing whitespace.
  * @ismaclabel:
  *	Check if the extended attribute specified by @name
  *	represents a MAC label. Returns 1 if name is a MAC
diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
index 31689437e0e1..03dbfdb2f2c0 100644
--- a/security/apparmor/include/procattr.h
+++ b/security/apparmor/include/procattr.h
@@ -11,7 +11,7 @@
 #ifndef __AA_PROCATTR_H
 #define __AA_PROCATTR_H
 
-int aa_getprocattr(struct aa_label *label, char **string);
+int aa_getprocattr(struct aa_label *label, char **string, bool newline);
 int aa_setprocattr_changehat(char *args, size_t size, int flags);
 
 #endif /* __AA_PROCATTR_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 4237536106aa..65a004597e53 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -602,6 +602,7 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
 	const struct cred *cred = get_task_cred(task);
 	struct aa_task_ctx *ctx = task_ctx(current);
 	struct aa_label *label = NULL;
+	bool newline = true;
 
 	if (strcmp(name, "current") == 0)
 		label = aa_get_newest_label(cred_label(cred));
@@ -609,11 +610,14 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
 		label = aa_get_newest_label(ctx->previous);
 	else if (strcmp(name, "exec") == 0 && ctx->onexec)
 		label = aa_get_newest_label(ctx->onexec);
-	else
+	else if (strcmp(name, "context") == 0) {
+		label = aa_get_newest_label(cred_label(cred));
+		newline = false;
+	} else
 		error = -EINVAL;
 
 	if (label)
-		error = aa_getprocattr(label, value);
+		error = aa_getprocattr(label, value, newline);
 
 	aa_put_label(label);
 	put_cred(cred);
diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
index c929bf4a3df1..be3b083d9b74 100644
--- a/security/apparmor/procattr.c
+++ b/security/apparmor/procattr.c
@@ -20,6 +20,7 @@
  * aa_getprocattr - Return the profile information for @profile
  * @profile: the profile to print profile info about  (NOT NULL)
  * @string: Returns - string containing the profile info (NOT NULL)
+ * @newline: Should a newline be added to @string.
  *
  * Returns: length of @string on success else error on failure
  *
@@ -30,20 +31,21 @@
  *
  * Returns: size of string placed in @string else error code on failure
  */
-int aa_getprocattr(struct aa_label *label, char **string)
+int aa_getprocattr(struct aa_label *label, char **string, bool newline)
 {
 	struct aa_ns *ns = labels_ns(label);
 	struct aa_ns *current_ns = aa_get_current_ns();
+	int flags = FLAG_VIEW_SUBNS | FLAG_HIDDEN_UNCONFINED;
 	int len;
 
 	if (!aa_ns_visible(current_ns, ns, true)) {
 		aa_put_ns(current_ns);
 		return -EACCES;
 	}
+	if (newline)
+		flags |= FLAG_SHOW_MODE;
 
-	len = aa_label_snxprint(NULL, 0, current_ns, label,
-				FLAG_SHOW_MODE | FLAG_VIEW_SUBNS |
-				FLAG_HIDDEN_UNCONFINED);
+	len = aa_label_snxprint(NULL, 0, current_ns, label, flags);
 	AA_BUG(len < 0);
 
 	*string = kmalloc(len + 2, GFP_KERNEL);
@@ -52,19 +54,19 @@ int aa_getprocattr(struct aa_label *label, char **string)
 		return -ENOMEM;
 	}
 
-	len = aa_label_snxprint(*string, len + 2, current_ns, label,
-				FLAG_SHOW_MODE | FLAG_VIEW_SUBNS |
-				FLAG_HIDDEN_UNCONFINED);
+	len = aa_label_snxprint(*string, len + 2, current_ns, label, flags);
 	if (len < 0) {
 		aa_put_ns(current_ns);
 		return len;
 	}
 
-	(*string)[len] = '\n';
-	(*string)[len + 1] = 0;
+	if (newline) {
+		(*string)[len] = '\n';
+		(*string)[++len] = 0;
+	}
 
 	aa_put_ns(current_ns);
-	return len + 1;
+	return len;
 }
 
 /**
diff --git a/security/security.c b/security/security.c
index bb4c7f6c62ec..d3ff61442877 100644
--- a/security/security.c
+++ b/security/security.c
@@ -799,6 +799,57 @@ static int lsm_superblock_alloc(struct super_block *sb)
 	return 0;
 }
 
+/**
+ * append_ctx - append a lsm/context pair to a compound context
+ * @ctx: the existing compound context
+ * @ctxlen: size of the old context, including terminating nul byte
+ * @lsm: new lsm name, nul terminated
+ * @new: new context, possibly nul terminated
+ * @newlen: maximum size of @new
+ *
+ * replace @ctx with a new compound context, appending @newlsm and @new
+ * to @ctx. On exit the new data replaces the old, which is freed.
+ * @ctxlen is set to the new size, which includes a trailing nul byte.
+ *
+ * Returns 0 on success, -ENOMEM if no memory is available.
+ */
+static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new,
+		      int newlen)
+{
+	char *final;
+	size_t llen;
+	size_t nlen;
+	size_t flen;
+
+	llen = strlen(lsm) + 1;
+	/*
+	 * A security module may or may not provide a trailing nul on
+	 * when returning a security context. There is no definition
+	 * of which it should be, and there are modules that do it
+	 * each way.
+	 */
+	nlen = strnlen(new, newlen);
+
+	flen = *ctxlen + llen + nlen + 1;
+	final = kzalloc(flen, GFP_KERNEL);
+
+	if (final == NULL)
+		return -ENOMEM;
+
+	if (*ctxlen)
+		memcpy(final, *ctx, *ctxlen);
+
+	memcpy(final + *ctxlen, lsm, llen);
+	memcpy(final + *ctxlen + llen, new, nlen);
+
+	kfree(*ctx);
+
+	*ctx = final;
+	*ctxlen = flen;
+
+	return 0;
+}
+
 /*
  * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
  * can be accessed with:
@@ -2232,6 +2283,10 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
 				char **value)
 {
 	struct security_hook_list *hp;
+	char *final = NULL;
+	char *cp;
+	int rc = 0;
+	int finallen = 0;
 	int ilsm = lsm_task_ilsm(current);
 	int slot = 0;
 
@@ -2259,6 +2314,30 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
 		return -ENOMEM;
 	}
 
+	if (!strcmp(name, "context")) {
+		hlist_for_each_entry(hp, &security_hook_heads.getprocattr,
+				     list) {
+			rc = hp->hook.getprocattr(p, "context", &cp);
+			if (rc == -EINVAL)
+				continue;
+			if (rc < 0) {
+				kfree(final);
+				return rc;
+			}
+			rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
+					cp, rc);
+			kfree(cp);
+			if (rc < 0) {
+				kfree(final);
+				return rc;
+			}
+		}
+		if (final == NULL)
+			return -EINVAL;
+		*value = final;
+		return finallen;
+	}
+
 	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
 		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
 			continue;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b7800fa55a34..0a9af748f77c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6468,7 +6468,7 @@ static int selinux_getprocattr(struct task_struct *p,
 			goto bad;
 	}
 
-	if (!strcmp(name, "current"))
+	if (!strcmp(name, "current") || !strcmp(name, "context"))
 		sid = __tsec->sid;
 	else if (!strcmp(name, "prev"))
 		sid = __tsec->osid;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index e65497a5c095..1618d7d6154b 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3477,7 +3477,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
 	char *cp;
 	int slen;
 
-	if (strcmp(name, "current") != 0)
+	if (strcmp(name, "current") != 0 && strcmp(name, "context") != 0)
 		return -EINVAL;
 
 	cp = kstrdup(skp->smk_known, GFP_KERNEL);
-- 
2.29.2

Re: [PATCH v26 14/25] LSM: Specify which LSM to display

[Kees Cook]

On Thu, May 13, 2021 at 01:07:56PM -0700, Casey Schaufler wrote:
> Create a new entry "interface_lsm" in the procfs attr directory for
> controlling which LSM security information is displayed for a
> process. A process can only read or write its own display value.
> 
> The name of an active LSM that supplies hooks for
> human readable data may be written to "interface_lsm" to set the
> value. The name of the LSM currently in use can be read from
> "interface_lsm". At this point there can only be one LSM capable
> of display active. A helper function lsm_task_ilsm() is
> provided to get the interface lsm slot for a task_struct.
> 
> Setting the "interface_lsm" requires that all security modules using
> setprocattr hooks allow the action. Each security module is
> responsible for defining its policy.
> 
> AppArmor hook provided by John Johansen <john.johansen@canonical.com>
> SELinux hook provided by Stephen Smalley <stephen.smalley.work@gmail.com>
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-api@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> ---
>  .../ABI/testing/procfs-attr-lsm_display       |  22 +++
>  Documentation/security/lsm.rst                |  14 ++
>  fs/proc/base.c                                |   1 +
>  include/linux/lsm_hooks.h                     |  17 ++
>  security/apparmor/include/apparmor.h          |   3 +-
>  security/apparmor/lsm.c                       |  32 ++++
>  security/security.c                           | 166 ++++++++++++++++--
>  security/selinux/hooks.c                      |  11 ++
>  security/selinux/include/classmap.h           |   2 +-
>  security/smack/smack_lsm.c                    |   7 +
>  10 files changed, 256 insertions(+), 19 deletions(-)
>  create mode 100644 Documentation/ABI/testing/procfs-attr-lsm_display
> 
> diff --git a/Documentation/ABI/testing/procfs-attr-lsm_display b/Documentation/ABI/testing/procfs-attr-lsm_display
> new file mode 100644
> index 000000000000..0f60005c235c
> --- /dev/null
> +++ b/Documentation/ABI/testing/procfs-attr-lsm_display
> @@ -0,0 +1,22 @@
> +What:		/proc/*/attr/lsm_display
> +Contact:	linux-security-module@vger.kernel.org,
> +Description:	The name of the Linux security module (LSM) that will
> +		provide information in the /proc/*/attr/current,
> +		/proc/*/attr/prev and /proc/*/attr/exec interfaces.
> +		The details of permissions required to read from
> +		this interface are dependent on the LSMs active on the
> +		system.
> +		A process cannot write to this interface unless it
> +		refers to itself.
> +		The other details of permissions required to write to
> +		this interface are dependent on the LSMs active on the
> +		system.
> +		The format of the data used by this interface is a
> +		text string identifying the name of an LSM. The values
> +		accepted are:
> +			selinux		- the SELinux LSM
> +			smack		- the Smack LSM
> +			apparmor	- The AppArmor LSM
> +		By convention the LSM names are lower case and do not
> +		contain special characters.
> +Users:		LSM user-space
> diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst
> index 6a2a2e973080..b77b4a540391 100644
> --- a/Documentation/security/lsm.rst
> +++ b/Documentation/security/lsm.rst
> @@ -129,3 +129,17 @@ to identify it as the first security module to be registered.
>  The capabilities security module does not use the general security
>  blobs, unlike other modules. The reasons are historical and are
>  based on overhead, complexity and performance concerns.
> +
> +LSM External Interfaces
> +=======================
> +
> +The LSM infrastructure does not generally provide external interfaces.
> +The individual security modules provide what external interfaces they
> +require.
> +
> +The file ``/sys/kernel/security/lsm`` provides a comma
> +separated list of the active security modules.
> +
> +The file ``/proc/pid/attr/interface_lsm`` contains the name of the security
> +module for which the ``/proc/pid/attr/current`` interface will
> +apply. This interface can be written to.
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 3851bfcdba56..10de522f3112 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2807,6 +2807,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>  	ATTR(NULL, "fscreate",		0666),
>  	ATTR(NULL, "keycreate",		0666),
>  	ATTR(NULL, "sockcreate",	0666),
> +	ATTR(NULL, "interface_lsm",	0666),

Anyone can open this file... (continued below)

>  #ifdef CONFIG_SECURITY_SMACK
>  	DIR("smack",			0555,
>  	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index c61a16f0a5bc..d2c4bc94d47f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1686,4 +1686,21 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
>  
>  extern int lsm_inode_alloc(struct inode *inode);
>  
> +/**
> + * lsm_task_ilsm - the "interface_lsm" for this task
> + * @task: The task to report on
> + *
> + * Returns the task's interface LSM slot.
> + */
> +static inline int lsm_task_ilsm(struct task_struct *task)
> +{
> +#ifdef CONFIG_SECURITY
> +	int *ilsm = task->security;
> +
> +	if (ilsm)
> +		return *ilsm;
> +#endif
> +	return LSMBLOB_INVALID;
> +}
> +
>  #endif /* ! __LINUX_LSM_HOOKS_H */
> diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
> index 1fbabdb565a8..b1622fcb4394 100644
> --- a/security/apparmor/include/apparmor.h
> +++ b/security/apparmor/include/apparmor.h
> @@ -28,8 +28,9 @@
>  #define AA_CLASS_SIGNAL		10
>  #define AA_CLASS_NET		14
>  #define AA_CLASS_LABEL		16
> +#define AA_CLASS_DISPLAY_LSM	17
>  
> -#define AA_CLASS_LAST		AA_CLASS_LABEL
> +#define AA_CLASS_LAST		AA_CLASS_DISPLAY_LSM
>  
>  /* Control parameters settable through module/boot flags */
>  extern enum audit_mode aa_g_audit;
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 392e25940d1f..4237536106aa 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -621,6 +621,25 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
>  	return error;
>  }
>  
> +
> +static int profile_interface_lsm(struct aa_profile *profile,
> +				 struct common_audit_data *sa)
> +{
> +	struct aa_perms perms = { };
> +	unsigned int state;
> +
> +	state = PROFILE_MEDIATES(profile, AA_CLASS_DISPLAY_LSM);
> +	if (state) {
> +		aa_compute_perms(profile->policy.dfa, state, &perms);
> +		aa_apply_modes_to_perms(profile, &perms);
> +		aad(sa)->label = &profile->label;
> +
> +		return aa_check_perms(profile, &perms, AA_MAY_WRITE, sa, NULL);
> +	}
> +
> +	return 0;
> +}
> +
>  static int apparmor_setprocattr(const char *name, void *value,
>  				size_t size)
>  {
> @@ -632,6 +651,19 @@ static int apparmor_setprocattr(const char *name, void *value,
>  	if (size == 0)
>  		return -EINVAL;
>  
> +	/* LSM infrastructure does actual setting of interface_lsm if allowed */
> +	if (!strcmp(name, "interface_lsm")) {
> +		struct aa_profile *profile;
> +		struct aa_label *label;
> +
> +		aad(&sa)->info = "set interface lsm";
> +		label = begin_current_label_crit_section();
> +		error = fn_for_each_confined(label, profile,
> +					profile_interface_lsm(profile, &sa));
> +		end_current_label_crit_section(label);
> +		return error;
> +	}
> +
>  	/* AppArmor requires that the buffer must be null terminated atm */
>  	if (args[size - 1] != '\0') {
>  		/* null terminate */
> diff --git a/security/security.c b/security/security.c
> index f5407a85641e..1ce125c01782 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -77,7 +77,16 @@ static struct kmem_cache *lsm_file_cache;
>  static struct kmem_cache *lsm_inode_cache;
>  
>  char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +
> +/*
> + * The task blob includes the "interface_lsm" slot used for
> + * chosing which module presents contexts.
> + * Using a long to avoid potential alignment issues with
> + * module assigned task blobs.
> + */
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> +	.lbs_task = sizeof(long),
> +};
>  
>  /* Boot-time LSM user choice */
>  static __initdata const char *chosen_lsm_order;
> @@ -669,6 +678,8 @@ int lsm_inode_alloc(struct inode *inode)
>   */
>  static int lsm_task_alloc(struct task_struct *task)
>  {
> +	int *ilsm;
> +
>  	if (blob_sizes.lbs_task == 0) {
>  		task->security = NULL;
>  		return 0;
> @@ -677,6 +688,15 @@ static int lsm_task_alloc(struct task_struct *task)
>  	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>  	if (task->security == NULL)
>  		return -ENOMEM;
> +
> +	/*
> +	 * The start of the task blob contains the "interface" LSM slot number.
> +	 * Start with it set to the invalid slot number, indicating that the
> +	 * default first registered LSM be displayed.
> +	 */
> +	ilsm = task->security;
> +	*ilsm = LSMBLOB_INVALID;
> +
>  	return 0;
>  }
>  
> @@ -1732,14 +1752,26 @@ int security_file_open(struct file *file)
>  
>  int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>  {
> +	int *oilsm = current->security;
> +	int *nilsm;
>  	int rc = lsm_task_alloc(task);
>  
> -	if (rc)
> +	if (unlikely(rc))
>  		return rc;
> +
>  	rc = call_int_hook(task_alloc, 0, task, clone_flags);
> -	if (unlikely(rc))
> +	if (unlikely(rc)) {
>  		security_task_free(task);
> -	return rc;
> +		return rc;
> +	}
> +
> +	if (oilsm) {
> +		nilsm = task->security;
> +		if (nilsm)
> +			*nilsm = *oilsm;
> +	}
> +
> +	return 0;
>  }
>  
>  void security_task_free(struct task_struct *task)
> @@ -2171,23 +2203,110 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>  				char **value)
>  {
>  	struct security_hook_list *hp;
> +	int ilsm = lsm_task_ilsm(current);
> +	int slot = 0;
> +
> +	if (!strcmp(name, "interface_lsm")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0)
> +			return -EINVAL;
> +
> +		/*
> +		 * Only allow getting the current process' interface_lsm.
> +		 * There are too few reasons to get another process'
> +		 * interface_lsm and too many LSM policy issues.
> +		 */
> +		if (current != p)
> +			return -EINVAL;

... but context isn't established by just checking "current", as this
file handle may have been given to another process.

I suspect the security_get/setprocattr needs to gain a pointer to "file"
so that the f_cred struct can be examined[1] (i.e. compare opener
against reader/writer).

[1] https://www.kernel.org/doc/html/latest/security/credentials.html#open-file-credentials

> +
> +		ilsm = lsm_task_ilsm(p);
> +		if (ilsm != LSMBLOB_INVALID)
> +			slot = ilsm;
> +		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> +		if (*value)
> +			return strlen(*value);
> +		return -ENOMEM;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && ilsm != LSMBLOB_INVALID &&
> +		    ilsm != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.getprocattr(p, name, value);
>  	}
>  	return LSM_RET_DEFAULT(getprocattr);
>  }
>  
> +/**
> + * security_setprocattr - Set process attributes via /proc
> + * @lsm: name of module involved, or NULL
> + * @name: name of the attribute
> + * @value: value to set the attribute to
> + * @size: size of the value
> + *
> + * Set the process attribute for the specified security module
> + * to the specified value. Note that this can only be used to set
> + * the process attributes for the current, or "self" process.
> + * The /proc code has already done this check.
> + *
> + * Returns 0 on success, an appropriate code otherwise.
> + */
>  int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size)
>  {
>  	struct security_hook_list *hp;
> +	char *termed;
> +	char *copy;
> +	int *ilsm = current->security;
> +	int rc = -EINVAL;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "interface_lsm")) {
> +		/*
> +		 * Change the "interface_lsm" value only if all the security
> +		 * modules that support setting a procattr allow it.
> +		 * It is assumed that all such security modules will be
> +		 * cooperative.
> +		 */
> +		if (size == 0)
> +			return -EINVAL;
> +
> +		hlist_for_each_entry(hp, &security_hook_heads.setprocattr,
> +				     list) {
> +			rc = hp->hook.setprocattr(name, value, size);
> +			if (rc < 0)
> +				return rc;
> +		}

Similarly here -- how do the LSMs reason about who actually _opened_
this file?

-Kees

> +
> +		rc = -EINVAL;
> +
> +		copy = kmemdup_nul(value, size, GFP_KERNEL);
> +		if (copy == NULL)
> +			return -ENOMEM;
> +
> +		termed = strsep(&copy, " \n");
> +
> +		for (slot = 0; slot < lsm_slot; slot++)
> +			if (!strcmp(termed, lsm_slotlist[slot]->lsm)) {
> +				*ilsm = lsm_slotlist[slot]->slot;
> +				rc = size;
> +				break;
> +			}
> +
> +		kfree(termed);
> +		return rc;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && *ilsm != LSMBLOB_INVALID &&
> +		    *ilsm != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.setprocattr(name, value, size);
>  	}
>  	return LSM_RET_DEFAULT(setprocattr);
> @@ -2207,15 +2326,15 @@ EXPORT_SYMBOL(security_ismaclabel);
>  int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int ilsm = lsm_task_ilsm(current);
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>  			continue;
> -		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> -					      secdata, seclen);
> -		if (rc != LSM_RET_DEFAULT(secid_to_secctx))
> -			return rc;
> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
> +			return hp->hook.secid_to_secctx(
> +					blob->secid[hp->lsmid->slot],
> +					secdata, seclen);
>  	}
>  
>  	return LSM_RET_DEFAULT(secid_to_secctx);
> @@ -2226,16 +2345,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>  			     struct lsmblob *blob)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int ilsm = lsm_task_ilsm(current);
>  
>  	lsmblob_init(blob, 0);
>  	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>  			continue;
> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
> -					      &blob->secid[hp->lsmid->slot]);
> -		if (rc != 0)
> -			return rc;
> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
> +			return hp->hook.secctx_to_secid(secdata, seclen,
> +						&blob->secid[hp->lsmid->slot]);
>  	}
>  	return 0;
>  }
> @@ -2243,7 +2361,14 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>  
>  void security_release_secctx(char *secdata, u32 seclen)
>  {
> -	call_void_hook(release_secctx, secdata, seclen);
> +	struct security_hook_list *hp;
> +	int ilsm = lsm_task_ilsm(current);
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot) {
> +			hp->hook.release_secctx(secdata, seclen);
> +			return;
> +		}
>  }
>  EXPORT_SYMBOL(security_release_secctx);
>  
> @@ -2384,8 +2509,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>  				      int __user *optlen, unsigned len)
>  {
> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> -				optval, optlen, len);
> +	int ilsm = lsm_task_ilsm(current);
> +	struct security_hook_list *hp;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> +			     list)
> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
> +			return hp->hook.socket_getpeersec_stream(sock, optval,
> +								 optlen, len);
> +	return -ENOPROTOOPT;
>  }
>  
>  int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0133b142e938..dba867721336 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6510,6 +6510,17 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>  	/*
>  	 * Basic control over ability to set these attributes at all.
>  	 */
> +
> +	/*
> +	 * For setting interface_lsm, we only perform a permission check;
> +	 * the actual update to the interface_lsm value is handled by the
> +	 * LSM framework.
> +	 */
> +	if (!strcmp(name, "interface_lsm"))
> +		return avc_has_perm(&selinux_state,
> +				    mysid, mysid, SECCLASS_PROCESS2,
> +				    PROCESS2__SETDISPLAY, NULL);
> +
>  	if (!strcmp(name, "exec"))
>  		error = avc_has_perm(&selinux_state,
>  				     mysid, mysid, SECCLASS_PROCESS,
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 62d19bccf3de..8f4b0dd6dd78 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -53,7 +53,7 @@ struct security_class_mapping secclass_map[] = {
>  	    "execmem", "execstack", "execheap", "setkeycreate",
>  	    "setsockcreate", "getrlimit", NULL } },
>  	{ "process2",
> -	  { "nnp_transition", "nosuid_transition", NULL } },
> +	  { "nnp_transition", "nosuid_transition", "setdisplay", NULL } },
>  	{ "system",
>  	  { "ipc_info", "syslog_read", "syslog_mod",
>  	    "syslog_console", "module_request", "module_load", NULL } },
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 5c10ad27be37..7aa7ea38f627 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3508,6 +3508,13 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
>  	struct smack_known_list_elem *sklep;
>  	int rc;
>  
> +	/*
> +	 * Allow the /proc/.../attr/current and SO_PEERSEC "interface_lsm"
> +	 * to be reset at will.
> +	 */
> +	if (strcmp(name, "interface_lsm") == 0)
> +		return 0;
> +
>  	if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
>  		return -EPERM;
>  
> -- 
> 2.29.2
> 

-- 
Kees Cook

Re: [PATCH v26 14/25] LSM: Specify which LSM to display

[Casey Schaufler]

On 5/14/2021 12:23 PM, Kees Cook wrote:
> On Thu, May 13, 2021 at 01:07:56PM -0700, Casey Schaufler wrote:
>> Create a new entry "interface_lsm" in the procfs attr directory for
>> controlling which LSM security information is displayed for a
>> process. A process can only read or write its own display value.
>>
>> The name of an active LSM that supplies hooks for
>> human readable data may be written to "interface_lsm" to set the
>> value. The name of the LSM currently in use can be read from
>> "interface_lsm". At this point there can only be one LSM capable
>> of display active. A helper function lsm_task_ilsm() is
>> provided to get the interface lsm slot for a task_struct.
>>
>> Setting the "interface_lsm" requires that all security modules using
>> setprocattr hooks allow the action. Each security module is
>> responsible for defining its policy.
>>
>> AppArmor hook provided by John Johansen <john.johansen@canonical.com>
>> SELinux hook provided by Stephen Smalley <stephen.smalley.work@gmail.com>
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Cc: John Johansen <john.johansen@canonical.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-api@vger.kernel.org
>> Cc: linux-doc@vger.kernel.org
>> ---
>>  .../ABI/testing/procfs-attr-lsm_display       |  22 +++
>>  Documentation/security/lsm.rst                |  14 ++
>>  fs/proc/base.c                                |   1 +
>>  include/linux/lsm_hooks.h                     |  17 ++
>>  security/apparmor/include/apparmor.h          |   3 +-
>>  security/apparmor/lsm.c                       |  32 ++++
>>  security/security.c                           | 166 ++++++++++++++++--
>>  security/selinux/hooks.c                      |  11 ++
>>  security/selinux/include/classmap.h           |   2 +-
>>  security/smack/smack_lsm.c                    |   7 +
>>  10 files changed, 256 insertions(+), 19 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/procfs-attr-lsm_display
>>
>> diff --git a/Documentation/ABI/testing/procfs-attr-lsm_display b/Documentation/ABI/testing/procfs-attr-lsm_display
>> new file mode 100644
>> index 000000000000..0f60005c235c
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/procfs-attr-lsm_display
>> @@ -0,0 +1,22 @@
>> +What:		/proc/*/attr/lsm_display
>> +Contact:	linux-security-module@vger.kernel.org,
>> +Description:	The name of the Linux security module (LSM) that will
>> +		provide information in the /proc/*/attr/current,
>> +		/proc/*/attr/prev and /proc/*/attr/exec interfaces.
>> +		The details of permissions required to read from
>> +		this interface are dependent on the LSMs active on the
>> +		system.
>> +		A process cannot write to this interface unless it
>> +		refers to itself.
>> +		The other details of permissions required to write to
>> +		this interface are dependent on the LSMs active on the
>> +		system.
>> +		The format of the data used by this interface is a
>> +		text string identifying the name of an LSM. The values
>> +		accepted are:
>> +			selinux		- the SELinux LSM
>> +			smack		- the Smack LSM
>> +			apparmor	- The AppArmor LSM
>> +		By convention the LSM names are lower case and do not
>> +		contain special characters.
>> +Users:		LSM user-space
>> diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst
>> index 6a2a2e973080..b77b4a540391 100644
>> --- a/Documentation/security/lsm.rst
>> +++ b/Documentation/security/lsm.rst
>> @@ -129,3 +129,17 @@ to identify it as the first security module to be registered.
>>  The capabilities security module does not use the general security
>>  blobs, unlike other modules. The reasons are historical and are
>>  based on overhead, complexity and performance concerns.
>> +
>> +LSM External Interfaces
>> +=======================
>> +
>> +The LSM infrastructure does not generally provide external interfaces.
>> +The individual security modules provide what external interfaces they
>> +require.
>> +
>> +The file ``/sys/kernel/security/lsm`` provides a comma
>> +separated list of the active security modules.
>> +
>> +The file ``/proc/pid/attr/interface_lsm`` contains the name of the security
>> +module for which the ``/proc/pid/attr/current`` interface will
>> +apply. This interface can be written to.
>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>> index 3851bfcdba56..10de522f3112 100644
>> --- a/fs/proc/base.c
>> +++ b/fs/proc/base.c
>> @@ -2807,6 +2807,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>>  	ATTR(NULL, "fscreate",		0666),
>>  	ATTR(NULL, "keycreate",		0666),
>>  	ATTR(NULL, "sockcreate",	0666),
>> +	ATTR(NULL, "interface_lsm",	0666),
> Anyone can open this file... (continued below)
>
>>  #ifdef CONFIG_SECURITY_SMACK
>>  	DIR("smack",			0555,
>>  	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index c61a16f0a5bc..d2c4bc94d47f 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1686,4 +1686,21 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
>>  
>>  extern int lsm_inode_alloc(struct inode *inode);
>>  
>> +/**
>> + * lsm_task_ilsm - the "interface_lsm" for this task
>> + * @task: The task to report on
>> + *
>> + * Returns the task's interface LSM slot.
>> + */
>> +static inline int lsm_task_ilsm(struct task_struct *task)
>> +{
>> +#ifdef CONFIG_SECURITY
>> +	int *ilsm = task->security;
>> +
>> +	if (ilsm)
>> +		return *ilsm;
>> +#endif
>> +	return LSMBLOB_INVALID;
>> +}
>> +
>>  #endif /* ! __LINUX_LSM_HOOKS_H */
>> diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
>> index 1fbabdb565a8..b1622fcb4394 100644
>> --- a/security/apparmor/include/apparmor.h
>> +++ b/security/apparmor/include/apparmor.h
>> @@ -28,8 +28,9 @@
>>  #define AA_CLASS_SIGNAL		10
>>  #define AA_CLASS_NET		14
>>  #define AA_CLASS_LABEL		16
>> +#define AA_CLASS_DISPLAY_LSM	17
>>  
>> -#define AA_CLASS_LAST		AA_CLASS_LABEL
>> +#define AA_CLASS_LAST		AA_CLASS_DISPLAY_LSM
>>  
>>  /* Control parameters settable through module/boot flags */
>>  extern enum audit_mode aa_g_audit;
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 392e25940d1f..4237536106aa 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -621,6 +621,25 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
>>  	return error;
>>  }
>>  
>> +
>> +static int profile_interface_lsm(struct aa_profile *profile,
>> +				 struct common_audit_data *sa)
>> +{
>> +	struct aa_perms perms = { };
>> +	unsigned int state;
>> +
>> +	state = PROFILE_MEDIATES(profile, AA_CLASS_DISPLAY_LSM);
>> +	if (state) {
>> +		aa_compute_perms(profile->policy.dfa, state, &perms);
>> +		aa_apply_modes_to_perms(profile, &perms);
>> +		aad(sa)->label = &profile->label;
>> +
>> +		return aa_check_perms(profile, &perms, AA_MAY_WRITE, sa, NULL);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int apparmor_setprocattr(const char *name, void *value,
>>  				size_t size)
>>  {
>> @@ -632,6 +651,19 @@ static int apparmor_setprocattr(const char *name, void *value,
>>  	if (size == 0)
>>  		return -EINVAL;
>>  
>> +	/* LSM infrastructure does actual setting of interface_lsm if allowed */
>> +	if (!strcmp(name, "interface_lsm")) {
>> +		struct aa_profile *profile;
>> +		struct aa_label *label;
>> +
>> +		aad(&sa)->info = "set interface lsm";
>> +		label = begin_current_label_crit_section();
>> +		error = fn_for_each_confined(label, profile,
>> +					profile_interface_lsm(profile, &sa));
>> +		end_current_label_crit_section(label);
>> +		return error;
>> +	}
>> +
>>  	/* AppArmor requires that the buffer must be null terminated atm */
>>  	if (args[size - 1] != '\0') {
>>  		/* null terminate */
>> diff --git a/security/security.c b/security/security.c
>> index f5407a85641e..1ce125c01782 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -77,7 +77,16 @@ static struct kmem_cache *lsm_file_cache;
>>  static struct kmem_cache *lsm_inode_cache;
>>  
>>  char *lsm_names;
>> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
>> +
>> +/*
>> + * The task blob includes the "interface_lsm" slot used for
>> + * chosing which module presents contexts.
>> + * Using a long to avoid potential alignment issues with
>> + * module assigned task blobs.
>> + */
>> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
>> +	.lbs_task = sizeof(long),
>> +};
>>  
>>  /* Boot-time LSM user choice */
>>  static __initdata const char *chosen_lsm_order;
>> @@ -669,6 +678,8 @@ int lsm_inode_alloc(struct inode *inode)
>>   */
>>  static int lsm_task_alloc(struct task_struct *task)
>>  {
>> +	int *ilsm;
>> +
>>  	if (blob_sizes.lbs_task == 0) {
>>  		task->security = NULL;
>>  		return 0;
>> @@ -677,6 +688,15 @@ static int lsm_task_alloc(struct task_struct *task)
>>  	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>>  	if (task->security == NULL)
>>  		return -ENOMEM;
>> +
>> +	/*
>> +	 * The start of the task blob contains the "interface" LSM slot number.
>> +	 * Start with it set to the invalid slot number, indicating that the
>> +	 * default first registered LSM be displayed.
>> +	 */
>> +	ilsm = task->security;
>> +	*ilsm = LSMBLOB_INVALID;
>> +
>>  	return 0;
>>  }
>>  
>> @@ -1732,14 +1752,26 @@ int security_file_open(struct file *file)
>>  
>>  int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>>  {
>> +	int *oilsm = current->security;
>> +	int *nilsm;
>>  	int rc = lsm_task_alloc(task);
>>  
>> -	if (rc)
>> +	if (unlikely(rc))
>>  		return rc;
>> +
>>  	rc = call_int_hook(task_alloc, 0, task, clone_flags);
>> -	if (unlikely(rc))
>> +	if (unlikely(rc)) {
>>  		security_task_free(task);
>> -	return rc;
>> +		return rc;
>> +	}
>> +
>> +	if (oilsm) {
>> +		nilsm = task->security;
>> +		if (nilsm)
>> +			*nilsm = *oilsm;
>> +	}
>> +
>> +	return 0;
>>  }
>>  
>>  void security_task_free(struct task_struct *task)
>> @@ -2171,23 +2203,110 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>>  				char **value)
>>  {
>>  	struct security_hook_list *hp;
>> +	int ilsm = lsm_task_ilsm(current);
>> +	int slot = 0;
>> +
>> +	if (!strcmp(name, "interface_lsm")) {
>> +		/*
>> +		 * lsm_slot will be 0 if there are no displaying modules.
>> +		 */
>> +		if (lsm_slot == 0)
>> +			return -EINVAL;
>> +
>> +		/*
>> +		 * Only allow getting the current process' interface_lsm.
>> +		 * There are too few reasons to get another process'
>> +		 * interface_lsm and too many LSM policy issues.
>> +		 */
>> +		if (current != p)
>> +			return -EINVAL;
> ... but context isn't established by just checking "current", as this
> file handle may have been given to another process.
>
> I suspect the security_get/setprocattr needs to gain a pointer to "file"
> so that the f_cred struct can be examined[1] (i.e. compare opener
> against reader/writer).
>
> [1] https://www.kernel.org/doc/html/latest/security/credentials.html#open-file-credentials

It's not credentials being checked here. The check is whether the task that
would be affected is "current". Process A can't open /proc/B/attr/interface_lsm
with write access. The only process that can open it for write access is B.
If process B opens /proc/B/attr/interface_lsm for write access it could send
the file handle to process A, but process A can't write to the file because
(current != p) that is, (A != B).

>> +
>> +		ilsm = lsm_task_ilsm(p);
>> +		if (ilsm != LSMBLOB_INVALID)
>> +			slot = ilsm;
>> +		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
>> +		if (*value)
>> +			return strlen(*value);
>> +		return -ENOMEM;
>> +	}
>>  
>>  	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>>  			continue;
>> +		if (lsm == NULL && ilsm != LSMBLOB_INVALID &&
>> +		    ilsm != hp->lsmid->slot)
>> +			continue;
>>  		return hp->hook.getprocattr(p, name, value);
>>  	}
>>  	return LSM_RET_DEFAULT(getprocattr);
>>  }
>>  
>> +/**
>> + * security_setprocattr - Set process attributes via /proc
>> + * @lsm: name of module involved, or NULL
>> + * @name: name of the attribute
>> + * @value: value to set the attribute to
>> + * @size: size of the value
>> + *
>> + * Set the process attribute for the specified security module
>> + * to the specified value. Note that this can only be used to set
>> + * the process attributes for the current, or "self" process.
>> + * The /proc code has already done this check.
>> + *
>> + * Returns 0 on success, an appropriate code otherwise.
>> + */
>>  int security_setprocattr(const char *lsm, const char *name, void *value,
>>  			 size_t size)
>>  {
>>  	struct security_hook_list *hp;
>> +	char *termed;
>> +	char *copy;
>> +	int *ilsm = current->security;
>> +	int rc = -EINVAL;
>> +	int slot = 0;
>> +
>> +	if (!strcmp(name, "interface_lsm")) {
>> +		/*
>> +		 * Change the "interface_lsm" value only if all the security
>> +		 * modules that support setting a procattr allow it.
>> +		 * It is assumed that all such security modules will be
>> +		 * cooperative.
>> +		 */
>> +		if (size == 0)
>> +			return -EINVAL;
>> +
>> +		hlist_for_each_entry(hp, &security_hook_heads.setprocattr,
>> +				     list) {
>> +			rc = hp->hook.setprocattr(name, value, size);
>> +			if (rc < 0)
>> +				return rc;
>> +		}
> Similarly here -- how do the LSMs reason about who actually _opened_
> this file?
>
> -Kees
>
>> +
>> +		rc = -EINVAL;
>> +
>> +		copy = kmemdup_nul(value, size, GFP_KERNEL);
>> +		if (copy == NULL)
>> +			return -ENOMEM;
>> +
>> +		termed = strsep(&copy, " \n");
>> +
>> +		for (slot = 0; slot < lsm_slot; slot++)
>> +			if (!strcmp(termed, lsm_slotlist[slot]->lsm)) {
>> +				*ilsm = lsm_slotlist[slot]->slot;
>> +				rc = size;
>> +				break;
>> +			}
>> +
>> +		kfree(termed);
>> +		return rc;
>> +	}
>>  
>>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>>  			continue;
>> +		if (lsm == NULL && *ilsm != LSMBLOB_INVALID &&
>> +		    *ilsm != hp->lsmid->slot)
>> +			continue;
>>  		return hp->hook.setprocattr(name, value, size);
>>  	}
>>  	return LSM_RET_DEFAULT(setprocattr);
>> @@ -2207,15 +2326,15 @@ EXPORT_SYMBOL(security_ismaclabel);
>>  int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>>  {
>>  	struct security_hook_list *hp;
>> -	int rc;
>> +	int ilsm = lsm_task_ilsm(current);
>>  
>>  	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>>  			continue;
>> -		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
>> -					      secdata, seclen);
>> -		if (rc != LSM_RET_DEFAULT(secid_to_secctx))
>> -			return rc;
>> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
>> +			return hp->hook.secid_to_secctx(
>> +					blob->secid[hp->lsmid->slot],
>> +					secdata, seclen);
>>  	}
>>  
>>  	return LSM_RET_DEFAULT(secid_to_secctx);
>> @@ -2226,16 +2345,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>>  			     struct lsmblob *blob)
>>  {
>>  	struct security_hook_list *hp;
>> -	int rc;
>> +	int ilsm = lsm_task_ilsm(current);
>>  
>>  	lsmblob_init(blob, 0);
>>  	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>>  			continue;
>> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
>> -					      &blob->secid[hp->lsmid->slot]);
>> -		if (rc != 0)
>> -			return rc;
>> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
>> +			return hp->hook.secctx_to_secid(secdata, seclen,
>> +						&blob->secid[hp->lsmid->slot]);
>>  	}
>>  	return 0;
>>  }
>> @@ -2243,7 +2361,14 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>>  
>>  void security_release_secctx(char *secdata, u32 seclen)
>>  {
>> -	call_void_hook(release_secctx, secdata, seclen);
>> +	struct security_hook_list *hp;
>> +	int ilsm = lsm_task_ilsm(current);
>> +
>> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
>> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot) {
>> +			hp->hook.release_secctx(secdata, seclen);
>> +			return;
>> +		}
>>  }
>>  EXPORT_SYMBOL(security_release_secctx);
>>  
>> @@ -2384,8 +2509,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>>  				      int __user *optlen, unsigned len)
>>  {
>> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
>> -				optval, optlen, len);
>> +	int ilsm = lsm_task_ilsm(current);
>> +	struct security_hook_list *hp;
>> +
>> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
>> +			     list)
>> +		if (ilsm == LSMBLOB_INVALID || ilsm == hp->lsmid->slot)
>> +			return hp->hook.socket_getpeersec_stream(sock, optval,
>> +								 optlen, len);
>> +	return -ENOPROTOOPT;
>>  }
>>  
>>  int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 0133b142e938..dba867721336 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6510,6 +6510,17 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>>  	/*
>>  	 * Basic control over ability to set these attributes at all.
>>  	 */
>> +
>> +	/*
>> +	 * For setting interface_lsm, we only perform a permission check;
>> +	 * the actual update to the interface_lsm value is handled by the
>> +	 * LSM framework.
>> +	 */
>> +	if (!strcmp(name, "interface_lsm"))
>> +		return avc_has_perm(&selinux_state,
>> +				    mysid, mysid, SECCLASS_PROCESS2,
>> +				    PROCESS2__SETDISPLAY, NULL);
>> +
>>  	if (!strcmp(name, "exec"))
>>  		error = avc_has_perm(&selinux_state,
>>  				     mysid, mysid, SECCLASS_PROCESS,
>> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
>> index 62d19bccf3de..8f4b0dd6dd78 100644
>> --- a/security/selinux/include/classmap.h
>> +++ b/security/selinux/include/classmap.h
>> @@ -53,7 +53,7 @@ struct security_class_mapping secclass_map[] = {
>>  	    "execmem", "execstack", "execheap", "setkeycreate",
>>  	    "setsockcreate", "getrlimit", NULL } },
>>  	{ "process2",
>> -	  { "nnp_transition", "nosuid_transition", NULL } },
>> +	  { "nnp_transition", "nosuid_transition", "setdisplay", NULL } },
>>  	{ "system",
>>  	  { "ipc_info", "syslog_read", "syslog_mod",
>>  	    "syslog_console", "module_request", "module_load", NULL } },
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 5c10ad27be37..7aa7ea38f627 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -3508,6 +3508,13 @@ static int smack_setprocattr(const char *name, void *value, size_t size)
>>  	struct smack_known_list_elem *sklep;
>>  	int rc;
>>  
>> +	/*
>> +	 * Allow the /proc/.../attr/current and SO_PEERSEC "interface_lsm"
>> +	 * to be reset at will.
>> +	 */
>> +	if (strcmp(name, "interface_lsm") == 0)
>> +		return 0;
>> +
>>  	if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
>>  		return -EPERM;
>>  
>> -- 
>> 2.29.2
>>

Re: [PATCH v26 14/25] LSM: Specify which LSM to display

[Paul Moore]

On Mon, May 17, 2021 at 3:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 5/14/2021 12:23 PM, Kees Cook wrote:
> > On Thu, May 13, 2021 at 01:07:56PM -0700, Casey Schaufler wrote:
> >> Create a new entry "interface_lsm" in the procfs attr directory for
> >> controlling which LSM security information is displayed for a
> >> process. A process can only read or write its own display value.
> >>
> >> The name of an active LSM that supplies hooks for
> >> human readable data may be written to "interface_lsm" to set the
> >> value. The name of the LSM currently in use can be read from
> >> "interface_lsm". At this point there can only be one LSM capable
> >> of display active. A helper function lsm_task_ilsm() is
> >> provided to get the interface lsm slot for a task_struct.
> >>
> >> Setting the "interface_lsm" requires that all security modules using
> >> setprocattr hooks allow the action. Each security module is
> >> responsible for defining its policy.
> >>
> >> AppArmor hook provided by John Johansen <john.johansen@canonical.com>
> >> SELinux hook provided by Stephen Smalley <stephen.smalley.work@gmail.com>
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> Cc: Kees Cook <keescook@chromium.org>
> >> Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
> >> Cc: Paul Moore <paul@paul-moore.com>
> >> Cc: John Johansen <john.johansen@canonical.com>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Cc: linux-api@vger.kernel.org
> >> Cc: linux-doc@vger.kernel.org
> >> ---
> >>  .../ABI/testing/procfs-attr-lsm_display       |  22 +++
> >>  Documentation/security/lsm.rst                |  14 ++
> >>  fs/proc/base.c                                |   1 +
> >>  include/linux/lsm_hooks.h                     |  17 ++
> >>  security/apparmor/include/apparmor.h          |   3 +-
> >>  security/apparmor/lsm.c                       |  32 ++++
> >>  security/security.c                           | 166 ++++++++++++++++--
> >>  security/selinux/hooks.c                      |  11 ++
> >>  security/selinux/include/classmap.h           |   2 +-
> >>  security/smack/smack_lsm.c                    |   7 +
> >>  10 files changed, 256 insertions(+), 19 deletions(-)
> >>  create mode 100644 Documentation/ABI/testing/procfs-attr-lsm_display

...

> >> @@ -2171,23 +2203,110 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> >>                              char **value)
> >>  {
> >>      struct security_hook_list *hp;
> >> +    int ilsm = lsm_task_ilsm(current);
> >> +    int slot = 0;
> >> +
> >> +    if (!strcmp(name, "interface_lsm")) {
> >> +            /*
> >> +             * lsm_slot will be 0 if there are no displaying modules.
> >> +             */
> >> +            if (lsm_slot == 0)
> >> +                    return -EINVAL;
> >> +
> >> +            /*
> >> +             * Only allow getting the current process' interface_lsm.
> >> +             * There are too few reasons to get another process'
> >> +             * interface_lsm and too many LSM policy issues.
> >> +             */
> >> +            if (current != p)
> >> +                    return -EINVAL;
> > ... but context isn't established by just checking "current", as this
> > file handle may have been given to another process.
> >
> > I suspect the security_get/setprocattr needs to gain a pointer to "file"
> > so that the f_cred struct can be examined[1] (i.e. compare opener
> > against reader/writer).
> >
> > [1] https://www.kernel.org/doc/html/latest/security/credentials.html#open-file-credentials
>
> It's not credentials being checked here. The check is whether the task that
> would be affected is "current". Process A can't open /proc/B/attr/interface_lsm
> with write access. The only process that can open it for write access is B.
> If process B opens /proc/B/attr/interface_lsm for write access it could send
> the file handle to process A, but process A can't write to the file because
> (current != p) that is, (A != B).

Agreed.

Acked-by: Paul Moore <paul@paul-moore.com>


--
paul moore
www.paul-moore.com