linux-kernel - Re: [PATCH v2] binder: use cred instead of task for selinux checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Oct 2021 13:10:41 -0700
From:   Casey Schaufler <casey@...aufler-ca.com>
To:     Jann Horn <jannh@...gle.com>
Cc:     Todd Kjos <tkjos@...gle.com>, gregkh@...uxfoundation.org,
        arve@...roid.com, tkjos@...roid.com, maco@...roid.com,
        christian@...uner.io, jmorris@...ei.org, serge@...lyn.com,
        paul@...l-moore.com, stephen.smalley.work@...il.com,
        eparis@...isplace.org, keescook@...omium.org, jeffv@...gle.com,
        zohar@...ux.ibm.com, linux-security-module@...r.kernel.org,
        selinux@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org, joel@...lfernandes.org,
        kernel-team@...roid.com, stable@...r.kernel.org,
        Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v2] binder: use cred instead of task for selinux checks

On 10/1/2021 12:50 PM, Jann Horn wrote:
> On Fri, Oct 1, 2021 at 9:36 PM Jann Horn <jannh@...gle.com> wrote:
>> On Fri, Oct 1, 2021 at 8:46 PM Casey Schaufler <casey@...aufler-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.