linux-kernel - Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

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]

Message-ID: <8491f76d5c8923f35216f55c030a68f478a0325a.camel@linux.ibm.com>
Date:   Tue, 07 Dec 2021 10:40:54 -0500
From:   James Bottomley <jejb@...ux.ibm.com>
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     Stefan Berger <stefanb@...ux.ibm.com>,
        linux-integrity@...r.kernel.org, zohar@...ux.ibm.com,
        serge@...lyn.com, containers@...ts.linux.dev,
        dmitry.kasatkin@...il.com, ebiederm@...ssion.com,
        krzysztof.struczynski@...wei.com, roberto.sassu@...wei.com,
        mpeters@...hat.com, lhinds@...hat.com, lsturman@...hat.com,
        puiterwi@...hat.com, jamjoom@...ibm.com,
        linux-kernel@...r.kernel.org, paul@...l-moore.com, rgb@...hat.com,
        linux-security-module@...r.kernel.org, jmorris@...ei.org
Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Tue, 2021-12-07 at 10:16 -0500, James Bottomley wrote:
> On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
> > On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley wrote:
[...]
> > >  static int securityfs_fill_super(struct super_block *sb, struct
> > > fs_context *fc)
> > >  {
> > >  	static const struct tree_descr files[] = {{""}};
> > >  	int error;
> > > +	struct user_namespace *ns = fc->user_ns;
> > >  
> > >  	error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> > >  	if (error)
> > >  		return error;
> > >  
> > > +	ns->securityfs_root = dget(sb->s_root);
> > > +
> > >  	sb->s_op = &securityfs_super_operations;
> > >  
> > > +	if (ns != &init_user_ns)
> > > +		blocking_notifier_call_chain(&securityfs_ns_notifier,
> > > +					     SECURITYFS_NS_ADD, ns);
> > 
> > I would propose not to use the notifier logic. While it might be
> > nifty it's over-engineered in my opinion.
> 
> The reason for a notifier is that this current patch set only
> namespaces ima, but we also have integrity and evm to do.  Plus, as
> Casey said, we might get apparmour and selinux.  Since each of those
> will also want to add entries in fill_super, the notifier mechanism
> seemed fairly tailor made for this.  The alternative is to have a
> load of 
> 
> #if CONFIG_securityfeature
> callback()
> #endif
> 
> Inside securityfs_fill_super which is a bit inelegant.
> 
> >  The dentry stashing in struct user_namespace currently serves the
> > purpose to make it retrievable in ima_fs_ns_init(). That doesn't
> > justify its existence imho.
> 
> I can thread the root as part of the callback.  I think I can still
> use the standard securityfs calls because the only reason for the
> dentry in the namespace is so the callee can pass NULL and have the
> dentry created at the top level.  We can insist in the namespaced use
> case that the callee always pass in the dentry, even for the top
> level.
> 
> > There is one central place were all users of namespaced securityfs
> > can create the files that they need to and that is in
> > securityfs_fill_super(). (If you want to make that more obvious
> > then give it a subdirectory securityfs and move inode.c in there.)
> 
> Right, that's what the patch does.
> 
> > We simply will expect users to add:
> > 
> > ima_init_securityfs()
> > mylsm_init_securityfs()
> 
> Yes, plus all the #ifdefs because securityfs can exist independently
> of each of the features.  We can hide the ifdefs in the header files
> and make the functions static do nothing if not defined, but the
> ifdeffery has to live somewhere.

Actually, I've got a much better reason: securityfs is a bool; all the
other LSMs and IMA are tristates.  We can't call module init functions
from core code, it has to be done by something like a notifier.

James