Re: [PATCH v20 05/23] net: Prepare UDS for security module stacking
On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
<stephen.smalley.work(a)gmail.com> wrote:
On Sat, Sep 5, 2020 at 3:07 PM John Johansen
<john.johansen(a)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(a)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(a)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.