Re: [PATCH v2 1/3] LSM: Add new hook for generic node initialization

From: Ondrej Mosnacek <omosnace@redhat.com>
To: Paul Moore <paul@paul-moore.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>,
	selinux@vger.kernel.org, Stephen Smalley <sds@tycho.nsa.gov>,
	Linux Security Module list 
	<linux-security-module@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Tejun Heo <tj@kernel.org>,
	linux-fsdevel@vger.kernel.org, cgroups@vger.kernel.org
Subject: Re: [PATCH v2 1/3] LSM: Add new hook for generic node initialization
Date: Mon, 14 Jan 2019 10:01:56 +0100	[thread overview]
Message-ID: <CAFqZXNse5Vijo_Vicqs0x=9sbO95MzssBKrAtyW9u=OP-ESK7A@mail.gmail.com> (raw)
In-Reply-To: <CAHC9VhS6pGycbT1GET5CHjxgYCDu1KCrcPtcF0n==1oH15kAVA@mail.gmail.com>

On Fri, Jan 11, 2019 at 2:57 AM Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Jan 9, 2019 at 12:08 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > On 1/9/2019 8:28 AM, Ondrej Mosnacek wrote:
> > > This patch introduces a new security hook that is intended for
> > > initializing the security data for newly created pseudo filesystem
> > > objects (such as kernfs nodes) that provide a way of storing a
> > > non-default security context, but need to operate independently from
> > > mounts.
> > >
> > > The main motivation is to allow kernfs nodes to inherit the context of
> > > the parent under SELinux, similar to the behavior of
> > > security_inode_init_security(). Other LSMs may implement their own logic
> > > for handling the creation of new nodes.
> > >
> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > ---
> > >  include/linux/lsm_hooks.h | 30 ++++++++++++++++++++++++++++++
> > >  include/linux/security.h  | 14 ++++++++++++++
> > >  security/security.c       | 10 ++++++++++
> > >  3 files changed, 54 insertions(+)
> > >
> > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > > index aaeb7fa24dc4..3a2399d7721f 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -429,6 +429,31 @@
> > >   *   to abort the copy up. Note that the caller is responsible for reading
> > >   *   and writing the xattrs as this hook is merely a filter.
> > >   *
> > > + * Security hooks for special file-like objects
> > > + *
> > > + * @object_init_security:
> >
> > I don't like the name. There are too many things that are "objects"
> > for this to be meaningful. I also dislike seeing names like
> > security_object_init_security. How about init_from_parent? If there's
> > never a chance that it will be used anywhere but with kernfs, it could
> > be kernfs_node_init. The existing set of hook names are sufficiently
> > confusing without adding to the mystery.

TBH, I wasn't completely satisfied with the name either, I was hoping
to get some better suggestions along the way.

>
> I like the naming similarity with inode_init_security(), that seems
> helpful.  Although I somewhat understand you concern about the generic
> "object".  Could you live with kernfs_init_security()?  If another fs
> adopts it, we could always changing the name later if needed.

I decided to leave out kernfs out of the name, since it ended up
looking quite generic (nothing particularly kernfs-specific in the
argument list). On second thought, I'm starting to prefer
kernfs_init_security(), considering the very low likelihood that it
would be useful elsewhere and that it will likely end up having some
more arguments tailored specifically for kernfs in the next revisions
(w.r.t. the cover letter discussion).

--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.