Re: [RFC] [PATCH] Stacking through chaining (v3)

From: Stephen Smalley (sds@private)
Date: Wed Nov 24 2004 - 09:45:29 PST

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