Message-ID: <CAGXu5jJZ1H1RFMFBxPsoBW+T17ya+50-d7-31Gr9ciRgpCOaDA@mail.gmail.com>
Date:   Wed, 27 Mar 2019 12:16:12 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     James Morris <jmorris@...ei.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        linux-security-module <linux-security-module@...r.kernel.org>
Subject: Re: Linux 5.1-rc2

On Mon, Mar 25, 2019 at 2:06 PM Tetsuo Handa
<penguin-kernel@...ove.sakura.ne.jp> wrote:
>
> On 2019/03/26 4:08, James Morris wrote:
> > On Sun, 24 Mar 2019, Randy Dunlap wrote:
> >
> >> On 3/24/19 2:26 PM, Linus Torvalds wrote:
> >>> Well, we're a week away from the merge window close, and here's rc2.
> >>> Things look fairly normal, but honestly, rc2 is usually too early to
> >>> tell.  People haven't necessarily had time to notice problems yet.
> >>> Which is just another way of saying "please test harder".
> >>>
> >>> Nothing particularly stands out. Yes, we had some fixes for the new
> >>> io_ring code for issues that were discussed when merging it. Other
> >>> than that, worth noting is that the bulk of the patches are for
> >>> tooling, not the core kernel. In fact, about two thirds of the patch
> >>> is just for the tools/ subdirectory, most of it due to some late perf
> >>> tool updates. The people involved promise they're done.
> >>
> >> Hmph.  I'm still looking for the patch that restores the various
> >> CONFIG_DEFAULT_<security> kconfig options to be merged.
> >>
> >> https://lore.kernel.org/linux-security-module/2bf23acd-22c4-a260-7648-845887a409d5@i-love.sakura.ne.jp/
> >>
> >> since commit 70b62c25665f636c9f6c700b26af7df296b0887e dropped them somehow.
> >
> > AFAICT we don't have a finalized version of the patch yet.
> >
> > Kees?

Sorry for the delay -- back from travel now.

> As far as I can tell, Kees's comment
>
>   It breaks the backward-compat for the "security=" line. If a system is
>   booted with CONFIG_LSM="minors...,apparmor" and "security=selinux",
>   neither apparmor nor selinux will be initialized. The logic on
>   "security=..." depends on the other LSMs being present in the list.
>
> was just a confusion

Yes, you are correct here. This is what I get for drive-by comments
while travelling. :) However, I don't like that it creates an
incomplete LSM list for no reason. I'd like CONFIG_LSM to be built in
a way that future stack-enabling will Just Work. Leaving off LSMs
means it won't. My original patch doesn't change the behavior relative
to the old configs (i.e. the CONFIG_DEFAULT_SECURITY_* will still be
selected and turn off the others) but does allow the other LSMs to be
initialized in the future once earlier ones in the list become
stackable.

The part I don't understand is what you've said about TOMOYO being
primary and not wanting the others stackable? That kind of goes
against the point, but I'm happy to do that if you want it that way.
If so, my current proposal would be:

 config LSM
        string "Ordered list of enabled LSMs"
+       default
"yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if
DEFAULT_SECURITY_SMACK
+       default
"yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if
DEFAULT_SECURITY_APPARMOR
+       default "yama,loadpin,safesetid,integrity,tomoyo" if
DEFAULT_SECURITY_TOMOYO
+       default "yama,loadpin,safesetid,integrity" if DEFAULT_SECURITY_DAC
        default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"

Note that the last default line holds for both "new build" and
"selinux chosen". The other change from my earlier patch is that _DAC
must turn off all the legacy major LSMs to get the behavior Randy was
expecting. Shall I send a patch that does the above, or is there
another wrinkle?

Thanks!

-Kees

> the finalized version.
>
> From 72f5f21b800c87f9ec3600f6e3acfb654690d8f0 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> Date: Tue, 26 Mar 2019 05:56:30 +0900
> Subject: [PATCH] LSM: Revive CONFIG_DEFAULT_SECURITY_* for "make oldconfig"
>
> Commit 70b62c25665f636c ("LoadPin: Initialize as ordered LSM") removed
> CONFIG_DEFAULT_SECURITY_{SELINUX,SMACK,TOMOYO,APPARMOR,DAC} from
> security/Kconfig and changed CONFIG_LSM to provide a fixed ordering as a
> default value. That commit expected that existing users (upgrading from
> Linux 5.0 and earlier) will edit CONFIG_LSM value in accordance with
> their CONFIG_DEFAULT_SECURITY_* choice in their old kernel configs. But
> since users might forget to edit CONFIG_LSM value, this patch revives
> the choice (only for providing the default value for CONFIG_LSM) in order
> to make sure that CONFIG_LSM reflects CONFIG_DEFAULT_SECURITY_* from their
> old kernel configs.
>
> Reported-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> Acked-by: Casey Schaufler <casey@...aufler-ca.com>
> ---
>  security/Kconfig | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 1d6463f..2f29805 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -239,9 +239,44 @@ source "security/safesetid/Kconfig"
>
>  source "security/integrity/Kconfig"
>
> +choice
> +       prompt "Default security module [superseded by 'Ordered list of enabled LSMs' below]"
> +       default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
> +       default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> +       default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> +       default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> +       default DEFAULT_SECURITY_DAC
> +
> +       help
> +         This choice is there only for converting CONFIG_DEFAULT_SECURITY in old
> +         kernel config to CONFIG_LSM in new kernel config. Don't change this choice
> +         unless you are creating a fresh kernel config, for this choice will be
> +         ignored after CONFIG_LSM is once defined.
> +
> +       config DEFAULT_SECURITY_SELINUX
> +               bool "SELinux" if SECURITY_SELINUX=y
> +
> +       config DEFAULT_SECURITY_SMACK
> +               bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
> +
> +       config DEFAULT_SECURITY_TOMOYO
> +               bool "TOMOYO" if SECURITY_TOMOYO=y
> +
> +       config DEFAULT_SECURITY_APPARMOR
> +               bool "AppArmor" if SECURITY_APPARMOR=y
> +
> +       config DEFAULT_SECURITY_DAC
> +               bool "Unix Discretionary Access Controls"
> +
> +endchoice
> +
>  config LSM
>         string "Ordered list of enabled LSMs"
> -       default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> +       default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
> +       default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
> +       default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
> +       default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
> +       default "yama,loadpin,safesetid,integrity"
>         help
>           A comma-separated list of LSMs, in initialization order.
>           Any LSMs left off this list will be ignored. This can be
> --
> 1.8.3.1



-- 
Kees Cook

Powered by blists - more mailing lists