Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2] binder: use cred instead of task for selinux checks
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>
References: <20211001175521.3853257-1-tkjos@google.com>
 <c6a650e4-15e4-2943-f759-0e9577784c7a@schaufler-ca.com>
 <CAG48ez2tejBUXJGf0R9qpEiauL9-ABgkds6mZTQD7sZKLMdAAQ@mail.gmail.com>
 <CAG48ez1SRau1Tnge5HVqxCFsNCizmnQLErqnC=eSeERv8jg-zQ@mail.gmail.com>
From:   Casey Schaufler <casey@schaufler-ca.com>
Message-ID: <f59c6e9f-2892-32da-62f8-8bbeec18ee4c@schaufler-ca.com>
Date:   Fri, 1 Oct 2021 13:10:41 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <CAG48ez1SRau1Tnge5HVqxCFsNCizmnQLErqnC=eSeERv8jg-zQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
Content-Length: 1987
Precedence: bulk

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.=20