lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 20 Oct 2022 16:42:04 -0700
From:   Casey Schaufler <casey@...aufler-ca.com>
To:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        casey.schaufler@...el.com, paul@...l-moore.com,
        linux-security-module@...r.kernel.org
Cc:     linux-audit@...hat.com, jmorris@...ei.org, selinux@...r.kernel.org,
        keescook@...omium.org, john.johansen@...onical.com,
        stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org,
        casey@...aufler-ca.com
Subject: Re: [PATCH v38 04/39] LSM: Maintain a table of LSM attribute data

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);

Powered by blists - more mailing lists