On 10/31/2022 3:26 AM, Tetsuo Handa wrote: > On 2022/10/31 1:37, Kees Cook wrote: >>> You have only three choices: >>> >>> (1) allow assigning LSM ID integer value to all LSM modules (regardless of >>> whether that module was merged into upstream kernel) >> We are not hardware manufacturers. >> > Excuse me? We are not talking about whether we are hardware manufacturers. > We are talking about the policy for assigning identifier. > > I don't like how LSM IDs are assigned, for Casey said > > >> If the upstream kernel assigns an LSM id for all LSM modules including out-of-tree > >> and/or private LSM modules (that's why I described that the upstream kernel behaves > >> as if a DNS registerer), we can assign LSM id = 100 to "belllapadula" from A and > >> LSM id = 101 to "belllapadula" from B, and both "belllapadula" modules can work > >> without conflicts by using LSM id. Of course, this implies that we need to preserve > >> unused space in lsm_idlist[LSMID_ENTRIES] etc. for such LSM modules (if we use > >> fixed-sized array rather than a linked list). > > > > Of course the upstream kernel isn't going to have LSM IDs for out-of-tree > > security modules. That's one of many reasons loadable modules are going to > > have to be treated differently from built-in modules, if they're allowed > > at all. > > at https://lkml.kernel.org/r/7263e155-9024-0508-370c-72692901b326@xxxxxxxxxxxxxxxx and > Paul confirmed > > >> Currently anyone can start writing new LSM modules using name as identifier. But > >> you are trying to forbid using name as identifier, and trying to force using integer > >> as identifier, but that integer will not be provided unless new LSM modules get > >> upstream. > > > > That is correct. In order to have a LSM identifier token the LSM must > > be upstream. > > at https://lkml.kernel.org/r/CAHC9VhT2Azg1F-G3RQ4xL7JgA3OAtHafzS1_nvUyEUFsCJ9+SA@xxxxxxxxxxxxxx . > > If we can agree that the upstream kernel never refuse to assign LSM IDs to whatever > LSM modules, I'm OK that we introduce LSM ID integer value itself. > > > > My next concern is that we are trying to use fixed sized capacity as LSMID_ENTRIES, > commented > > On 2022/10/13 19:04, Tetsuo Handa wrote: > > On 2022/09/28 4:53, Casey Schaufler wrote: > >> + if (lsm_id > LSMID_ENTRIES) > >> + panic("%s Too many LSMs registered.\n", __func__); > > > > I'm not happy with LSMID_ENTRIES. This is a way towards forever forbidding LKM-based LSMs. > > at https://lkml.kernel.org/r/9907d724-4668-cd50-7454-1a8ca86542b0@xxxxxxxxxxxxxxxxxxx , for > > struct lsm_id *lsm_idlist[LSMID_ENTRIES] __lsm_ro_after_init; > > may cause out-of-tree LSM modules to fail to use the slot. > > It is a strange hack that users have to enable in-tree LSM modules or rewrite the > definition of LSMID_ENTRIES in order to use out-of-tree (either built-in or loadable) > LSM modules, for LSMID_ENTRIES is defined based on only in-tree LSM modules. > > #define LSMID_ENTRIES ( \ > 1 + /* capabilities */ \ > (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ > (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) > > Although built-in out-of-tree LSM modules would be able to rewrite LSMID_ENTRIES definition > because users will rebuild the whole kernel, loadable out-of-tree LSM modules would not be > able to rewrite LSMID_ENTRIES definition because users will not rebuild the whole kernel. > It is still effectively a lock out for loadable out-of-tree LSM modules even if the problem > of assigning LSM ID integer value is solved. Propose an implementation of security module loading. If LSMID_ENTRIES is a problem I will help you resolve the issue. My bet is that there will be an easy solution. It may be as simple as adding (IS_ENABLED(CONFIG_SECURITY_LOADABLE) ? 1 : 0) + \ to the code referenced above. But I can't say until I see how you're going to address all of the real issues.
