[PATCH v20 05/23] net: Prepare UDS for security module stacking

[Casey Schaufler]

On 9/8/2020 6:35 AM, Stephen Smalley wrote:
> On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
> <stephen.smalley.work at gmail.com> wrote:
>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen
>> <john.johansen at canonical.com> wrote:
>>> On 9/5/20 11:13 AM, Casey Schaufler wrote:
>>>> On 9/5/2020 6:25 AM, Paul Moore wrote:
>>>>> On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote:
>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
>>>>>>>> On 9/4/2020 1:08 PM, Paul Moore wrote:
>>>>> ...
>>>>>
>>>>>>> I understand the concerns you mention, they are all valid as far as
>>>>>>> I'm concerned, but I think we are going to get burned by this code as
>>>>>>> it currently stands.
>>>>>> Yes, I can see that. We're getting burned by the non-extensibility
>>>>>> of secids. It will take someone smarter than me to figure out how to
>>>>>> fit N secids into 32bits without danger of either failure or memory
>>>>>> allocation.
>>>>> Sooo what are the next steps here?  It sounds like there is some
>>>>> agreement that the currently proposed unix_skb_params approach is a
>>>>> problem, but it also sounds like you just want to merge it anyway?
>>>> There are real problems with all the approaches. This is by far the
>>>> least invasive of the lot. If this is acceptable for now I will commit
>>>> to including the dynamic allocation version in the full stacking
>>>> (e.g. Smack + SELinux) stage. If it isn't, well, this stage is going
>>>> to take even longer than it already has. Sigh.
>>>>
>>>>
>>>>> I was sorta hoping for something a bit better.
>>>> I will be looking at alternatives. I am very much open to suggestions.
>>>> I'm not even 100% convinced that Stephen's objections to my separate
>>>> allocation strategy outweigh its advantages. If you have an opinion on
>>>> that, I'd love to hear it.
>>>>
>>> fwiw I prefer the separate allocation strategy, but as you have already
>>> said it trading off one set of problems for another. I would rather see
>>> this move forward and one set of trade offs isn't significantly worse
>>> than the other to me so, either wfm.
>> I remain unclear that AppArmor needs this patch at all even when
>> support for SO_PEERSEC lands.
>> Contrary to the patch description, it is about supporting SCM_SECURITY
>> for datagram not SO_PEERSEC.  And I don't know of any actual users of
>> SCM_SECURITY even for SELinux, just SO_PEERSEC.
> I remembered that systemd once tried using SCM_SECURITY but that was a
> bug since systemd was using it with stream sockets and that wasn't
> supported by the kernel at the time,
> https://bugzilla.redhat.com/show_bug.cgi?id=1224211, so systemd
> switched over to using SO_PEERSEC.  Subsequently I did fix
> SCM_SECURITY to work with stream sockets via kernel commit
> 37a9a8df8ce9de6ea73349c9ac8bdf6ba4ec4f70 but SO_PEERSEC is still
> preferred.  Looking around, I see that there is still one usage of
> SCM_SECURITY in systemd-journald but it doesn't seem to be required
> (if provided, journald will pass the label along but nothing seems to
> depend on it AFAICT).  In any event, I don't believe this patch is
> needed to support stacking AppArmor.

Stephen is, as is so often the case, correct. AppArmor has a stub
socket_getpeersec_dgram() that gets removed in patch 23. If I remove
it earlier and throw in a touch of scaffolding for secid_to_secctx()
we can leave the secid as is for now. This can't be the final solution
as AppArmor will be using the hook someday and we still have the all
modules case to worry about for the next phase. It also assumes that
The BPF module isn't going to suddenly sprout a security context.