Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling

From: Paul Moore <paul@paul-moore.com>
To: Stephen Smalley <sds@tycho.nsa.gov>, takondra@cisco.com
Cc: Eric Paris <eparis@parisplace.org>,
	selinux@tycho.nsa.gov, linux-kernel@vger.kernel.org,
	xe-linux-external@cisco.com
Subject: Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
Date: Mon, 24 Sep 2018 23:46:57 -0400	[thread overview]
Message-ID: <CAHC9VhRorkfqRTsssx_eGMy+fJQtoJew7f5i24UBkZ6tGJ5CUw@mail.gmail.com> (raw)
In-Reply-To: <66ec16b0-a335-d9ef-deb3-ef391cdd66e0@tycho.nsa.gov>

On Fri, Sep 21, 2018 at 10:39 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 09/20/2018 06:59 PM, Taras Kondratiuk wrote:
> > Quoting Stephen Smalley (2018-09-20 07:49:12)
> >> On 09/19/2018 10:41 PM, Taras Kondratiuk wrote:
> >>> Quoting Stephen Smalley (2018-09-19 12:00:33)
> >>>> On 09/19/2018 12:52 PM, Taras Kondratiuk wrote:

...

> > IMO it would be more consistent if defcontext cover all "unlabeled"
> > groups. It seems unlikely to me that somebody who currently uses
> > defcontext can somehow rely on mapping invalid labels to unlabeled
> > instead of default context.
>
> Yes, and that seems more consistent with the current documentation in
> the mount man page for defcontext=.
>
> I'd be inclined to change selinux_inode_notifysecctx() to call
> security_context_to_sid_default() directly instead of using
> selinux_inode_setsecurity() and change security_context_to_sid_core()
> and sidtab_search_core() as suggested above to save and use the def_sid
> instead of SECINITSID_UNLABELED always (initializing the context def_sid
> to SECINITSID_UNLABELED as the default).  selinux_inode_setsecurity() we
> should leave unchanged, or if we change it at all, it should be more
> like the handling in selinux_inode_setxattr().  The notifysecctx hook is
> invoked by the filesystem to notify the security module of the file's
> existing security context, so in that case we always want the _default
> behavior, whereas the setsecurity hook is invoked by the vfs or the
> filesystem to set the security context of a file to a new value, so in
> that case we would only use the _force interface if the caller had
> CAP_MAC_ADMIN.
>
> Paul, what say you?  NB This would be a user-visible behavior change for
> mounts specifying defcontext= on xattr filesystems; files with invalid
> contexts will then show up with the defcontext value instead of the
> unlabeled context.  If that's too risky, then we'd need a flag or
> something to security_context_to_sid_default() to distinguish the
> behaviors and only set it when called from selinux_inode_notifysecctx().

Visible changes like this are always worrisome, even though I think it
is safe to assume that the defcontext option is not widely used.  I'd
feel much better if this change was opt-in.

Which brings about it's own problems.  We have the policy capability
functionality, but that is likely a poor fit for this as the policy
capabilities are usually controlled by the Linux distribution while
the mount options are set by the system's administrator when the
filesystem is mounted.  We could add a toggle somewhere in selinuxfs,
but I really dislike that idea, and would prefer to find a different
solution if possible.  I'm not sure how much flak we would get for
introducing a new mount option, but perhaps that is the best way to
handle this: defcontext would continue to behave as it does now, but
new option X would behave as mentioned in this thread.

Thoughts?

-- 
paul moore
www.paul-moore.com