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

[Jan Engelhardt <jengelh@linux01.gwdg.de>]

>
>The bulk of the first patch appears to be capable -> capable_x changes.
>What is the purpose of that?
>
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).

>What's the rationale for the int->gid_t and int->uid_t changes in sys?
>

I don't know, but there must be a reason uid_t and gid_t exist in the kernel in
the first place.

>From a technical point, gid_t and uid_t are unsigned, while int is not.

>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.

>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.



Jan Engelhardt
-- 
-
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/