[PATCH] apparmor: Restore Y/N in /sys for apparmor's "enabled"

[Kees Cook]

On Mon, Apr 8, 2019 at 9:58 AM John Johansen
<john.johansen at canonical.com> wrote:
> > +/* Can only be set before AppArmor is initialized (i.e. on boot cmdline). */
> > +static int param_set_aaintbool(const char *val, const struct kernel_param *kp)
> > +{
> > +     struct kernel_param kp_local;
> > +     bool value;
> > +     int error;
> > +
> > +     if (apparmor_initialized)
> > +             return -EPERM;
> > +
> This isn't sufficient/correct. apparmor_initialized is only set after
> apparmor has gone through and completed initialization. However if
> apparmor is not selected as one of the LSMs to enable, then this check
> won't stop apparmor_enabled from being set post boot.
>
> However with the apparmor_enabled param being 0444 and the
> apparmor_enabled_setup() fn handling boot cmdline do with even need
> the set parameter fn?

Yup, that's true. I've gone and tested this, and yes, the 0444 is
sufficient to protect the logic here (even if root chmods the inode).
So the test here is redundant. However, very early in the threads
about LSM boot cmdline enabling it was made clear that
"apparmor.enabled=..." needed to stay working, which means the "set"
op is still needed. (But I'm happy to do whatever you want here -- I
was just trying to keep the functionality as it was.)

Should I send a v2 without the "initialized" check or is this okay to
leave as-is with the redundant check?

-- 
Kees Cook