diff mbox series

[v38,04/39] LSM: Maintain a table of LSM attribute data

Message ID 20220927195421.14713-5-casey@schaufler-ca.com (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series LSM: Module stacking for AppArmor | expand

Commit Message

Casey Schaufler Sept. 27, 2022, 7:53 p.m. UTC
As LSMs are registered add their lsm_id pointers to a table.
This will be used later for attribute reporting.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/security.h | 17 +++++++++++++++++
 security/security.c      | 18 ++++++++++++++++++
 2 files changed, 35 insertions(+)

Comments

Tetsuo Handa Oct. 13, 2022, 10:04 a.m. UTC | #1
On 2022/09/28 4:53, Casey Schaufler wrote:
> @@ -483,6 +491,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  {
>  	int i;
>  
> +	/*
> +	 * A security module may call security_add_hooks() more
> +	 * than once. Landlock is one such case.
> +	 */
> +	if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid)
> +		lsm_idlist[lsm_id++] = lsmid;
> +
> +	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.

I'm fine with using UAPI-visible constants for switching /proc/ files.
But TOMOYO does not need such constant because TOMOYO does not use /proc/ files.

Also, lsm_self_attr() will be limited for LSM modules which use /proc/ files, and
therefore I think prctl() will be already there.

> +
>  	for (i = 0; i < count; i++) {
>  		hooks[i].lsmid = lsmid;
>  		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
Casey Schaufler Oct. 20, 2022, 11:42 p.m. UTC | #2
On 10/13/2022 3:04 AM, Tetsuo Handa wrote:
> On 2022/09/28 4:53, Casey Schaufler wrote:
>> @@ -483,6 +491,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>  {
>>  	int i;
>>  
>> +	/*
>> +	 * A security module may call security_add_hooks() more
>> +	 * than once. Landlock is one such case.
>> +	 */
>> +	if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid)
>> +		lsm_idlist[lsm_id++] = lsmid;
>> +
>> +	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.

I don't see any way given the locking issues that we're ever going to
mix built in security modules and loaded security modules on the same
hook lists. The SELinux module deletion code is sufficiently scary that
it is being removed. That does not mean that I think loadable modules
are impossible, I think it means that their management is going to have
to be separate, the same way the BPF programs are handled. The only way
that I see a unified hook list is for all the LSMs to be implemented as
loadable modules, and I can't see that happening in my lifetime.

I can see an LSM like BPF, as I mentioned before, that manages loaded
modules. Over the years I've seen several designs that might work. I'm
encouraged (and not a little bit frightened) by the success of the BPF
work.

Converting the array[LSMID_ENTRIES] implementation to a hlist like the
hooks have used would not be that big a project and I don't see that
making such a change would be a show-stopper for implementing loadable
modules. I think that a lot of other issues would be more significant.

I will, on the other hand, listen to compelling arguments. It is not the
intention of this code to lock out loadable modules. If I thought it would
I would not have proposed it.

> I'm fine with using UAPI-visible constants for switching /proc/ files.
> But TOMOYO does not need such constant because TOMOYO does not use /proc/ files.
>
> Also, lsm_self_attr() will be limited for LSM modules which use /proc/ files, and
> therefore I think prctl() will be already there.

While the proposed set of attributes map to those in /proc/.../attr there is
no reason to assume they will be limited to those. I can see providing several
of the Smack attributes currently manipulated in smackfs, such as relabel-self.
If we are providing SELinux specific values like keycreate there's no reason
we can't provide Smack or TOMOYO specific values as well.

>
>> +
>>  	for (i = 0; i < count; i++) {
>>  		hooks[i].lsmid = lsmid;
>>  		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
Tetsuo Handa Oct. 23, 2022, 7:27 a.m. UTC | #3
On 2022/10/21 8:42, Casey Schaufler wrote:
> On 10/13/2022 3:04 AM, Tetsuo Handa wrote:
>> On 2022/09/28 4:53, Casey Schaufler wrote:
>>> @@ -483,6 +491,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>>  {
>>>  	int i;
>>>  
>>> +	/*
>>> +	 * A security module may call security_add_hooks() more
>>> +	 * than once. Landlock is one such case.
>>> +	 */
>>> +	if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid)
>>> +		lsm_idlist[lsm_id++] = lsmid;
>>> +
>>> +	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.
> 
> I don't see any way given the locking issues that we're ever going to
> mix built in security modules and loaded security modules on the same
> hook lists. The SELinux module deletion code is sufficiently scary that
> it is being removed. That does not mean that I think loadable modules
> are impossible, I think it means that their management is going to have
> to be separate, the same way the BPF programs are handled. The only way
> that I see a unified hook list is for all the LSMs to be implemented as
> loadable modules, and I can't see that happening in my lifetime.

I'm not expecting for unloadable LSM modules.
I'm expecting for loadable LSM modules.

I'm not expecting to make all LSM modules to be implemented as loadable
LSM modules, for some want to associate "security label" to everything
(including processes which might start before the global init process starts)
but others do not need to associate "security label" to everything.

> 
> I can see an LSM like BPF, as I mentioned before, that manages loaded
> modules. Over the years I've seen several designs that might work. I'm
> encouraged (and not a little bit frightened) by the success of the BPF
> work.

There can be LSM modules whose lifetime of hooks match the lifetime of
a process which registered hooks for that process. In that case, being
automatically unregistered upon process termination would be preferable.

But there are LSM modules whose lifetime of hooks is irrelevant to a process
which registered a hook for that process. In that case, we need a method for
allowing registered hooks to remain even after that process terminated.

Please don't think loadable LSM modules as something that require special
handling. TOMOYO is an LSM module whose lifetime of hooks is irrelevant to
a process which registered a hook for that process, but does not need to
associate "security label" to everything. It has to be trivial to convert
TOMOYO as a loadable LSM module.

> 
> Converting the array[LSMID_ENTRIES] implementation to a hlist like the
> hooks have used would not be that big a project and I don't see that
> making such a change would be a show-stopper for implementing loadable
> modules. I think that a lot of other issues would be more significant.

Defining constants for each LSM module (i.e. "LSM: Add an LSM identifier
for external use") is the show-stopper for implementing loadable modules.
We won't be able to accept whatever LSM modules to upstream, and we won't
be able to enable whatever LSM modules in distributor kernels.

LSM modules which cannot define a constant due to either "not accepted
to upstream" or "not enabled by distributor kernels" will be forbidden.
I expect that we assign a constant upon module registration (instead of
API visible constants) if we require all LSM modules to have a constant.

> 
> I will, on the other hand, listen to compelling arguments. It is not the
> intention of this code to lock out loadable modules. If I thought it would
> I would not have proposed it.

This code is exactly for locking out loadable modules.
Tetsuo Handa Oct. 23, 2022, 10:10 a.m. UTC | #4
On 2022/10/23 16:27, Tetsuo Handa wrote:
> On 2022/10/21 8:42, Casey Schaufler wrote:
>> I will, on the other hand, listen to compelling arguments. It is not the
>> intention of this code to lock out loadable modules. If I thought it would
>> I would not have proposed it.
> 
> This code is exactly for locking out loadable modules.
> 

Imagine a situation where two individuals independently develop their own
web applications using the same identifier, and then their web applications
started working together with other web applications using that identifier.
When they published their web applications for public and wider use, a problem
that both web applications are already using the same identifier arises.
It is too late to reassign the identifier.

The same trouble can happen with loadable LSM modules. Unless the upstream kernel
behaves as if a DNS registerer that assigns a unique domainname for whatever web
sites (regardless of whether a web site is for public or not), defining a permanent
constant for LSM module is a way towards locking out loadable LSM modules. And it
is well possible that a loadable LSM module wants to run on older kernels which
do not have LSM id defined yet.

This "define LSM id as userspace visible constant" is more dangerous than just
reserving some space for future use. You are trying to control all IP addresses
for the sake of only in-tree LSM modules. No, no, no, please don't do that...
Casey Schaufler Oct. 23, 2022, 5:13 p.m. UTC | #5
On 10/23/2022 12:27 AM, Tetsuo Handa wrote:
> On 2022/10/21 8:42, Casey Schaufler wrote:
>> On 10/13/2022 3:04 AM, Tetsuo Handa wrote:
>>> On 2022/09/28 4:53, Casey Schaufler wrote:
>>>> @@ -483,6 +491,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>>>  {
>>>>  	int i;
>>>>  
>>>> +	/*
>>>> +	 * A security module may call security_add_hooks() more
>>>> +	 * than once. Landlock is one such case.
>>>> +	 */
>>>> +	if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid)
>>>> +		lsm_idlist[lsm_id++] = lsmid;
>>>> +
>>>> +	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.
>> I don't see any way given the locking issues that we're ever going to
>> mix built in security modules and loaded security modules on the same
>> hook lists. The SELinux module deletion code is sufficiently scary that
>> it is being removed. That does not mean that I think loadable modules
>> are impossible, I think it means that their management is going to have
>> to be separate, the same way the BPF programs are handled. The only way
>> that I see a unified hook list is for all the LSMs to be implemented as
>> loadable modules, and I can't see that happening in my lifetime.
> I'm not expecting for unloadable LSM modules.
> I'm expecting for loadable LSM modules.
>
> I'm not expecting to make all LSM modules to be implemented as loadable
> LSM modules, for some want to associate "security label" to everything
> (including processes which might start before the global init process starts)
> but others do not need to associate "security label" to everything.
>
>> I can see an LSM like BPF, as I mentioned before, that manages loaded
>> modules. Over the years I've seen several designs that might work. I'm
>> encouraged (and not a little bit frightened) by the success of the BPF
>> work.
> There can be LSM modules whose lifetime of hooks match the lifetime of
> a process which registered hooks for that process. In that case, being
> automatically unregistered upon process termination would be preferable.
>
> But there are LSM modules whose lifetime of hooks is irrelevant to a process
> which registered a hook for that process. In that case, we need a method for
> allowing registered hooks to remain even after that process terminated.
>
> Please don't think loadable LSM modules as something that require special
> handling. TOMOYO is an LSM module whose lifetime of hooks is irrelevant to
> a process which registered a hook for that process, but does not need to
> associate "security label" to everything. It has to be trivial to convert
> TOMOYO as a loadable LSM module.

I don't see that having a built-in version of TOMOYO and a loadable version
needs to be difficult. That's something that whoever creates the loadable
security module scheme is going to have to address. It will depend on the
details of the loadable module mechanism. I can't comment on how that will
work because I don't see loadable modules hitting the top of my queue.

>
>> Converting the array[LSMID_ENTRIES] implementation to a hlist like the
>> hooks have used would not be that big a project and I don't see that
>> making such a change would be a show-stopper for implementing loadable
>> modules. I think that a lot of other issues would be more significant.
> Defining constants for each LSM module (i.e. "LSM: Add an LSM identifier
> for external use") is the show-stopper for implementing loadable modules.

One possible way for loadable modules to work would be to have a built-in
module LSM_ID_MODLOADER which maintains its own list of module hooks.
The values returned from lsm_self_attr() would identify the this LSM
and the data value would have to identify the loaded module it refers to,
perhaps as "TOMOYO=XYZ" or "datastate=foobar". A flag LSM_ATTR_LOADED would
indicate that the attribute needed to be processed according to loadable
module attribute rules, whatever they might be.

So no, it's not a show stopper. Not any more than u32 secids are a showstopper
for process attributes it audit records. LSM IDs are inconvenient, and not my
first choice, but I'm not going to let that get in the way of getting this
code upstream.  

> We won't be able to accept whatever LSM modules to upstream, and we won't
> be able to enable whatever LSM modules in distributor kernels.

A built in module loader security module would address this issue.
Getting such a module accepted upstream is not going to be trivial,
but the BPF people seem to have managed it.

> LSM modules which cannot define a constant due to either "not accepted
> to upstream" or "not enabled by distributor kernels" will be forbidden.
> I expect that we assign a constant upon module registration (instead of
> API visible constants) if we require all LSM modules to have a constant.

Maybe the thing to do is rewrite TOMOYO in eBPF. If I wanted to have a
loadable security module I could either take ten years or so to get a
loadable module scheme upstream in addition to my module, or I could
write it in eBPF and use it the next day. I don't know enough about eBPF
programming to say if it has everything TOMOYO needs, but it sure looks
like an easier path if it does.

>> I will, on the other hand, listen to compelling arguments. It is not the
>> intention of this code to lock out loadable modules. If I thought it would
>> I would not have proposed it.
> This code is exactly for locking out loadable modules.

I hope that I have suggested viable (if not convenient) alternatives.
I suppose it is possible that locking out loadable modules is one
motivation behind the LSM ID scheme, but I really doubt it. And more
importantly, as I've outlined above, I can't be successful in locking
out loadable security modules. I don't even see it as an additional
complication.
Casey Schaufler Oct. 23, 2022, 5:20 p.m. UTC | #6
On 10/23/2022 3:10 AM, Tetsuo Handa wrote:
> On 2022/10/23 16:27, Tetsuo Handa wrote:
>> On 2022/10/21 8:42, Casey Schaufler wrote:
>>> I will, on the other hand, listen to compelling arguments. It is not the
>>> intention of this code to lock out loadable modules. If I thought it would
>>> I would not have proposed it.
>> This code is exactly for locking out loadable modules.
>>
> Imagine a situation where two individuals independently develop their own
> web applications using the same identifier, and then their web applications
> started working together with other web applications using that identifier.
> When they published their web applications for public and wider use, a problem
> that both web applications are already using the same identifier arises.
> It is too late to reassign the identifier.
>
> The same trouble can happen with loadable LSM modules. Unless the upstream kernel
> behaves as if a DNS registerer that assigns a unique domainname for whatever web
> sites (regardless of whether a web site is for public or not), defining a permanent
> constant for LSM module is a way towards locking out loadable LSM modules. And it
> is well possible that a loadable LSM module wants to run on older kernels which
> do not have LSM id defined yet.
>
> This "define LSM id as userspace visible constant" is more dangerous than just
> reserving some space for future use. You are trying to control all IP addresses
> for the sake of only in-tree LSM modules. No, no, no, please don't do that...

It's really no more dangerous than using the LSM name. What if two developers
implement modules and both name it "belllapadula"? User space won't be able to
tell the difference if they base behavior on the module name. That's one thing
that a loadable module mechanism is going to need to address that a built-in
mechanism doesn't.
Tetsuo Handa Oct. 24, 2022, 3:13 p.m. UTC | #7
On 2022/10/24 2:13, Casey Schaufler wrote:
>> We won't be able to accept whatever LSM modules to upstream, and we won't
>> be able to enable whatever LSM modules in distributor kernels.
> 
> A built in module loader security module would address this issue.
> Getting such a module accepted upstream is not going to be trivial,
> but the BPF people seem to have managed it.

How can we guarantee that a built-in module loader security module is
always built-in ? What I'm insisting is that "warrant the freedom to load
loadable LSM modules without recompiling the whole kernel".

Sure, we can load LSM modules which were not built into distributor kernels
if we can recompile the whole kernel". But give me a break, that's a stupid
opinion for non-kernel developers. People won't replace distributor kernels
with rebuilt kernels only for enabling LSM modules which were not built into
distributor kernels.

Quoting from https://lkml.kernel.org/r/7f9ffd77-a329-ab13-857b-f8e34b2bfc77@schaufler-ca.com

  > I'm waiting to see what happens if eBPF security modules
  > become popular. I can easily see distributions turning the BPF LSM off.

Even if TOMOYO could be rewritten in eBPF (I don't think it is possible), how TOMOYO
can be loaded into distributor kernels if distributions turn the BPF LSM off ?

  > Before I go any further, I think that the loadable module manager LSM would be
  > very hard to get upstream.

Not only it will be very hard to get the loadable module manager LSM upstream,
it will be also very hard to keep the loadable module manager LSM enabled in
distributor kernels.

Again, how can we guarantee that a built-in module loader security module is
always built-in ?

What I'm insisting is that "warrant the freedom to load loadable LSM modules
without recompiling the whole kernel".

Adding EXPORT_SYMBOL_GPL(security_hook_heads) is the only way that can "allow
LSM modules which distributors cannot support to be legally loaded".

Any fixed-sized array like lsm_idlist[LSMID_ENTRIES] that defines max capacity
based on whether that LSM module is built-in will lock out loadable LSM modules.
Thus, I'm not happy with LSMID_ENTRIES.



On 2022/10/24 2:20, Casey Schaufler wrote:
> On 10/23/2022 3:10 AM, Tetsuo Handa wrote:
>> On 2022/10/23 16:27, Tetsuo Handa wrote:
>>> On 2022/10/21 8:42, Casey Schaufler wrote:
>>>> I will, on the other hand, listen to compelling arguments. It is not the
>>>> intention of this code to lock out loadable modules. If I thought it would
>>>> I would not have proposed it.
>>> This code is exactly for locking out loadable modules.
>>>
>> Imagine a situation where two individuals independently develop their own
>> web applications using the same identifier, and then their web applications
>> started working together with other web applications using that identifier.
>> When they published their web applications for public and wider use, a problem
>> that both web applications are already using the same identifier arises.
>> It is too late to reassign the identifier.
>>
>> The same trouble can happen with loadable LSM modules. Unless the upstream kernel
>> behaves as if a DNS registerer that assigns a unique domainname for whatever web
>> sites (regardless of whether a web site is for public or not), defining a permanent
>> constant for LSM module is a way towards locking out loadable LSM modules. And it
>> is well possible that a loadable LSM module wants to run on older kernels which
>> do not have LSM id defined yet.
>>
>> This "define LSM id as userspace visible constant" is more dangerous than just
>> reserving some space for future use. You are trying to control all IP addresses
>> for the sake of only in-tree LSM modules. No, no, no, please don't do that...
> 
> It's really no more dangerous than using the LSM name. What if two developers
> implement modules and both name it "belllapadula"? User space won't be able to
> tell the difference if they base behavior on the module name. That's one thing
> that a loadable module mechanism is going to need to address that a built-in
> mechanism doesn't. 

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).
Casey Schaufler Oct. 24, 2022, 4:37 p.m. UTC | #8
On 10/24/2022 8:13 AM, Tetsuo Handa wrote:
> On 2022/10/24 2:13, Casey Schaufler wrote:
>>> We won't be able to accept whatever LSM modules to upstream, and we won't
>>> be able to enable whatever LSM modules in distributor kernels.
>> A built in module loader security module would address this issue.
>> Getting such a module accepted upstream is not going to be trivial,
>> but the BPF people seem to have managed it.
> How can we guarantee that a built-in module loader security module is
> always built-in ?

You can't. That's up to the distribution. Just like BPF.

>  What I'm insisting is that "warrant the freedom to load
> loadable LSM modules without recompiling the whole kernel".

Since security modules are optional and the LSM infrastructure
itself is optional you can't ensure that any given kernel would
support a loadable security module.

> Sure, we can load LSM modules which were not built into distributor kernels
> if we can recompile the whole kernel". But give me a break, that's a stupid
> opinion for non-kernel developers. People won't replace distributor kernels
> with rebuilt kernels only for enabling LSM modules which were not built into
> distributor kernels.
>
> Quoting from https://lkml.kernel.org/r/7f9ffd77-a329-ab13-857b-f8e34b2bfc77@schaufler-ca.com
>
>   > I'm waiting to see what happens if eBPF security modules
>   > become popular. I can easily see distributions turning the BPF LSM off.
>
> Even if TOMOYO could be rewritten in eBPF (I don't think it is possible), how TOMOYO
> can be loaded into distributor kernels if distributions turn the BPF LSM off ?
>
>   > Before I go any further, I think that the loadable module manager LSM would be
>   > very hard to get upstream.
>
> Not only it will be very hard to get the loadable module manager LSM upstream,
> it will be also very hard to keep the loadable module manager LSM enabled in
> distributor kernels.

That is correct.

>
> Again, how can we guarantee that a built-in module loader security module is
> always built-in ?

Again, you can't. You can't guarantee that the LSM infrastructure is built in.

> What I'm insisting is that "warrant the freedom to load loadable LSM modules
> without recompiling the whole kernel".
>
> Adding EXPORT_SYMBOL_GPL(security_hook_heads) is the only way that can "allow
> LSM modules which distributors cannot support to be legally loaded".

I believe that I've identified an alternative. It isn't easy or cheap.

>
> Any fixed-sized array like lsm_idlist[LSMID_ENTRIES] that defines max capacity
> based on whether that LSM module is built-in will lock out loadable LSM modules.
> Thus, I'm not happy with LSMID_ENTRIES.
>
>
>
> On 2022/10/24 2:20, Casey Schaufler wrote:
>> On 10/23/2022 3:10 AM, Tetsuo Handa wrote:
>>> On 2022/10/23 16:27, Tetsuo Handa wrote:
>>>> On 2022/10/21 8:42, Casey Schaufler wrote:
>>>>> I will, on the other hand, listen to compelling arguments. It is not the
>>>>> intention of this code to lock out loadable modules. If I thought it would
>>>>> I would not have proposed it.
>>>> This code is exactly for locking out loadable modules.
>>>>
>>> Imagine a situation where two individuals independently develop their own
>>> web applications using the same identifier, and then their web applications
>>> started working together with other web applications using that identifier.
>>> When they published their web applications for public and wider use, a problem
>>> that both web applications are already using the same identifier arises.
>>> It is too late to reassign the identifier.
>>>
>>> The same trouble can happen with loadable LSM modules. Unless the upstream kernel
>>> behaves as if a DNS registerer that assigns a unique domainname for whatever web
>>> sites (regardless of whether a web site is for public or not), defining a permanent
>>> constant for LSM module is a way towards locking out loadable LSM modules. And it
>>> is well possible that a loadable LSM module wants to run on older kernels which
>>> do not have LSM id defined yet.
>>>
>>> This "define LSM id as userspace visible constant" is more dangerous than just
>>> reserving some space for future use. You are trying to control all IP addresses
>>> for the sake of only in-tree LSM modules. No, no, no, please don't do that...
>> It's really no more dangerous than using the LSM name. What if two developers
>> implement modules and both name it "belllapadula"? User space won't be able to
>> tell the difference if they base behavior on the module name. That's one thing
>> that a loadable module mechanism is going to need to address that a built-in
>> mechanism doesn't. 
> 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.
Tetsuo Handa Oct. 25, 2022, 9:59 a.m. UTC | #9
(Oops. I chose a wrong mail. Replying to intended mail.)

On 2022/10/25 1:37, Casey Schaufler wrote:
>>  What I'm insisting is that "warrant the freedom to load
>> loadable LSM modules without recompiling the whole kernel".
> 
> Since security modules are optional and the LSM infrastructure
> itself is optional you can't ensure that any given kernel would
> support a loadable security module.

Like I propose adding EXPORT_SYMBOL_GPL(security_hook_heads),
I'm not taking about distributors who choose CONFIG_SECURITY=n.

>> Adding EXPORT_SYMBOL_GPL(security_hook_heads) is the only way that can "allow
>> LSM modules which distributors cannot support to be legally loaded".
> 
> I believe that I've identified an alternative. It isn't easy or cheap.

No. You are just handwaving/postponing the problem using something unknown
that is not yet shown as a workable code. Anything that can be disabled via
kernel config option cannot be an alternative.

  Quoting from https://lkml.kernel.org/r/2225aec6-f0f3-d38e-ee3c-6139a7c25a37@I-love.SAKURA.ne.jp
  > Like Paul Moore said
  > 
  >   However, I will caution that it is becoming increasingly difficult for people
  >   to find time to review potential new LSMs so it may a while to attract sufficient
  >   comments and feedback.
  > 
  > , being unable to legally use loadable LSMs deprives of chances to develop/try
  > new LSMs, and makes LSM interface more and more unattractive. The consequence
  > would be "The LSM interface is dead. We will give up implementing as LSMs."

The biggest problem is that quite few developers show interest in loadable LSM modules.
How many developers responded to this topic? Once the ability to allow loadable LSM
modules is technically lost, nobody shall be able to revive it. You will be happy with
ignoring poor people.

You are already and completely trapped into "only in-tree and supported by distributors
is correct" crap.

> 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.

Then, I have to hate your idea of having fixed sized array.

Nacked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
diff mbox series

Patch

diff --git a/include/linux/security.h b/include/linux/security.h
index 7bd0c490703d..abdd151fc720 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -136,6 +136,23 @@  enum lockdown_reason {
 
 extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
 
+#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))
+
+extern int lsm_id;
+extern struct lsm_id *lsm_idlist[];
+
 /* These functions are in security/commoncap.c */
 extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
 		       int cap, unsigned int opts);
diff --git a/security/security.c b/security/security.c
index ff7fda4ffa43..14f22d9c9d84 100644
--- a/security/security.c
+++ b/security/security.c
@@ -28,6 +28,7 @@ 
 #include <linux/backing-dev.h>
 #include <linux/string.h>
 #include <linux/msg.h>
+#include <uapi/linux/lsm.h>
 #include <net/flow.h>
 
 #define MAX_LSM_EVM_XATTR	2
@@ -318,6 +319,12 @@  static void __init lsm_early_task(struct task_struct *task);
 
 static int lsm_append(const char *new, char **result);
 
+/*
+ * Current index to use while initializing the lsm id list.
+ */
+int lsm_id __lsm_ro_after_init;
+struct lsm_id *lsm_idlist[LSMID_ENTRIES] __lsm_ro_after_init;
+
 static void __init ordered_lsm_init(void)
 {
 	struct lsm_info **lsm;
@@ -362,6 +369,7 @@  static void __init ordered_lsm_init(void)
 	for (lsm = ordered_lsms; *lsm; lsm++)
 		initialize_lsm(*lsm);
 
+	init_debug("lsm count            = %d\n", lsm_id);
 	kfree(ordered_lsms);
 }
 
@@ -483,6 +491,16 @@  void __init security_add_hooks(struct security_hook_list *hooks, int count,
 {
 	int i;
 
+	/*
+	 * A security module may call security_add_hooks() more
+	 * than once. Landlock is one such case.
+	 */
+	if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid)
+		lsm_idlist[lsm_id++] = lsmid;
+
+	if (lsm_id > LSMID_ENTRIES)
+		panic("%s Too many LSMs registered.\n", __func__);
+
 	for (i = 0; i < count; i++) {
 		hooks[i].lsmid = lsmid;
 		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);