On Mon, Oct 1, 2018 at 3:20 PM, John Johansen
[off-list ref] wrote:

On 10/01/2018 02:56 PM, Kees Cook wrote:

quoted

On Mon, Oct 1, 2018 at 2:47 PM, James Morris [off-list ref] wrote:

quoted

On Mon, 24 Sep 2018, Kees Cook wrote:

quoted

In preparation for lifting the "is this LSM enabled?" logic out of the
individual LSMs, pass in any special enabled state tracking (as needed
for SELinux, AppArmor, and LoadPin). This should be an "int" to include
handling any future cases where "enabled" is exposed via sysctl which
has no "bool" type.

Signed-off-by: Kees Cook <redacted>
---
 include/linux/lsm_hooks.h | 1 +
 security/apparmor/lsm.c   | 5 +++--
 security/selinux/hooks.c  | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 5056f7374b3d..2a41e8e6f6e5 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -2044,6 +2044,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,
 struct lsm_info {
      const char *name;       /* Populated automatically. */
      unsigned long flags;    /* Optional: flags describing LSM */
+     int *enabled;           /* Optional: NULL means enabled. */

This seems potentially confusing.

Perhaps initialize 'enabled' to a default int pointer, like:

        static int lsm_default_enabled = 1;

Then,

        DEFINE_LSM(foobar)
        flags = LSM_FLAG_LEGACY_MAJOR,
        .enabled = &lsm_default_enabled,
        .init = foobar_init,
        END_LSM;

The reason I didn't do this is because there are only two LSMs that
expose this "enabled" variable, so I didn't like making the other LSMs
have to declare this. Internally, though, this is exactly what the
infrastructure does: if it finds a NULL, it aims it at
&lsm_default_enabled (in a later patch).

However, it seems more discussion is needed on the "enable" bit of
this, so I'll reply to John in a moment...

fwiw the apparmor.enabled config is really only a meant to be used to
disable apparmor. I'd drop it entirely except its part of the userspace
api now and needs to show up in

  /sys/module/apparmor/parameters/enabled

Showing the enabled-ness there can be wired up. What should happen if
someone sets apparmor.enabled=0/1 in new-series-world? (See other
thread...)

-Kees

-- 
Kees Cook
Pixel Security