On 9/8/22 11:05, Casey Schaufler wrote:
On 9/7/2022 8:57 PM, Paul Moore wrote:
On Wed, Sep 7, 2022 at 7:53 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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.
AppArmor too, I am working on revising the out of tree af_unix mediation
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.
well at a minimum we shouldn't export the kernel internal LSM_ID if its
exposed to userspace it needs to be something that can live with for a
long time
- Fixed length strings, which really are just a long LSM ID, Say 8 bytes.
Can still even look human readable. For most* LSMs this could just
be their name.
* only safesetid and capability don't fit atm
- and certainly uglier, for variable length use an index for one of the
variable length strings, with an embedded \0 inside the variable length
string
{
size_t lsm_id_len;
size_t lsm_id_ctx_index;
size_t ctx_len;
unsigned char ctx[];
}
with access to lsm id being ctx[lsm_id_ctx_index]
