Re: [PATCH 2/2] sysctl: Restore the selinux path based label lookup for sysctls.

[Stephen Smalley <sds@tycho.nsa.gov>]

On Wed, 2007-02-07 at 16:12 -0500, Stephen Smalley wrote:
> On Wed, 2007-02-07 at 13:24 -0500, Stephen Smalley wrote:
> > On Tue, 2007-02-06 at 14:21 -0700, Eric W. Biederman wrote:
> > > This time instead of generating the generating the paths from proc_dir_entries
> > > generate the labels from the names in the sysctl ctl_tables themselves.  This
> > > removes an unnecessary layer of indirection, allows this to work even when
> > > procfs support is not compiled into the kernel, and especially allows it
> > > to work now that ctl_tables no longer have a proc_dir_entry field.
> > 
> > Thanks, looks sane.
> > 
> > > I continue passing "proc" into genfs sid although that is complete nonsense
> > > to allow existing selinux policies to work without modification.
> > > 
> > > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> > 
> > Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>
> 
> Hmmm...but in testing the patch, I don't seem to (consistently) reach
> these checks when accessing via /proc/sys.  I see that you are caching
> the mode information and using it in some cases rather than calling the
> sysctl_perm function.

Actually, on further inspection, it looks like the real issue is the
"path" name generation; "cat /proc/sys/kernel/modprobe" yields a call to
security_genfs_sid() with just "/modprobe" rather than the expected
"/sys/kernel/modprobe".  Which likewise leaves us with the generic proc
label, just as with the inode permission check, so I end up seeing
checks against it only.

> One related but separate issue is that the /proc/sys inode labeling is
> also affected by the sysctl patch series.  Those inodes used to be
> labeled by selinux_proc_get_sid (from selinux_d_instantiate), but that
> no longer works, so they now fall back to the superblock SID (generic
> proc label).  That changes the inode permission checks on an attempt to
> access a /proc/sys node and will likely cause denials under current
> policy for confined domains since one wouldn't generally be writing to
> the generic proc label. If you always called sysctl_perm from the proc
> sysctl code, we could possibly dispense with inode permission checking
> on those inodes, e.g. marking them private.

-- 
Stephen Smalley
National Security Agency