SELinux Archive on lore.kernel.org
 help / color / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Casey Schaufler <casey@schaufler-ca.com>,
	selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
	Tejun Heo <tj@kernel.org>,
	linux-fsdevel@vger.kernel.org, cgroups@vger.kernel.org
Subject: Re: [PATCH v2 0/3] Allow initializing the kernfs node's secctx based on its parent
Date: Thu, 10 Jan 2019 21:20:34 -0500
Message-ID: <CAHC9VhRWfaju_pL8US=gyTn-qu+iF1Nc_71OqGMiJSY6bOwg5Q@mail.gmail.com> (raw)
In-Reply-To: <2612c523-084a-7652-58ca-9e062b80138e@tycho.nsa.gov>

On Thu, Jan 10, 2019 at 2:36 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/10/19 12:54 PM, Casey Schaufler wrote:
> > Resending after email configuration repair.
> >
> > On 1/10/2019 6:15 AM, Stephen Smalley wrote:
> >> On 1/9/19 5:03 PM, Casey Schaufler wrote:
> >>> On 1/9/2019 12:37 PM, Stephen Smalley wrote:
> >>>> On 1/9/19 12:19 PM, Casey Schaufler wrote:
> >>>>> On 1/9/2019 8:28 AM, Ondrej Mosnacek wrote:
> >>>>>> Changes in v2:
> >>>>>> - add docstring for the new hook in union security_list_options
> >>>>>> - initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
> >>>>>>      implemented
> >>>>>> v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@redhat.com/T/
> >>>>>>
> >>>>>> This series adds a new security hook that allows to initialize the security
> >>>>>> context of kernfs properly, taking into account the parent context. Kernfs
> >>>>>> nodes require special handling here, since they are not bound to specific
> >>>>>> inodes/superblocks, but instead represent the backing tree structure that
> >>>>>> is used to build the VFS tree when the kernfs tree is mounted.
> >>>>>>
> >>>>>> The kernfs nodes initially do not store any security context and rely on
> >>>>>> the LSM to assign some default context to inodes created over them.
> >>>>>
> >>>>> This seems like a bug in kernfs. Why doesn't kernfs adhere to the usual
> >>>>> and expected filesystem behavior?
> >>>>
> >>>> sysfs / kernfs didn't support xattrs at all when we first added support for setting security contexts to it, so originally all sysfs / kernfs inodes had a single security context, and we only required separate storage for the inodes that were explicitly labeled by userspace.
> >>>>
> >>>> Later kernfs grew support for trusted.* xattrs using simple_xattrs but the existing security.* support was left mostly unchanged.
> >>>
> >>> OK, so as I said, this seems like a bug in kernfs.
> >>>
> >>>>
> >>>>>
> >>>>>> Kernfs
> >>>>>> inodes, however, allow setting an explicit context via the *setxattr(2)
> >>>>>> syscalls, in which case the context is stored inside the kernfs node's
> >>>>>> metadata.
> >>>>>>
> >>>>>> SELinux (and possibly other LSMs) initialize the context of newly created
> >>>>>> FS objects based on the parent object's context (usually the child inherits
> >>>>>> the parent's context, unless the policy dictates otherwise).
> >>>>>
> >>>>> An LSM might use information about the parent other than the "context".
> >>>>> Smack, for example, uses an attribute SMACK64TRANSMUTE from the parent
> >>>>> to determine whether the Smack label of the new object should be taken
> >>>>> from the parent or the process. Passing the "context" of the parent is
> >>>>> insufficient for Smack.
> >>>>
> >>>> IIUC, this would involve switching the handling of security.* xattrs in kernfs over to use simple_xattrs too (so that we can store multiple such attributes), and then pass the entire simple_xattrs list or at least anything with a security.* prefix when initializing a new node or refreshing an existing inode.  Then the security module could extract any security.* attributes of interest for use in determining the label of new inodes and in refreshing the label of an inode.
> >>>
> >>> Right. But I'll point out that there is nothing to prevent an
> >>> LSM from using inode information outside of the xattrs (e.g. uids)
> >>> to determine the security state it wants to give a new object.
> >>
> >> If that's a real concern, the hook could pass the ia_iattr structure in addition to the simple_xattrs list and the security module could use any inode attributes it likes in making the decision.  Effectively it would be passing the entire kernfs_iattrs structure, but probably not directly since that definition is presently private to kernfs.
> >
> > Yes, it's a real concern. And no, just passing all of the kernfs internal data
> > out in j-random formats does not pass muster. Al Viro was commenting the other
> > day on how bad the LSM infrastructure interfaces are. The original proposal here
> > is already big, cluttered and inadequate. Adding more to it to make up for its
> > shortcomings should be sending up red flags
>
> I don't quite see how the original patch set or hook can be called big
> and cluttered.  Switching the handling of security xattrs in kernfs to
> use simple_xattrs (a natural and seemingly straightforward cleanup) and
> passing the entire simple_xattrs list to the hook interface would allow
> you to support SMACK64TRANSMUTE, which was the one actual inadequacy you
> identified.  You claim that someone might need/want the parent uid/gid
> too, but there are no in-tree security modules that do so nor any
> submitted AFAIK, and if that situation arises, all we need to do to
> support it is to add the iattrs.  Obviously they can all be wrapped up
> in some larger structure if desired. At that point the security modules
> would have access to all of the inode attributes supported by kernfs.

I'm with Stephen on this; if Ondrej changes it over to simple_xattrs
as described above so that Smack would have what it needs, I don't see
why we should hold off on this.

Everything we are talking about is a kernel internal issue, we can
change it as needed to take into account new LSMs or new functionality
in existing LSMs.

Ondrej, a gentle reminder that it would be nice to have a simple
selinux-testsuite test to make sure we are labeling
kernfs-based/cgroup files correctly.

-- 
paul moore
www.paul-moore.com

  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 16:28 Ondrej Mosnacek
2019-01-09 16:28 ` [PATCH v2 1/3] LSM: Add new hook for generic node initialization Ondrej Mosnacek
2019-01-09 17:08   ` Casey Schaufler
2019-01-11  1:57     ` Paul Moore
2019-01-11 18:30       ` Casey Schaufler
2019-01-14  9:01       ` Ondrej Mosnacek
2019-01-09 16:28 ` [PATCH v2 2/3] selinux: Implement the object_init_security hook Ondrej Mosnacek
2019-01-09 16:28 ` [PATCH v2 3/3] kernfs: Initialize security of newly created nodes Ondrej Mosnacek
2019-01-11 20:52   ` Tejun Heo
2019-01-09 17:19 ` [PATCH v2 0/3] Allow initializing the kernfs node's secctx based on its parent Casey Schaufler
2019-01-09 20:37   ` Stephen Smalley
2019-01-09 22:03     ` Casey Schaufler
2019-01-10 14:15       ` Stephen Smalley
2019-01-10 17:54         ` Casey Schaufler
2019-01-10 19:37           ` Stephen Smalley
2019-01-11  2:20             ` Paul Moore [this message]
2019-01-14  9:01               ` Ondrej Mosnacek
2019-01-11 18:22             ` Casey Schaufler
2019-01-14  9:01           ` Ondrej Mosnacek
2019-01-22  8:49             ` Ondrej Mosnacek
2019-01-22 14:17               ` Stephen Smalley
2019-01-22 15:26                 ` Stephen Smalley

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHC9VhRWfaju_pL8US=gyTn-qu+iF1Nc_71OqGMiJSY6bOwg5Q@mail.gmail.com' \
    --to=paul@paul-moore.com \
    --cc=casey@schaufler-ca.com \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org
	public-inbox-index selinux

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git