On 9/9/2020 6:19 AM, Stephen Smalley wrote: > On Tue, Sep 8, 2020 at 8:21 PM John Johansen > <john.johansen@xxxxxxxxxxxxx> 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@xxxxxxxxx> wrote: >>>>> On Sat, Sep 5, 2020 at 3:07 PM John Johansen >>>>> <john.johansen@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> wrote: >>>>>>>>> On 9/4/2020 2:53 PM, Paul Moore wrote: >>>>>>>>>> On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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?
