Re: [PATCH 1/9] integrity: Prepare for having "ima" and "evm" available in "integrity" LSM

Mickaël Salaün
2022-10-14T14:40:18Z

On 14/10/2022 00:36, Kees Cook wrote:
> Move "integrity" LSM to the end of the Kconfig list and prepare for
> having ima and evm LSM initialization called from the top-level
> "integrity" LSM.
>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: "Mickaël Salaün" <mic@digikod.net>
> Cc: linux-security-module@vger.kernel.org
> Cc: linux-integrity@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> security/Kconfig | 10 +++++-----
> security/integrity/evm/evm_main.c | 4 ++++
> security/integrity/iint.c | 17 +++++++++++++----
> security/integrity/ima/ima_main.c | 4 ++++
> security/integrity/integrity.h | 6 ++++++
> 5 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index e6db09a779b7..d472e87a2fc4 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -246,11 +246,11 @@ 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,integrity" if DEFAULT_SECURITY_SMACK
> + default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf,integrity" if DEFAULT_SECURITY_APPARMOR
> + default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf,integrity" if DEFAULT_SECURITY_TOMOYO
> + default "landlock,lockdown,yama,loadpin,safesetid,bpf,integrity" if DEFAULT_SECURITY_DAC
> + default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf,integrity"

This is not backward compatible, but can easily be fixed thanks to
DEFINE_LSM().order

Side node: I proposed an alternative to that but it was Nacked:
https://lore.kernel.org/all/20210222150608.808146-1-mic@digikod.net/

> help
> A comma-separated list of LSMs, in initialization order.
> Any LSMs left off this list will be ignored. This can be
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 2e6fb6e2ffd2..1ef965089417 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -904,3 +904,7 @@ static int __init init_evm(void)
> }
>
> late_initcall(init_evm);
> +
> +void __init integrity_lsm_evm_init(void)
> +{
> +}
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 8638976f7990..4f322324449d 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -18,7 +18,6 @@
> #include <linux/file.h>
> #include <linux/uaccess.h>
> #include <linux/security.h>
> -#include <linux/lsm_hooks.h>
> #include "integrity.h"
>
> static struct rb_root integrity_iint_tree = RB_ROOT;
> @@ -172,19 +171,29 @@ static void init_once(void *foo)
> mutex_init(&iint->mutex);
> }
>
> -static int __init integrity_iintcache_init(void)
> +void __init integrity_add_lsm_hooks(struct security_hook_list *hooks,
> + int count)
> +{
> + security_add_hooks(hooks, count, "integrity");
> +}
> +
> +static int __init integrity_lsm_init(void)
> {
> iint_cache =
> kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
> 0, SLAB_PANIC, init_once);
> +
> + integrity_lsm_ima_init();
> + integrity_lsm_evm_init();
> +
> return 0;
> }
> +
> DEFINE_LSM(integrity) = {
> .name = "integrity",
> - .init = integrity_iintcache_init,
> + .init = integrity_lsm_init,

For backward compatibility, there should be an ".order =
LSM_ORDER_FIRST," here.