netdev - Re: [PATCH] security: fix the default value of secid_to

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] [day] [month] [year] [list]

Message-ID: <9d0eb6c6-803a-ff3a-5603-9ad6d9edfc00@schaufler-ca.com>
Date:   Mon, 18 May 2020 15:02:09 -0700
From:   Casey Schaufler <casey@...aufler-ca.com>
To:     "Schaufler, Casey" <casey.schaufler@...el.com>,
        Arnd Bergmann <arnd@...db.de>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     James Morris <jamorris@...ux.microsoft.com>,
        Anders Roxell <anders.roxell@...aro.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        LKML <linux-kernel@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        linux-security-module <linux-security-module@...r.kernel.org>,
        Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH] security: fix the default value of secid_to_secctx hook

On 5/18/2020 2:43 PM, Schaufler, Casey wrote:
>> -----Original Message-----
>> From: linux-kernel-owner@...r.kernel.org <linux-kernel-
>> owner@...r.kernel.org> On Behalf Of Arnd Bergmann
>> Sent: Saturday, May 16, 2020 1:05 AM
>> To: Alexei Starovoitov <alexei.starovoitov@...il.com>
>> Cc: James Morris <jamorris@...ux.microsoft.com>; Anders Roxell
>> <anders.roxell@...aro.org>; Alexei Starovoitov <ast@...nel.org>; Daniel
>> Borkmann <daniel@...earbox.net>; LKML <linux-kernel@...r.kernel.org>;
>> Network Development <netdev@...r.kernel.org>; bpf
>> <bpf@...r.kernel.org>
>> Subject: Re: [PATCH] security: fix the default value of secid_to_secctx hook
> I would *really* appreciate it if discussions about the LSM infrastructure
> where done on the linux-security-module mail list. (added to CC).
>
>> On Sat, May 16, 2020 at 1:29 AM Alexei Starovoitov
>> <alexei.starovoitov@...il.com> wrote:
>>> On Thu, May 14, 2020 at 12:47 PM Alexei Starovoitov
>>> <alexei.starovoitov@...il.com> wrote:
>>>> On Thu, May 14, 2020 at 12:43 PM James Morris
>>>> <jamorris@...ux.microsoft.com> wrote:
>>>>> On Wed, 13 May 2020, Alexei Starovoitov wrote:
>>>>>
>>>>>> James,
>>>>>>
>>>>>> since you took the previous similar patch are you going to pick this
>>>>>> one up as well?
>>>>>> Or we can route it via bpf tree to Linus asap.
>>>>> Routing via your tree is fine.
>>>> Perfect.
>>>> Applied to bpf tree. Thanks everyone.
>>> Looks like it was a wrong fix.
>>> It breaks audit like this:
>>> sudo auditctl -e 0
>>> [   88.400296] audit: error in audit_log_task_context
>>> [   88.400976] audit: error in audit_log_task_context
>>> [   88.401597] audit: type=1305 audit(1589584951.198:89): op=set
>>> audit_enabled=0 old=1 auid=0 ses=1 res=0
>>> [   88.402691] audit: type=1300 audit(1589584951.198:89):
>>> arch=c000003e syscall=44 success=yes exit=52 a0=3 a1=7ffe42a37400
>>> a2=34 a3=0 items=0 ppid=2250 pid=2251 auid=0 uid=0 gid=0 euid=0 suid=0
>>> fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 se)
>>> [   88.405587] audit: type=1327 audit(1589584951.198:89):
>>> proctitle=617564697463746C002D650030
>>> Error sending enable request (Operation not supported)
>>>
>>> when CONFIG_LSM= has "bpf" in it.
>> Do you have more than one LSM enabled? It looks like
>> the problem with security_secid_to_secctx() is now that it
>> returns an error if any of the LSMs fail and the caller expects
>> it to succeed if at least one of them sets the secdata pointer.

security_secid_to_secctx() is not currently stackable (I'm
looking at 5.7-rc6) even for this simple case. call_int_hook()
does bail-on-fail and will try all hooks registered, looking for
a failure.

You need to replace the call_int_hook() with an explicit
hlist_for_each_entry(), as is done in security_inode_getsecurity().

>>
>> The problem earlier was that the call succeeded even though
>> no LSM had set the pointer.
>>
>> What is the behavior we actually expect from this function if
>> multiple LSM are loaded?
>>
>>        Arnd