On Tue, Sep 8, 2020 at 8:21 PM John Johansen
[off-list ref] wrote:

On 9/8/20 4:37 PM, Casey Schaufler wrote:

quoted

On 9/8/2020 6:35 AM, Stephen Smalley wrote:

quoted

On Mon, Sep 7, 2020 at 9:28 PM Stephen Smalley
[off-list ref] wrote:

quoted

On Sat, Sep 5, 2020 at 3:07 PM John Johansen
[off-list ref] wrote:

quoted

On 9/5/20 11:13 AM, Casey Schaufler wrote:

quoted

On 9/5/2020 6:25 AM, Paul Moore wrote:

quoted

On Fri, Sep 4, 2020 at 7:58 PM Casey Schaufler [off-list ref] wrote:

quoted

On 9/4/2020 2:53 PM, Paul Moore wrote:

quoted

On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler [off-list ref] wrote:

quoted

On 9/4/2020 1:08 PM, Paul Moore wrote:

...

quoted

quoted

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.

quoted

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

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?