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

[Mickaël Salaün]

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@xxxxxxxxxxxxxx>
> Cc: James Morris <jmorris@xxxxxxxxx>
> Cc: "Serge E. Hallyn" <serge@xxxxxxxxxx>
> Cc: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@xxxxxxxxx>
> Cc: "Mickaël Salaün" <mic@xxxxxxxxxxx>
> Cc: linux-security-module@xxxxxxxxxxxxxxx
> Cc: linux-integrity@xxxxxxxxxxxxxxx
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>   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@xxxxxxxxxxx/


>   	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.