Re: LSM stacking in next for 6.1?

On 9/7/2022 8:57 PM, Paul Moore wrote: > On Wed, Sep 7, 2022 at 7:53 PM Casey Schaufler <casey(a)schaufler-ca.com&gt; wrote: >> On 9/7/2022 4:27 PM, Paul Moore wrote: > .. > >>> I >>> just want an interface that is clearly defined, has reasonable >>> capacity to be extended in the future as needed, and is easy(ish) to >>> use and support over extended periods of time (both from a kernel and >>> userspace perspective). >>> >>> The "smack_label\0apparmor_label\0selinux_label\0future_lsm_label\0" >>> string interface is none of these things. That wasn't the proposal. The proposal was "smack\0smack_label\0apparmor\0apparmor_label\0future_lsm\0future_lsm_label\0" >> In this we disagree .... > I think we can both agree that there is a subjective aspect to this > argument and it may be that we never reach agreement on the "best" > approach, if there even is such a thing. What I am trying to do here > is describe a path that would allow me to be more comfortable merging > the LSM stacking functionality; I don't think you've had a clearly > defined path, of any sort, towards getting these patches merged, which > is a problem and I suspect the source of some of the frustration. My > comments, as objectionable as you may find them to be, are intended to > help you move forward with these patches. OK. Let's get'er done. >>> It is not clearly defined >>> as it requires other interfaces to associate the labels with the >>> correct LSMs. >> The label follows the lsm name directly. What other association is required? > You need to know the order of the LSMs in order to > interpret/de-serialize the string. That's true for the string you included, but not for what I had actually proposed. >>> The ease-of-use quality is a bit subjective, but it does need >>> another interface to use properly and it requires string parsing which >>> history has shown to be an issue time and time again (although it is >>> relatively simple here). >> There was a lot of discussion regarding that. My proposed >> apparmor="unconfined",smack="User" format was panned for those same reasons. >> The nil byte format has been used elsewhere and suggested for that reason. > Based on what I recall from those discussions, it was my impression > the nil byte label delimiter was suggested simply because no one was > entertaining the idea of using something other than the existing > procfs interface. It is my opinion that we've taken that interface > about as far as it can go, and while it needs to stay intact for > compatibility reasons, the LSM stacking functionality should move to a > different API that is better suited for it. It's going to raise its ugly head again with SO_PEERCONTEXT for the SELinux+Smack case. But we can cross that bridge when we come to it.

>>> Once again, the syscall example I tossed out was a quick strawman, but >>> it had clearly separated and defined labels conveyed in data >>> structures that didn't require string parsing to find the label >>> associated with an LSM. >> True, but it uses pointers internally and you can't do that in memory >> you're sending up from the system. What comes from the syscall has to >> look something like the nil byte format. The strawman would have to do >> the same sort of parsing in userspace that you are objecting to. > Then we change the strawman. That's pretty much the definition of a > strawman example, it's something you toss out as starting point for > discussion and future improvements. If it was something much more > fully developed I would have submitted a patch .... sheesh. Fair enough. > In case it helps spur your imagination, here is a revised strawman: > > /** > * struct lsm_ctx - LSM context > * @id: the LSM id number, see LSM_ID_XXX A LSM ID hard coded in a kernel header makes it harder to develop new security modules. The security module can't be self contained. I say that, but I acknowledge that I've done the same kind of thing with the definition of the struct lsmblob. That isn't part of an external API however. It may also interfere with Tetsuo's long standing request that we don't do things that prevent the possibility of loadable security modules at some point in the future. I will also mention the out-of-tree security module objection, not because I care, but because someone most likely will bring it up. On the other hand, there's no great way to include two variable length strings in a structure like this. So unless we adopt something as ugly as the nil byte scheme this is supposed to replace I expect we're stuck with an LSM ID.