On Tue, 2004-11-23 at 15:31, Serge Hallyn wrote: > Attached is the next set of patches to implement stacking through > chaining. - I noticed that your patch changes SELinux to use unregister_security() for runtime disable rather than explicitly resetting security_ops to secondary_ops. While I agree abstractly that we shouldn't be manually resetting security_ops (and doing so could create confusion for the other module, e.g. capability is suddenly being escalated from a secondary to a primary module by our code), unregister_security() isn't what you want either, as it will always reset to the dummy module, so you lose any secondary module at the same time you unregister SELinux. If you consider common usage in Fedora today, /sbin/init will invoke the runtime disable when /etc/selinux/config specifies that SELinux is disabled, and the expected behavior is that we are then left with capabilities. Your patch would change the behavior. So we have to consider the right way to approach that issue. - If someone were to use the stacker module to stack capabilities and SELinux (or any module and SELinux), they would actually end up with capabilities (or the other module) + SELinux + dummy, given the retention of the secondary_ops in SELinux. Further, as the stacker module does not call the stacked modules upon a register_security hook, there would no longer be any way to change secondary_ops for SELinux from dummy to any other setting, so the only way to stack capabilities and SELinux with the stacker module would be via the stacker module itself, correct? Hence, secondary_ops becomes useless in the presence of the stacker at present. - You don't want the #ifdef CONFIG_SECURITY_STACKER stuff in security.h, because that means that enabling/disabling the stacker requires a major rebuild even though it only truly affects the security subtree, right? So you want a new header that is only included by the security subtree, not callers of the security hooks. - Claim that the stacker_capable() hook shouldn't just use RETURN_ERROR_IF_ANY_ERROR() seems dubious to me; capable hook is authoritative with respect to the core kernel, but I would still typically expect restrictive behavior for multiple modules. In general, stacker module needs major cleanup. - stacker-selinux-procattr-hack.patch doesn't seem to work as expected; booting a kernel built with stacker+selinux+capabilities, I get vast numbers of "stacker_setprocattr: no data after module name" errors and resulting permission denials due to failure to set process attributes; system doesn't get very far in enforcing mode. In permissive mode, I also see unexpected denials occurring for sys_admin capability from programs that don't normally trigger such attempts with a non-stacker kernel. Possibly a bad interaction between SELinux and stacker, as SELinux sometimes calls capable() internally, expecting it to apply both SELinux and capability module checking, e.g. the CAP_SYS_ADMIN check in selinux_inode_setxattr(). -- Stephen Smalley <sds@private> National Security Agency
This archive was generated by hypermail 2.1.3 : Wed Nov 24 2004 - 09:50:31 PST