Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 7611 bytes --]

I finally got a weekend free, and started doing some major code hacking,
working on producing an LSM-based version of Brad Spengler's Grsecurity patch
(I previously posted to lkml an earlier version that did many of the parts that
weren't doable in an LSM).  As a starting point for the LSM itself, I used
the 'capabilities' module, and started adding the hooks I needed. (I'd
like to thank Serge Hallyn - his BSDJail LSM has been helpful.. Life got a
lot simpler once I realized that although grsecurity hooks chroot() and the
*chdir() functions, we really don't need to, since we can do what we need
via inode_permission() ;)

The ultimate goal is to produce something that will do much of the same
things as BSDJail, a number of other generally-useful system hardening things,
and be able to work either stand-alone or stacked with SELinux.

I discovered a few things:

1) if you have an LSM that's non-modular, you can't call the
register_sysctl_table() function from the security_initcall(), because it
happens before the sysctl infrastructure gets set up - you get a very dead
system, even early_printk doesn't do any good...  I ended up adding a call in
post_mountroot(), which was meant for that sort of thing...

2) It stacks just fine with SELinux, except there's hooks that selinux/hooks.c
either doesn't hook, or it doesn't call secondary_ops() in its hook.

I've attached a patch that does the secondary_ops callback for essentially
everything used by either my already-written code, in-progress code, or the
BSDJail module.  I've done a careful check of the list of hooks, the grsecurity
stuff, the BSDJail stuff, and my to-do list, and I believe that this is a
fairly complete list for everything I'm trying to do. The only things that
aren't addressed are the BSD Jail's use of the task_(alloc|free)_security and
network restrictions (which would require some major rethinking for
usage in a stacked LSM).

Question:  In some places, hooks.c calls secondary_ops->foo() before doing
its own checking, and some places, it does it afterwards.  I'm failing to
understand the rhyme and reason...

--- linux-2.6.6-rc3-mm2/security/selinux/hooks.c.vt	2004-05-08 14:37:12.000000000 -0400
+++ linux-2.6.6-rc3-mm2/security/selinux/hooks.c	2004-05-09 21:55:14.559521204 -0400
@@ -1402,6 +1402,10 @@ static int selinux_sysctl(ctl_table *tab
 	u32 tsid;
 	int rc;
 
+	rc = secondary_ops->sysctl(table, op);
+	if (rc)
+		return rc;
+
 	tsec = current->security;
 
 	rc = selinux_proc_get_sid(table->de, (op == 001) ?
@@ -2048,6 +2052,12 @@ static int selinux_mount(char * dev_name
                          unsigned long flags,
                          void * data)
 {
+	int rc;
+
+	rc = secondary_ops->sb_mount(dev_name, nd, type, flags, data);
+	if (rc)
+		return rc;
+
 	if (flags & MS_REMOUNT)
 		return superblock_has_perm(current, nd->mnt->mnt_sb,
 		                           FILESYSTEM__REMOUNT, NULL);
@@ -2058,10 +2068,32 @@ static int selinux_mount(char * dev_name
 
 static int selinux_umount(struct vfsmount *mnt, int flags)
 {
+	int rc;
+
+	rc = secondary_ops->sb_umount(mnt, flags);
+	if (rc)
+		return rc;
+
 	return superblock_has_perm(current,mnt->mnt_sb,
 	                           FILESYSTEM__UNMOUNT,NULL);
 }
 
+static void selinux_post_mountroot(void)
+{
+	secondary_ops->sb_post_mountroot();
+}
+
+static int selinux_pivotroot(struct nameidata *old_nd, struct nameidata *new_nd)
+{
+	int rc;
+
+	rc = secondary_ops->sb_pivotroot(old_nd, new_nd);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
 /* inode security operations */
 
 static int selinux_inode_alloc_security(struct inode *inode)
@@ -2131,6 +2163,12 @@ static int selinux_inode_rmdir(struct in
 
 static int selinux_inode_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 {
+	int rc;
+
+	rc = secondary_ops->inode_mknod(dir, dentry, mode, dev);
+	if (rc)
+		return rc;
+
 	return may_create(dir, dentry, inode_mode_to_security_class(mode));
 }
 
@@ -2169,17 +2207,29 @@ static int selinux_inode_follow_link(str
 static int selinux_inode_permission(struct inode *inode, int mask,
 				    struct nameidata *nd)
 {
+	int rc;
+
 	if (!mask) {
 		/* No permission to check.  Existence test. */
 		return 0;
 	}
 
+	rc = secondary_ops->inode_permission(inode, mask, nd);
+	if (rc)
+		return rc;
+
 	return inode_has_perm(current, inode,
 			       file_mask_to_av(inode->i_mode, mask), NULL, NULL);
 }
 
 static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
 {
+	int rc;
+
+	rc = secondary_ops->inode_setattr(dentry, iattr);
+	if (rc)
+		return rc;
+
 	if (iattr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
 			       ATTR_ATIME_SET | ATTR_MTIME_SET))
 		return dentry_has_perm(current, NULL, dentry, FILE__SETATTR);
@@ -2446,6 +2496,11 @@ static int selinux_file_ioctl(struct fil
 static int selinux_file_mmap(struct file *file, unsigned long prot, unsigned long flags)
 {
 	u32 av;
+	int rc;
+
+	rc = secondary_ops->file_mmap(file, prot, flags);
+	if (rc)
+		return rc;
 
 	if (file) {
 		/* read access is always possible with a mapping */
@@ -2466,6 +2521,12 @@ static int selinux_file_mmap(struct file
 static int selinux_file_mprotect(struct vm_area_struct *vma,
 				 unsigned long prot)
 {
+	int rc;
+
+	rc = secondary_ops->file_mprotect(vma, prot);
+	if (rc)
+		return rc;
+
 	return selinux_file_mmap(vma->vm_file, prot, vma->vm_flags);
 }
 
@@ -2563,6 +2624,12 @@ static int selinux_file_receive(struct f
 
 static int selinux_task_create(unsigned long clone_flags)
 {
+	int rc;
+
+	rc = secondary_ops->task_create(clone_flags);
+	if (rc)
+		return rc;
+
 	return task_has_perm(current, current, PROCESS__FORK);
 }
 
@@ -2638,13 +2705,24 @@ static int selinux_task_setgroups(struct
 
 static int selinux_task_setnice(struct task_struct *p, int nice)
 {
+	int rc;
+
+	rc = secondary_ops->task_setnice(p, nice);
+	if (rc)
+		return rc;
+
 	return task_has_perm(current,p, PROCESS__SETSCHED);
 }
 
 static int selinux_task_setrlimit(unsigned int resource, struct rlimit *new_rlim)
 {
 	struct rlimit *old_rlim = current->rlim + resource;
+	int rc;
 
+	rc = secondary_ops->task_setrlimit(resource, new_rlim);
+	if (rc)
+		return rc;
+	
 	/* Control the ability to change the hard limit (whether
 	   lowering or raising it), so that the hard limit can
 	   later be used as a safe reset point for the soft limit
@@ -2678,6 +2756,11 @@ static int selinux_task_getscheduler(str
 static int selinux_task_kill(struct task_struct *p, struct siginfo *info, int sig)
 {
 	u32 perm;
+	int rc;
+
+	rc = secondary_ops->task_kill(p, info, sig);
+	if (rc)
+		return rc;
 
 	if (info && ((unsigned long)info == 1 ||
 	             (unsigned long)info == 2 || SI_FROMKERNEL(info)))
@@ -3119,6 +3202,10 @@ static int selinux_socket_unix_stream_co
 	struct avc_audit_data ad;
 	int err;
 
+	err = secondary_ops->unix_stream_connect(sock, other, newsk);
+	if (err)
+		return err;
+
 	isec = SOCK_INODE(sock)->i_security;
 	other_isec = SOCK_INODE(other)->i_security;
 
@@ -3783,6 +3870,11 @@ static int selinux_shm_shmat(struct shmi
 			     char *shmaddr, int shmflg)
 {
 	u32 perms;
+	int rc;
+
+	rc = secondary_ops->shm_shmat(shp, shmaddr, shmflg);
+	if (rc)
+		return rc;
 
 	if (shmflg & SHM_RDONLY)
 		perms = SHM__READ;
@@ -4080,6 +4172,8 @@ struct security_operations selinux_ops =
 	.sb_statfs =			selinux_sb_statfs,
 	.sb_mount =			selinux_mount,
 	.sb_umount =			selinux_umount,
+	.sb_post_mountroot =		selinux_post_mountroot,
+	.sb_pivotroot =			selinux_pivotroot,
 
 	.inode_alloc_security =		selinux_inode_alloc_security,
 	.inode_free_security =		selinux_inode_free_security,



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Serge Hallyn]

>  The only things that
> aren't addressed are the BSD Jail's use of the task_(alloc|free)_security and
> network restrictions (which would require some major rethinking for
> usage in a stacked LSM).

Hi,

Both the /proc and network restrictions are done in ways which are
probably not acceptable to the kernel maintainers anyway.  For /proc, I
am considering doing a simpler inode_permission on pathname based
restriction.  This may be a part of the next version (which I have to
put out to handle iopl() and shared memory anyway.)

For network restrictions, I'm open to suggestions for another way to
implement these.

Regarding the task_security.  One possibility would be for selinux to
have a task_security->secondary pointer, and change get_security and
set_security in bsdjail.c to use task_security->secondary if it found it
was loaded under selinux.

Of course, another possibility which I prefer is to use the stacker lsm
and rewrite selinux to use that  :)

-serge


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]

On Mon, 10 May 2004 13:32:35 CDT, Serge Hallyn said:
> Both the /proc and network restrictions are done in ways which are
> probably not acceptable to the kernel maintainers anyway.  For /proc, I
> am considering doing a simpler inode_permission on pathname based
> restriction.  This may be a part of the next version (which I have to
> put out to handle iopl() and shared memory anyway.)

Would you be open to a collaboration?  A lot of my module is stuff that I'm
re-implementing mostly because it's such a pain to stack modules,
and I have some people that want selinux+chroot, some that want
selinux+tempdir, and some that want chroot+tempdir, and nobody
seems to like the invasiveness level of the grsecurity patch..

> For network restrictions, I'm open to suggestions for another way to
> implement these.

I admit not having thought about them yet, other than to realize that it's
a messy issue... ;)

> Regarding the task_security.  One possibility would be for selinux to
> have a task_security->secondary pointer, and change get_security and
> set_security in bsdjail.c to use task_security->secondary if it found it
> was loaded under selinux.

Unfortunately, that's probably a dead end.  What you'd probably want to
do is generalize the *_alloc_security() routines to support a linked list of
structures of the form:

struct sec_mem {	struct secmem *next; int magic; void *blob};

(I seem to remember that being proposed on the LSM list, and shot down
because it was encouraging massive stacking of modules, with the resulting
composition issues...)

> Of course, another possibility which I prefer is to use the stacker lsm
> and rewrite selinux to use that  :)

Unfortunately, Stephen Smalley is right - rampant composition is generally
a bad idea.


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Stephen Smalley]

On Sun, 2004-05-09 at 22:38, Valdis.Kletnieks@vt.edu wrote:
> 1) if you have an LSM that's non-modular, you can't call the
> register_sysctl_table() function from the security_initcall(), because it
> happens before the sysctl infrastructure gets set up - you get a very dead
> system, even early_printk doesn't do any good...  I ended up adding a call in
> post_mountroot(), which was meant for that sort of thing...

I'd recommend just using an ordinary initcall.  Note that the SELinux
module has several such ordinary initcall functions that handle similar
issues, e.g. registering Netfilter hooks, registering the selinuxfs
pseudo filesystem, registering a netdevice notifier for the netif table,
and creating the netlink selinux socket.  I'd expect post_mountroot to
die eventually; think early userspace.  Note that the SELinux initial
policy load has been moved into userspace; we don't use post_mountroot
anymore.

> 2) It stacks just fine with SELinux, except there's hooks that selinux/hooks.c
> either doesn't hook, or it doesn't call secondary_ops() in its hook.

As I mentioned in a previous reply to Joshua Brindle (for his attempt to
stack SELinux with digsig), we didn't want to encourage blindly stacking
modules with SELinux, so we only provided secondary ops as required. 
You will naturally encounter difficulties if your other security module
wants to use the security fields.

> Question:  In some places, hooks.c calls secondary_ops->foo() before doing
> its own checking, and some places, it does it afterwards.  I'm failing to
> understand the rhyme and reason...

We typically want to call the secondary module first, so that ordinary
Linux capability checks are applied first and the code never reaches
SELinux if they would deny access.  capget and capset are unusual in
that they get or set state; they don't just mediate access.  Hence, the
capset_set secondary hook must be called after the SELinux check.  The
capget secondary hook could be called first (since an error in the
subsequent SELinux check will still prevent copying to userspace by the
caller), but it doesn't matter much since capget doesn't perform any
checking of its own.  The capset_check hook should likely be called
first, to avoid a spurious SELinux check if the base logic would deny
anyway.
  
-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]

On Mon, 10 May 2004 14:38:39 EDT, Stephen Smalley said:

> I'd recommend just using an ordinary initcall.  Note that the SELinux
> module has several such ordinary initcall functions that handle similar
> issues, e.g. registering Netfilter hooks, registering the selinuxfs
> pseudo filesystem, registering a netdevice notifier for the netif table,
> and creating the netlink selinux socket.  I'd expect post_mountroot to
> die eventually; think early userspace.  Note that the SELinux initial
> policy load has been moved into userspace; we don't use post_mountroot
> anymore.

D'Oh! ;)

OK, I can do that... cancel the request for post_mountroot() then. ;)

> > 2) It stacks just fine with SELinux, except there's hooks that selinux/hooks.c
> > either doesn't hook, or it doesn't call secondary_ops() in its hook.
> 
> As I mentioned in a previous reply to Joshua Brindle (for his attempt to
> stack SELinux with digsig), we didn't want to encourage blindly stacking
> modules with SELinux, so we only provided secondary ops as required. 
> You will naturally encounter difficulties if your other security module
> wants to use the security fields.

I'm being very careful to *not* use the security fields, for exactly that
reason. All the info I'm examining is either somewhere in a dentry/inode or
passed in as an LSM API parameter.

Also, I'm intentionally restricting the scope of what I check for to a set of
things that are relatively easy to express in C code, but difficult to write
SELinux policy for - the two big predicates I'm testing are "if the process is
chroot'ed..." and "if the process is working with a file in a world-writeable
+t directory...".

I'm intentionally *not* doing any of the grsecurity patch's ACL stuff,
precisely because having 2 ACL systems active is guaranteed to be b0rked..

> We typically want to call the secondary module first, so that ordinary
> Linux capability checks are applied first and the code never reaches
> SELinux if they would deny access.  capget and capset are unusual in
> that they get or set state; they don't just mediate access.

Ahh.. That was the part I was missing... ;)



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Stephen Smalley]

On Sun, 2004-05-09 at 22:38, Valdis.Kletnieks@vt.edu wrote:
> I've attached a patch that does the secondary_ops callback for essentially
> everything used by either my already-written code, in-progress code, or the
> BSDJail module.

A couple of notes regarding your insertion of secondary_ops calls:

1) Joshua Brindle (cc'd) posted a patch to this list earlier to likewise
add secondary ops for another security module.  There were two points of
overlap:  inode_permission and file_mmap.  In the case of
inode_permission, you both inserted the secondary hook call after the
null mask test (existence test).  Are you sure that you will never want
to see such calls in a secondary module?  In the case of file_mmap,
Joshua's diff only calls the secondary module if there is a file struct,
whereas your diff always calls the secondary module.  The latter offers
greater flexibility to the secondary module, so I'm inclined to use it,
but thought I should note it as it means that the secondary module will
need to test for the null file case and act accordingly.

2) Neither the post_mountroot nor pivotroot hooks are likely to survive
long term IMHO, so I don't plan on adding them to the SELinux module.
 
-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Joshua Brindle]

Stephen Smalley wrote:

>On Sun, 2004-05-09 at 22:38, Valdis.Kletnieks@vt.edu wrote:
>  
>
>>I've attached a patch that does the secondary_ops callback for essentially
>>everything used by either my already-written code, in-progress code, or the
>>BSDJail module.
>>    
>>
>
>A couple of notes regarding your insertion of secondary_ops calls:
>
>1) Joshua Brindle (cc'd) posted a patch to this list earlier to likewise
>add secondary ops for another security module.  There were two points of
>overlap:  inode_permission and file_mmap.  In the case of
>inode_permission, you both inserted the secondary hook call after the
>null mask test (existence test).  Are you sure that you will never want
>to see such calls in a secondary module?  In the case of file_mmap,
>Joshua's diff only calls the secondary module if there is a file struct,
>whereas your diff always calls the secondary module.  The latter offers
>greater flexibility to the secondary module, so I'm inclined to use it,
>but thought I should note it as it means that the secondary module will
>need to test for the null file case and act accordingly.
>
>2) Neither the post_mountroot nor pivotroot hooks are likely to survive
>long term IMHO, so I don't plan on adding them to the SELinux module.
> 
>  
>
It might be a better idea for other modules to call the secondary module 
before the check, I wasn't too concerned about it for the module I was 
working on but it might be an issue for others.

Joshua Brindle

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]

On Thu, 13 May 2004 13:55:20 EDT, Stephen Smalley said:

> overlap:  inode_permission and file_mmap.  In the case of
> inode_permission, you both inserted the secondary hook call after the
> null mask test (existence test).  Are you sure that you will never want
> to see such calls in a secondary module?

Hmm... I remember thinking about that issue, and deciding that for the set of
things that I was trying to address, that if there was an issue with
information leakage on file existence checks that it would be handled by the
SElinux code in the subsequent policy check..

Of course, now that you mention it, I was obviously not thinking clearly, as
we'll never call inode_has_perm either... ;)

> 2) Neither the post_mountroot nor pivotroot hooks are likely to survive
> long term IMHO, so I don't plan on adding them to the SELinux module.

It turned out that my need for post_mountroot hooks was spurious - I needed a
callback later than the security_initcalls, and it turns out a regular initcall
was sufficient.

The only thing I'm doing in the pivotroot check is protecting against a buggy
daemon that runs chroot'ed but still has a file descriptor open on a directory
outside the chroot (as can happen if it doesn't chdir() itself into the
chroot() directory, to prohibit a pivot_root out of the chroot environment
(basically, fchdir(open dir) and then pivot_root("../../..") yourself back to
the real root).

However, it occurs to me that I can live without that specific check, because
pivot_root will end up calling __user_file_walk, which will call
inode_permission on the components - and I'll be able to notice at that point
that ".." isn't under the chroot and toss an -EPERM back.  

So I'd not be heartbroken if those two callback didn't happen... :)


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Linux 2.6.6-rc3-mm2 and SELinux support of stacked modules

[Stephen Smalley]

On Thu, 2004-05-13 at 17:54, Valdis.Kletnieks@vt.edu wrote:
> Hmm... I remember thinking about that issue, and deciding that for the set of
> things that I was trying to address, that if there was an issue with
> information leakage on file existence checks that it would be handled by the
> SElinux code in the subsequent policy check..
> 
> Of course, now that you mention it, I was obviously not thinking clearly, as
> we'll never call inode_has_perm either... ;)

Hiding file existence would require more than what LSM supports (e.g.
directory read filtering, fake results on file creation requests for the
same name).  And we'd prefer an approach that implements security union
directories anyway (union view of a collection of member subdirectories,
partitioned by security context, with the view computed based on the
security context of the process and the security context of the
members).

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.