On 10/02/2018 12:17 PM, Kees Cook wrote: > On Tue, Oct 2, 2018 at 11:57 AM, John Johansen > <john.johansen@xxxxxxxxxxxxx> wrote: >> Under the current scheme >> >> lsm.enabled=selinux >> >> could actually mean selinux,yama,loadpin,something_else are >> enabled. If we extend this behavior to when full stacking lands >> >> lsm.enabled=selinux,yama >> >> might mean selinux,yama,apparmor,loadpin,something_else >> >> and what that list is will vary from kernel to kernel, which I think >> is harder for the user than the lsm.enabled list being what is >> actually enabled at boot > > Ah, I think I missed this in your earlier emails. What you don't like > here is that "lsm.enable=" is additive. You want it to be explicit. > > Are you okay with lsm.order= having fallback? > yeah, if we are going to separate order, fallbacks are fine for anything that isn't specified. I am still not convinced that separating order from enablement is right, but its generally something a user should care about so I can live with it. > The situation we were trying to solve was with new LSMs getting > implicitly disabled if someone is booting with an explicit list. For > example: > > lsm.enable=yama,apparmor > > means when "landlock" gets added to the kernel, it will be implicitly disabled. > And here is the point of contention, I wouldn't call that implicitly disabled. The user explicitly selected a set of LSMs to enable. Having other LSMs enable when they aren't specified is confusing to a user, as now they have to consider what is enabled by default in the Kconfig. I think requiring distros/builders to consider Kconfig options is fine, but its a lot higher hurdle for regular users. >> If we have to have multiple kernel parameter, I prefer a behvior where >> if you hav conflicting kernel parameters specified >> >> apparmor=0 lsm.enabled=apparmor >> >> that the conflict is logged and the lsm is left disabled, as I think >> it is easier for users to understand than the overrides scheme of v3, >> and sans logging of the conflict is effectively what we had in the >> past >> >> apparmor=0 security=apparmor >> or >> apparmor=1 security=selinux >> >> would result in apparmor being disabed > > Okay, so for this part you want per-LSM boot param to have priority > (which seems to match SELinux's concerns), possibly logging the hrmmm I wouldn't call it priority :) If you look at the above logic its a boolean AND operation. The LSM is only enabled if $LSM=1 AND security=$LSM all other combinations result in $LSM being disabled > conflict, but still accepting the apparmor= and selinux= state. logging is nice for the user but certainly isn't required and is more than we are doing today > security= would still driving initialization ordering (so I think the > behavior I have in the series would be correct). > >> That being said I get we have a mess currently, and there really >> doesn't seem to be a good way to fix it. I think getting this right >> for the user is important enough that I am willing to break current >> apparmor userspace api. While apparmor=0 is documented we have also >> documented security=X for years and apparmor=0 isn't used too often >> so I think we can drop it to help clean this mess up abit. >> >> I am not going to Nak, or block on v3 behavior if that is considered >> the best path forward after this discussion/rant. > > I could define CONFIG_LSM_ENABLE as being "additive" to > SECURITY_APPARMOR_BOOTPARAM_VALUE and > SECURITY_SELINUX_BOOTPARAM_VALUE? > Oh sure lets deal with my complaint about too many ways to configure this beast by adding yet another config option :P seriously though, please no. That just adds another layer of confusion even if it is only being foisted on the distro/builder