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

[Casey Schaufler]

On 9/9/2020 6:19 AM, Stephen Smalley wrote:
> On Tue, Sep 8, 2020 at 8:21 PM John Johansen
> <john.johansen at canonical.com> wrote:
>> On 9/8/20 4:37 PM, Casey Schaufler wrote:
>>> 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
>> right but as I said before this is coming, I have been playing with
>> it and have code. So the series doesn't need it today but sooner than
>> later it will be needed

Is sooner like 5.10, or 5.15? It matters.

> I don't understand why.  Is there a userspace component that relies on
> SCM_SECURITY today for anything real (more than just blindly passing
> it along and maybe writing to a log somewhere)?  And this doesn't
> provide support for a composite SCM_SECURITY or SCM_CONTEXT, so it
> doesn't really solve the stacking problem for it anyway.  What am I
> missing?  Why do you care about this patch?