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

From: Casey Schaufler <casey@schaufler-ca.com>
To: "Schaufler, Casey" <casey.schaufler@intel.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: James Morris <jamorris@linux.microsoft.com>,
	Anders Roxell <anders.roxell@linaro.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	LKML <linux-kernel@vger.kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH] security: fix the default value of secid_to_secctx hook
Date: Mon, 18 May 2020 15:02:09 -0700	[thread overview]
Message-ID: <9d0eb6c6-803a-ff3a-5603-9ad6d9edfc00@schaufler-ca.com> (raw)
In-Reply-To: <CY4PR11MB17990BA2B00B4CEB167057C9FDB80@CY4PR11MB1799.namprd11.prod.outlook.com>

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