All of lore.kernel.org
 help / color / mirror / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: Jann Horn <jannh@google.com>
Cc: Todd Kjos <tkjos@google.com>,
	gregkh@linuxfoundation.org, arve@android.com, tkjos@android.com,
	maco@android.com, christian@brauner.io, jmorris@namei.org,
	serge@hallyn.com, paul@paul-moore.com,
	stephen.smalley.work@gmail.com, eparis@parisplace.org,
	keescook@chromium.org, jeffv@google.com, zohar@linux.ibm.com,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	joel@joelfernandes.org, kernel-team@android.com,
	stable@vger.kernel.org, Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH v2] binder: use cred instead of task for selinux checks
Date: Fri, 1 Oct 2021 13:10:41 -0700	[thread overview]
Message-ID: <f59c6e9f-2892-32da-62f8-8bbeec18ee4c@schaufler-ca.com> (raw)
In-Reply-To: <CAG48ez1SRau1Tnge5HVqxCFsNCizmnQLErqnC=eSeERv8jg-zQ@mail.gmail.com>

On 10/1/2021 12:50 PM, Jann Horn wrote:
> On Fri, Oct 1, 2021 at 9:36 PM Jann Horn <jannh@google.com> wrote:
>> On Fri, Oct 1, 2021 at 8:46 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> On 10/1/2021 10:55 AM, Todd Kjos wrote:
>>>> Save the struct cred associated with a binder process
>>>> at initial open to avoid potential race conditions
>>>> when converting to a security ID.
>>>>
>>>> Since binder was integrated with selinux, it has passed
>>>> 'struct task_struct' associated with the binder_proc
>>>> to represent the source and target of transactions.
>>>> The conversion of task to SID was then done in the hook
>>>> implementations. It turns out that there are race conditions
>>>> which can result in an incorrect security context being used.
>>> In the LSM stacking patch set I've been posting for a while
>>> (on version 29 now) I use information from the task structure
>>> to ensure that the security information passed via the binder
>>> interface is agreeable to both sides. Passing the cred will
>>> make it impossible to do this check. The task information
>>> required is not appropriate to have in the cred.
>> Why not? Why can't you put the security identity of the task into the creds?
> Ah, I get it now, you're concerned about different processes wanting
> to see security contexts formatted differently (e.g. printing the
> SELinux label vs printing the AppArmor label), right?

That is correct.

> But still, I don't think you can pull that information from the
> receiving task. Maybe the easiest solution would be to also store that
> in the creds? Or you'd have to manually grab that information when
> /dev/binder is opened.

I'm storing the information in the task security blob because that's
the appropriate scope. Today the LSM hook is given both task_struct's.
It's easy to compare to make sure the tasks are compatible. Adding the
information to the cred would be yet another case where the scope of
security information is wrong. 



  reply	other threads:[~2021-10-01 20:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 17:55 [PATCH v2] binder: use cred instead of task for selinux checks Todd Kjos
2021-10-01 18:46 ` Casey Schaufler
2021-10-01 19:36   ` Jann Horn
2021-10-01 19:50     ` Jann Horn
2021-10-01 20:10       ` Casey Schaufler [this message]
2021-10-01 22:58         ` Jann Horn
2021-10-04 16:19           ` Casey Schaufler
2021-10-04 22:28             ` Jann Horn
2021-10-04 23:37               ` Casey Schaufler
2021-10-05  0:26                 ` Jann Horn
2021-10-05 15:21                   ` Stephen Smalley
2021-10-05 16:49                     ` Todd Kjos
2021-10-05 17:12                       ` Stephen Smalley
2021-10-05 22:01                         ` Paul Moore
2021-10-05 16:59                     ` Casey Schaufler
2021-10-06  2:27                       ` Jann Horn
2021-10-06 19:49                         ` Casey Schaufler
2021-10-06  1:25                     ` Jann Horn
2021-10-05 13:31 ` Greg KH
2021-10-05 13:53   ` Paul Moore
2021-10-05 14:05     ` Greg KH

Reply instructions:

You may reply publicly 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=f59c6e9f-2892-32da-62f8-8bbeec18ee4c@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=arve@android.com \
    --cc=christian@brauner.io \
    --cc=devel@driverdev.osuosl.org \
    --cc=eparis@parisplace.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=jeffv@google.com \
    --cc=jmorris@namei.org \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=maco@android.com \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=stable@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tkjos@android.com \
    --cc=tkjos@google.com \
    --cc=zohar@linux.ibm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.