[PATCH 1/3] selinux: make dentry_init_security() return security module name

[Yan, Zheng]

On Wed, Sep 12, 2018 at 1:23 AM Casey Schaufler <casey at schaufler-ca.com> wrote:
>
> On 9/9/2018 8:06 PM, Yan, Zheng wrote:
> > On Sat, Sep 8, 2018 at 4:31 AM Casey Schaufler <casey at schaufler-ca.com> wrote:
> >> On 9/7/2018 1:31 AM, Yan, Zheng wrote:
> >>> On Thu, Sep 6, 2018 at 11:42 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> >>>> On 9/6/2018 8:08 AM, Jeff Layton wrote:
> >>>>> On Tue, 2018-06-26 at 16:43 +0800, Yan, Zheng wrote:
> >>>>>> This is preparation for CephFS security label. CephFS's implementation uses
> >>>>>> dentry_init_security() to get security context before inode is created,
> >>>>>> then sends open/mkdir/mknod request to MDS, together with security xattr
> >>>>>> "security.<security module name>"
> >>>> Please excuse my late entry into this review.
> >>>>
> >>>> First, *Do not prefix general LSM interface work with "selinux:"*
> >>>> You should only use that prefix when you're dealing with the
> >>>> internals of SELinux. Use "LSM:" if you are changing LSM interfaces.
> >>>> If you are changing security.h or lsm_hooks.h you are almost
> >>>> certainly changing LSM interfaces.
> >>>>
> >>>> Because you prefixed this work with "selinux:" I pretty much
> >>>> ignored it. Please don't do that again.
> >>>>
> >>>> Why aren't you using security_inode_getsecctx, like kernfs and nfs?
> >>>>
> >>> Sorry, I don't understand why you mention security_inode_getsecctx().
> >>> The selinux_dentry_init_security is called for new inode, I don't how
> >>> security_inode_getsecctx() can help.
> >> I didn't do a great job describing my issue. Let me try again.
> >>
> >> There is no way you should need to know the name of the security
> >> module (e.g. SELinux, Smack) in the CephFS code. Nor should you be
> >> assuming that there is only one security attribute, nor that it is
> >> named "security.<security module name>".  Sure, that's true for
> >> SELinux, but SELinux isn't the only security module in town.
> >>
> >> Other filesystems don't need anything like this. Why do you?
> >>
> > What I want is a way to get security context before inode is created.
>
> What are you doing differently from the other filesystems?
> Why doesn't the security_d_instantiate mechanism work for you?
>

Because I want to minimize overhead of setting security xattrs for new
inode. When creating new file/dir, cephfs client sends one request to
MDS. The request can carry xattrs. MDS can create new file/dir and
setting xattrs at the same time. If cephfs follows other filesystems'
code, cephfs client needs to send to two request to MDS for each new
file/dir, one for create, one for setting security xattrs.

> > The closest interface is security_dentry_init_security(). Currently
> > the only NFS uses security_dentry_init_security() and only selinux has
> > dentry_init_security callback. NFS store selinux context in its
> > security label (NFS inode can only have one security label, security
> > label can only store one security context). SO current NFS security
> > label implementation can not support multiple security modules.
>
> That's correct. The labeled NFS is a very specific mechanism. It
> does not do anything except Mandatory Access Control labeling. It
> cannot be used for any other attributes. I am not a fan of the
> labeled NFS implementation, and was vocal about it at the time.
>
> > I don't want to limit cephfs to single security module. So I make
> > cephfs store security context in security.<security module name>
> > xattr.
>
> Please look at the mechanism used in kernfs.
>

kernfs' mechanism is simple. It just provides
kernfs_trusted_xattr_handler.  It does not initialize security context
for new file/dir. I don't see how to adapt its mechanism for cephfs.

Regards
Yan, Zheng

> > When multiple security modules work is done in the future,
> > cephfs can easily supports it. (introduce a new
> > security_dentry_init_security() which return security contexts for all
> > enabled security modules)
>
> That is easier said than done. I've proposed several mechanisms,
> none of which are especially satisfactory. The best mechanism I see
> is the one used in kernfs, which is why I suggest you use that.
>