Re: [PATCH 1/1] NAX LSM: Add initial support support

Hi Simon,

Thanks for thorough review. Will post the corrected version soon.

quoted

@@ -278,11 +279,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 "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
+     default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
+     default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
+     default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
+     default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"

I don't know the policy with regard to new LSMs, but enabling this one by default is maybe a bit violent?
That said, considering the default mode is SECURITY_NAX_MODE_LOG, this would just log kernel messages and
not break existing systems, so this shouldn't be a problem.

I've just oriended on landlock and lockdown. They are handled in the similar
way. But, yes, by default NAX module doesn't block anything.
If you suggest not to do that, I can remove it.

quoted

+__setup("nax_mode=", setup_mode);
+
+static int __init setup_quiet(char *str)
+{
+     unsigned long val;
+
+     if (!locked && !kstrtoul(str, 0, &val))
+             quiet = val ? 1 : 0;

The order of the kernel parameters will have an impact on NAX behavior.

"nax_mode=1 nax_locked=1" and "nax_locked=1 nax_mode=1" will end up with the same behavior.
in the first case the mode is enforced, in the second case it isn't (well unless you changed
 the kernel configuration from the default) and it can't be enabled later either.

Is that desired?

Yes. The idea is that on boot you can set-up the proper options then block
further changing (especially turning the module off).
Initially it was implemented for sysctl parameters, so, you can change
something in init-scripts, then lock. Then, I've extended it to command-line
parameters.
I can ignore "locked" flag in setup_* functions to defer locking to sysctl
parsing. (Unless our command-line is glued by the bootloader from several
parts, so we want to lock it as early as possible...).

Thanks.