Messages in this thread | | | Date | Sat, 22 Apr 2006 13:13:18 +0200 (MEST) | From | Jan Engelhardt <> | Subject | Re: Time to remove LSM (was Re: [RESEND][RFC][PATCH 2/7] implementation of LSM hooks) |
| |
>> 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.
The subadmin gets CAP_SYS_ADMIN. This is because capable() is often done before the LSM callout, e.g. sys_sethostname(). Even when capable() is after an LSM call, the process needs a certain capability or the syscall/function will return failure to userspace. The best solution I can think of is that capable() disappears entirely from functions and that an LSM function does it all then.
>- Writing a security module based on function names in other modules is >obviously very fragile; some kind of abstraction would be required.
Yes, but at least it returns permission denied in the default case :) Best would probably if the caller uses a fixed string rather than __FUNCTION__ from the expanded capable_x() macro. Or an enum value...
enum { HK_SETHOSTNAME, };
>> >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).
static bool mt_file_alloc_override(int cur, int max) { /* Called when the maximum number of files is open. Only superadmins may override this. Return >0 for success. */ return is_any_superadm(current->euid, current->egid); }
I see what you mean - cur and max are unused. However, if I was to call to a seconday module from within this function (which is not really implemented), I would suddenly need cur and max again. I do think that the parameters have a reason to be there - a user might wish to enforce this sort:
return is_any_superadm(...) || (is_any_subadm(...) && (max <= 2048))
E.g. "allow some more" for subadms. Multiadmin is currently extremely tailored to one specific (call it the default :-) use case.
>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.
Exactly (like above with capable(), DAC shares the same 'positional' problem). In fact, after some thought I could come to the conclusion that the POSIX CAPability framework is more a hindrance for this sort of LSM, since it's mostly UID/GID-based. We only raise capabilities so the rest of the kernel plays fine with us.
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/
| |