[PATCH v4 0/3] security: Always enable integrity LSM

[PATCH v4 0/3] security: Always enable integrity LSM

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Since the integrity (including IMA and EVM) functions are currently always
called by the LSM infrastructure, and always after all LSMs, formalize
these requirements by introducing a new LSM ordering called LSM_ORDER_LAST,
and set it for the 'integrity' LSM (patch 1).

Consequently, revert commit 92063f3ca73a ("integrity: double check
iint_cache was initialized"), as the double check becomes always verified
(patch 2), and remove 'integrity' from the list of LSMs in
security/Kconfig (patch 3).

Changelog

v3:
- Remove Signed-off-by tag by Mimi (suggested by Paul)
- Clarify that an LSM with order LSM_ORDER_FIRST or LSM_ORDER_LAST is
  always enabled if it is selected in the kernel configuration (suggested
  by Paul)

v2:
- Fix commit message in patch 1 (suggested by Mimi)
- Bump version of patch 2 (v1 -> v3) to make one patch set
- Add patch 3 (suggested by Mimi)

v1:
- Add comment for LSM_ORDER_LAST definition (suggested by Mimi)
- Add Fixes tag (suggested by Mimi)
- Do minor corrections in the commit messages (suggested by Mimi and
  Stefan)

Roberto Sassu (3):
  security: Introduce LSM_ORDER_LAST and set it for the integrity LSM
  Revert "integrity: double check iint_cache was initialized"
  security: Remove integrity from the LSM list in Kconfig

 include/linux/lsm_hooks.h |  1 +
 security/Kconfig          | 16 +++++++++-------
 security/integrity/iint.c |  9 +--------
 security/security.c       | 12 +++++++++---
 4 files changed, 20 insertions(+), 18 deletions(-)

-- 
2.25.1

[PATCH v4 1/3] security: Introduce LSM_ORDER_LAST and set it for the integrity LSM

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce LSM_ORDER_LAST, to satisfy the requirement of LSMs needing to be
last, e.g. the 'integrity' LSM, without changing the kernel command line or
configuration.

Also, set this order for the 'integrity' LSM. While not enforced, this is
the only LSM expected to use it.

Similarly to LSM_ORDER_FIRST, LSMs with LSM_ORDER_LAST are always enabled
and put at the end of the LSM list, if selected in the kernel
configuration. Setting one of these orders alone, does not cause the LSMs
to be selected and compiled built-in in the kernel.

Finally, for LSM_ORDER_MUTABLE LSMs, set the found variable to true if an
LSM is found, regardless of its order. In this way, the kernel would not
wrongly report that the LSM is not built-in in the kernel if its order is
LSM_ORDER_LAST.

Fixes: 79f7865d844c ("LSM: Introduce "lsm=" for boottime LSM selection")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/lsm_hooks.h |  1 +
 security/integrity/iint.c |  1 +
 security/security.c       | 12 +++++++++---
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 6e156d2acff..c55761d93a2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1716,6 +1716,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,
 enum lsm_order {
 	LSM_ORDER_FIRST = -1,	/* This is only for capabilities. */
 	LSM_ORDER_MUTABLE = 0,
+	LSM_ORDER_LAST = 1,	/* This is only for integrity. */
 };
 
 struct lsm_info {
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 8638976f799..b97eb59e0e3 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -182,6 +182,7 @@ static int __init integrity_iintcache_init(void)
 DEFINE_LSM(integrity) = {
 	.name = "integrity",
 	.init = integrity_iintcache_init,
+	.order = LSM_ORDER_LAST,
 };
 
 
diff --git a/security/security.c b/security/security.c
index cf6cc576736..2f36229d5b6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -284,9 +284,9 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
 		bool found = false;
 
 		for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
-			if (lsm->order == LSM_ORDER_MUTABLE &&
-			    strcmp(lsm->name, name) == 0) {
-				append_ordered_lsm(lsm, origin);
+			if (strcmp(lsm->name, name) == 0) {
+				if (lsm->order == LSM_ORDER_MUTABLE)
+					append_ordered_lsm(lsm, origin);
 				found = true;
 			}
 		}
@@ -306,6 +306,12 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
 		}
 	}
 
+	/* LSM_ORDER_LAST is always last. */
+	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+		if (lsm->order == LSM_ORDER_LAST)
+			append_ordered_lsm(lsm, "   last");
+	}
+
 	/* Disable all LSMs not in the ordered list. */
 	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
 		if (exists_ordered_lsm(lsm))
-- 
2.25.1

[PATCH v4 2/3] Revert "integrity: double check iint_cache was initialized"

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

With the recent introduction of LSM_ORDER_LAST, the 'integrity' LSM is
always initialized (if selected in the kernel configuration) and the
iint_cache is always created (the kernel panics on error). Thus, the
additional check of iint_cache in integrity_inode_get() is no longer
necessary. If the 'integrity' LSM is not selected in the kernel
configuration, integrity_inode_get() just returns NULL.

This reverts commit 92063f3ca73aab794bd5408d3361fd5b5ea33079.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/iint.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index b97eb59e0e3..c73858e8c6d 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -98,14 +98,6 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
 	struct rb_node *node, *parent = NULL;
 	struct integrity_iint_cache *iint, *test_iint;
 
-	/*
-	 * The integrity's "iint_cache" is initialized at security_init(),
-	 * unless it is not included in the ordered list of LSMs enabled
-	 * on the boot command line.
-	 */
-	if (!iint_cache)
-		panic("%s: lsm=integrity required.\n", __func__);
-
 	iint = integrity_iint_find(inode);
 	if (iint)
 		return iint;
-- 
2.25.1

[PATCH v4 3/3] security: Remove integrity from the LSM list in Kconfig

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Remove 'integrity' from the list of LSMs in Kconfig, as it is no longer
necessary. Since the recent change (set order to LSM_ORDER_LAST), the
'integrity' LSM is always enabled (if selected in the kernel
configuration).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/Kconfig | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index e6db09a779b..1699dda6821 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -246,15 +246,17 @@ endchoice
 
 config LSM
 	string "Ordered list of enabled LSMs"
-	default "landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
-	default "landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
-	default "landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
-	default "landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
-	default "landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
+	default "landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
+	default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
+	default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
+	default "landlock,lockdown,yama,loadpin,safesetid,bpf" if DEFAULT_SECURITY_DAC
+	default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf"
 	help
 	  A comma-separated list of LSMs, in initialization order.
-	  Any LSMs left off this list will be ignored. This can be
-	  controlled at boot with the "lsm=" parameter.
+	  Any LSMs left off this list, except for those with order
+	  LSM_ORDER_FIRST and LSM_ORDER_LAST, which are always enabled
+	  if selected in the kernel configuration, will be ignored.
+	  This can be controlled at boot with the "lsm=" parameter.
 
 	  If unsure, leave this as the default.
 
-- 
2.25.1

Re: [PATCH v4 1/3] security: Introduce LSM_ORDER_LAST and set it for the integrity LSM

[Mimi Zohar]

On Fri, 2023-03-10 at 09:53 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Introduce LSM_ORDER_LAST, to satisfy the requirement of LSMs needing to be
> last, e.g. the 'integrity' LSM, without changing the kernel command line or
> configuration.
> 
> Also, set this order for the 'integrity' LSM. While not enforced, this is
> the only LSM expected to use it.
> 
> Similarly to LSM_ORDER_FIRST, LSMs with LSM_ORDER_LAST are always enabled
> and put at the end of the LSM list, if selected in the kernel
> configuration. Setting one of these orders alone, does not cause the LSMs
> to be selected and compiled built-in in the kernel.
> 
> Finally, for LSM_ORDER_MUTABLE LSMs, set the found variable to true if an
> LSM is found, regardless of its order. In this way, the kernel would not
> wrongly report that the LSM is not built-in in the kernel if its order is
> LSM_ORDER_LAST.
> 
> Fixes: 79f7865d844c ("LSM: Introduce "lsm=" for boottime LSM selection")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>

Re: [PATCH v4 2/3] Revert "integrity: double check iint_cache was initialized"

[Mimi Zohar]

On Fri, 2023-03-10 at 09:54 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> With the recent introduction of LSM_ORDER_LAST, the 'integrity' LSM is
> always initialized (if selected in the kernel configuration) and the
> iint_cache is always created (the kernel panics on error). Thus, the
> additional check of iint_cache in integrity_inode_get() is no longer
> necessary. If the 'integrity' LSM is not selected in the kernel
> configuration, integrity_inode_get() just returns NULL.
> 
> This reverts commit 92063f3ca73aab794bd5408d3361fd5b5ea33079.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>

Re: [PATCH v4 3/3] security: Remove integrity from the LSM list in Kconfig

[Mimi Zohar]

On Fri, 2023-03-10 at 09:54 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Remove 'integrity' from the list of LSMs in Kconfig, as it is no longer
> necessary. Since the recent change (set order to LSM_ORDER_LAST), the
> 'integrity' LSM is always enabled (if selected in the kernel
> configuration).
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>b

Re: [PATCH v4 0/3] security: Always enable integrity LSM

[Paul Moore]

On Fri, Mar 10, 2023 at 3:54 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Since the integrity (including IMA and EVM) functions are currently always
> called by the LSM infrastructure, and always after all LSMs, formalize
> these requirements by introducing a new LSM ordering called LSM_ORDER_LAST,
> and set it for the 'integrity' LSM (patch 1).
>
> Consequently, revert commit 92063f3ca73a ("integrity: double check
> iint_cache was initialized"), as the double check becomes always verified
> (patch 2), and remove 'integrity' from the list of LSMs in
> security/Kconfig (patch 3).
>
> Changelog
>
> v3:
> - Remove Signed-off-by tag by Mimi (suggested by Paul)
> - Clarify that an LSM with order LSM_ORDER_FIRST or LSM_ORDER_LAST is
>   always enabled if it is selected in the kernel configuration (suggested
>   by Paul)
>
> v2:
> - Fix commit message in patch 1 (suggested by Mimi)
> - Bump version of patch 2 (v1 -> v3) to make one patch set
> - Add patch 3 (suggested by Mimi)
>
> v1:
> - Add comment for LSM_ORDER_LAST definition (suggested by Mimi)
> - Add Fixes tag (suggested by Mimi)
> - Do minor corrections in the commit messages (suggested by Mimi and
>   Stefan)
>
> Roberto Sassu (3):
>   security: Introduce LSM_ORDER_LAST and set it for the integrity LSM
>   Revert "integrity: double check iint_cache was initialized"
>   security: Remove integrity from the LSM list in Kconfig
>
>  include/linux/lsm_hooks.h |  1 +
>  security/Kconfig          | 16 +++++++++-------
>  security/integrity/iint.c |  9 +--------
>  security/security.c       | 12 +++++++++---
>  4 files changed, 20 insertions(+), 18 deletions(-)

I just merged the full patchset into the lsm/next branch, thanks everyone!

-- 
paul-moore.com