Re: [PATCH] security: fix the default value of secid_to_secctx hook

[Casey Schaufler]

On 5/18/2020 2:43 PM, Schaufler, Casey wrote:
>> -----Original Message-----
>> From: linux-kernel-owner@xxxxxxxxxxxxxxx <linux-kernel-
>> owner@xxxxxxxxxxxxxxx> On Behalf Of Arnd Bergmann
>> Sent: Saturday, May 16, 2020 1:05 AM
>> To: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
>> Cc: James Morris <jamorris@xxxxxxxxxxxxxxxxxxx>; Anders Roxell
>> <anders.roxell@xxxxxxxxxx>; Alexei Starovoitov <ast@xxxxxxxxxx>; Daniel
>> Borkmann <daniel@xxxxxxxxxxxxx>; LKML <linux-kernel@xxxxxxxxxxxxxxx>;
>> Network Development <netdev@xxxxxxxxxxxxxxx>; bpf
>> <bpf@xxxxxxxxxxxxxxx>
>> 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@xxxxxxxxx> wrote:
>>> On Thu, May 14, 2020 at 12:47 PM Alexei Starovoitov
>>> <alexei.starovoitov@xxxxxxxxx> wrote:
>>>> On Thu, May 14, 2020 at 12:43 PM James Morris
>>>> <jamorris@xxxxxxxxxxxxxxxxxxx> 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