On Fri, 2004-12-03 at 13:04, Serge Hallyn wrote: > Changes: > 1. uses RCU to protect the list elements. > 2. capable() now just uses RETURN_ERROR_IF_ANY_ERROR explicitly. > 3. stacker exports lsm_adopt_next_secondary(char *name), which will > delegate stacking of the next loaded LSM to the module identified by > (name). > 4. stacker doesn't touch the get/setprocattr functions. There's no > point messing with that unless/until userspace code can handle it, as > there's no clean way to handle all cases otherwise. > 5. mod_unreg_security is removed. All modules use unregister_security > () regardless of whether they were loaded using register_security() or > mod_reg_security(). In this way, if capability() was escalated by > selinux from secondary to a primary module, it doesn't end up > unregistering the wrong way. - When would we not want stacker to short circuit processing upon encountering an error from any security module for hooks that return error codes? The rationale in the comments is that some modules may change state when called, but the question is whether any such state update should occur when the operation is going to fail anyway. In the current built-in "stacking" support in SELinux, we intentionally short circuit when the secondary module returns an error to avoid filling the logs with audit messages that would have failed anyway due to the normal capability check. Further, in the capget and capset_set cases, we intentionally short circuit when the SELinux permission check fails to avoid having the capability module get or set the capability bits, as we don't want that to happen if access is denied by SELinux. - The last point highlights another issue if we were to try to stack capabilities and SELinux via stacker rather than via SELinux secondary_ops: we presently apply different orderings for capget and capset_set than for other hooks. Possibly reflects a more basic problem in the capset code and hooks; capset_check is supposed to handle security checking while capset_set is supposed to just update state, but we often don't have the specific target task in capset_check, so capset_set has to apply a check too for completeness. Also, looking at the core kernel, it looks like capset_check isn't serving a useful purpose anymore, as the core kernel is applying the same checks directly. capget may not be a real issue, as it is only getting the bits into a kernel data structure and will not copy to userspace if we returned an error. Possibly need two separate hooks at the same points where capset_set is presently being invoked to cleanly separate permission checking hooks from state update. - It seems like the alloc_security hooks are a problem for stacker, because you could have one or more modules succeed in allocating the security state before one fails, and the core kernel doesn't call the free_security hook upon an error from the alloc_security hook, as it reasonably assumes cleanup of any partially allocated state prior to the hook returning. Either the stacker needs to internally call the free_security() hooks for all modules that returned success from alloc_security() to cleanup, or the core kernel needs to be changed to always call free_security() after any call to alloc_security(), even if it failed. -- Stephen Smalley <sds@private> National Security Agency
This archive was generated by hypermail 2.1.3 : Mon Dec 06 2004 - 08:50:16 PST