Thread (92 messages) 92 messages, 7 authors, 2018-10-08

Re: [PATCH security-next v4 23/32] selinux: Remove boot parameter

From: Kees Cook <hidden>
Date: 2018-10-03 23:55:39
Also in: linux-arch, linux-doc, lkml

On Wed, Oct 3, 2018 at 2:34 PM, James Morris [off-list ref] wrote:
On Wed, 3 Oct 2018, Kees Cook wrote:
quoted
On Wed, Oct 3, 2018 at 11:28 AM, James Morris [off-list ref] wrote:
quoted
On Wed, 3 Oct 2018, Kees Cook wrote:
quoted
On Wed, Oct 3, 2018 at 11:17 AM, James Morris [off-list ref] wrote:
quoted
On Tue, 2 Oct 2018, John Johansen wrote:
quoted
To me a list like
  lsm.enable=X,Y,Z
What about even simpler:

lsm=selinux,!apparmor,yama
We're going to have lsm.order=, so I'd like to keep it with a dot
separator (this makes it more like module parameters, too). You want
to mix enable/disable in the same string? That implies you'd want
implicit enabling (i.e. it complements the builtin enabling), which is
opposite from what John wanted.
Why can't this be the order as well?
That was covered extensively in the earlier threads. It boils down to
making sure we do not create a pattern of leaving LSMs disabled by
default when they are added to the kernel. The v1 series used
security= like this:

+       security=       [SECURITY] An ordered comma-separated list of
+                       security modules to attempt to enable at boot. If
+                       this boot parameter is not specified, only the
+                       security modules asking for initialization will be
+                       enabled (see CONFIG_DEFAULT_SECURITY). Duplicate
+                       or invalid security modules will be ignored. The
+                       capability module is always loaded first, without
+                       regard to this parameter.

This meant booting "security=apparmor" would disable all the other
LSMs, which wasn't friendly at all. So "security=" was left alone (to
leave it to only select the "major" LSM: all major LSMs not matching
"security=" would be disabled). So I proposed "lsm.order=" to specify
the order things were going to be initialized in, but to avoid kernels
booting with newly added LSMs forced-off due to not being listed in
"lsm.order=", it had to have implicit fall-back for unlisted LSMs.
(i.e. anything missing from lsm.order would then follow their order in
CONFIG_LSM_ORDER, and anything missing there would fall back to
link-time ordering.) However, then the objection was raised that this
didn't provide a way to explicitly disable an LSM. So I proposed
lsm.enable/disable, and John argued for CONFIG_LSM_ENABLE over
CONFIG_LSM_DISABLE.
Ok, but it may end up being clearer, simpler, and thus more secure to just
have a single way to configure LSM.

For example:

  - All LSMs which are built are NOT enabled by default

  - You specify enablement and order via a Kconfig:

        CONFIG_LSM="selinux,yama"

  - This can be entirely overridden by a boot param:

        lsm="apparmor,landlock"
This doesn't work with how SELinux and AppArmor do their bootparams,
unfortunately. (And Paul and Stephen have expressed that the
documented selinux on/off must continue to work.) For example, let's
say you've built an Ubuntu kernel with:

CONFIG_SELINUX=y
...
CONFIG_LSM="yama,apparmor"

(i.e. you want SELinux available, but not enabled, so it's left out of
CONFIG_LSM)

Then someone boots the system with:

selinux=1 security=selinux

In what order does selinux get initialized relative to yama?
(apparmor, flagged as a "legacy major", would have been disabled by
the "security=" not matching it.)


The LSM order needs to be defined externally to enablement because
something may become enabled when not listed in the order.

Now, maybe I misunderstood your earlier suggestion, and what you meant
was to do something like:

CONFIG_LSM="yama,apparmor,!selinux"

to mean "put selinux here in the order, but don't enable it". Then the
problem becomes what happens to an LSM that has been built in but not
listed in CONFIG_LSM?

Related to that, this means that when new LSMs are added, they will
need to be added to any custom CONFIG_LSM= or lsm= parameters. If
that's really how we have to go, I'll accept it, but I think it's a
bit unfriendly. :P

Another reason I don't like it is because it requires users to know
about all the LSMs to make changes. One LSM can't be added/removed
without specifying ALL of the LSMs. (i.e. there is no trivial way to
enable/disable a single LSM without it growing its own enable/disable
code as in SELinux/AppArmor. I'd hoped to make that easier for both
users and developers.) Again, I can live with it, but I think it's
unfriendly.

I just want to have a direct I can go that meets all the requirements.
:) I'm fine to ignore my sense of aesthetics if everyone can agree on
the code.
And that's it.

Of course, capabilities is always enabled and not be visible to kconfig or
boot params.
Correct. I've made sure that's true in all the versions.

BTW, there doesn't seem to be disagreement about the earlier part of
the series, though (patches 1-10). Could these go into -next just so I
don't have to keep sending them? :)

LSM: Correctly announce start of LSM initialization
vmlinux.lds.h: Avoid copy/paste of security_init section
LSM: Rename .security_initcall section to .lsm_info
LSM: Remove initcall tracing
LSM: Convert from initcall to struct lsm_info
vmlinux.lds.h: Move LSM_TABLE into INIT_DATA
LSM: Convert security_initcall() into DEFINE_LSM()
LSM: Record LSM name in struct lsm_info
LSM: Provide init debugging infrastructure
LSM: Don't ignore initialization failures

Thanks!

-Kees

-- 
Kees Cook
Pixel Security
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help