On Mon, 2005-07-25 at 10:57 -0500, serue@private wrote: > Thanks Stephen, I like the patch. Without it: > > 1. Having to remember to enable TMPFS_FS_SECURITY seems silly. > If I want selinux, obviously I want this. > > 2. The xattr handlers end up being silly. > > To clarify: the security implication of including the > fs/xattr.c:setxattr part of the patch is that for a filesystem which > does not support setxattr, the security_inode_setsecurity is > unconditionally called for "security.*" values? So that a module which > unconditinally acted upon the values sent to security_inode_setsecurity() > could be tricked? You still have to pass the earlier security_inode_setxattr hook call for authorization; the patch moves the authorization hook call so that it is always applied, regardless of whether we are calling the fs method or using the fallback for security xattrs. However, this puts a burden on security module writers and/or policy writers to ensure that setxattr is not misused on pseudo and legacy filesystems unless you specifically want that behavior (e.g. allow for tmpfs and devpts and deny for proc), and the existing permission checks on the inode may not be adequate, as the security label on the inode might be the same as the security label on some inode in another filesystem type where you do want to allow relabeling. Alternative is to strictly partition the security labels (TE types) used on inodes in filesystems that should support relabeling from those that should not. > I would agree that it is best to make it well known that this should be > controlled through policy on filesystem types. Possibly a superblock_has_perm() check, using the superblock SID/context to distinguish such filesystems. > As an alternative, what about calling security_inode_setxattr() before the > setsecurity() anyway, and making it clear that security_inode_setxattr() > should be used as an authorization hook, and > security_inode_post_setxattr() should be used for any actions? The patch does call the authorization hooks (e.g. security_inode_setxattr) prior to the operation hook (e.g. security_inode_setsecurity). security_inode_post_setxattr is superfluous in this case, as it normally handles the incore inode security state update (since the ->setxattr method only handles the on-disk xattr state update), and that duplicates what security_inode_setsecurity does, so we don't call post_setxattr in the fallback case. -- Stephen Smalley National Security Agency
This archive was generated by hypermail 2.1.3 : Mon Jul 25 2005 - 10:51:36 PDT