Re: Time to remove LSM (was Re: [RESEND][RFC][PATCH 2/7] implementation of LSM hooks)

[Stephen Smalley <sds@tycho.nsa.gov>]

On Fri, 2006-04-21 at 20:57 +0200, Jan Engelhardt wrote:
> When you grant CAP_SYS_ADMIN to a user, he can do _a lot_ of things. I wanted
> to have the sub-admin have read rights in most places (e.g. nvram to pick a
> random example), but not to have write rights. Unfortunately, read rights and
> write rights for nvram both fall under CAP_SYS_ADMIN.
> 
> capable_x will call out to security_cap_extra(), passing it the current
> function name as a string. An LSM module can then provide security_cap_extra
> via the security_operations vector and decide whether to allow the request. I
> primarily did this because it reduces the amount of recompiling needed. For
> example, instead of having to add a ->nvram_allow_read and ->nvram_allow_write
> hooks in include/linux/security.h -- which requires compilation of a lot of
> parts -- I can simply use strcmp(func, "nvram_read") in the LSM. I agree that
> this is not optimal, but having 1000 pointers in the security_ops vector is not
> a solution either (-> code bloat).

- Where is the user of the cap_extra hook?  I don't see it in multiadm.
If you aren't using it, it isn't a requirement for multiadm, obviously;
you should drop those diffs out and just focus on what you need for
multiadm itself.
- Writing a security module based on function names in other modules is
obviously very fragile; some kind of abstraction would be required.

> >Some of the hooks used to exist in LSM patches but didn't have a real
> >user for merging at the time.  But it isn't clear whether you actually
> >need separate hooks for each of them or if they are being mapped to the
> >same check in many cases - can it be abstracted to a common hook?
> 
> Not without passing a handful of useless parameters (NULL or 0) to each
> function.
> As much as I would like to combine for example mt_sb_mount and mt_sb_pivotroot,
> the prototypes are just too different.
> Suggestions welcome.

I wasn't suggesting passing the union of their parameters, just the ones
that are actually used by your module (plus any parameters used by any
other in-tree user, i.e. capability and SELinux).  And trying to
abstract a higher level conceptual operation as the hook rather than
making them per-syscall/operation.  I know that LSM currently does
things the other way, but I'd much rather see it move toward more
general permission checking interfaces (ala the internal SELinux ones,
although I understand those specific interfaces aren't what you would
use).  Others may have a different POV.

> >Seems like you are duplicating a lot of the base DAC logic in the
> >process; would be nice to encapsulate that in the core kernel, and then
> >just use a common helper in both cases?
> 
> The problem is that the base DAC logic is done after security_*(). Sometimes
> that's good, most of the times, it leads to duplicated DAC logic because the
> "usual DAC decision" is part of how multiadm decides.
> 
> Suggestions welcome too.

At least for common cases like permission(9) and ipcperms(9), the
security hook call comes after the DAC checking.  But not always.
Sounds like you want authoritative hooks, which were discussed and
experimented with during LSM development, see archives.  You could
attempt to revive that for the cases where you need it, moving the
checking into the commoncap functions (defining new ones where needed,
and making sure that existing modules call them so that they still apply
those checks) and calling those functions rather than duplicating the
logic in your own module.  You presumably don't want to have to maintain
the duplicated logic in your own module.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/