selinux stacked ops

selinux stacked ops

[Chris Wright]

I'm unclear on the internal stacking for SELinux.  Some of them make sense
from the perpsective of working with capablities, specifically these:

rc = secondary_ops->ptrace(parent,child);
return secondary_ops->capget(target, effective, inheritable, permitted);        error = secondary_ops->capset_check(target, effective, inheritable, permitted);
secondary_ops->capset_set(target, effective, inheritable, permitted);
rc = secondary_ops->capable(tsk, cap);
rc = secondary_ops->syslog(type);
rc = secondary_ops->capable(current, CAP_SYS_ADMIN); /* for vm_enough_memory */
rc = secondary_ops->bprm_set_security(bprm);
return (atsecure || secondary_ops->bprm_secureexec(bprm));
secondary_ops->bprm_apply_creds(bprm, unsafe);
err = secondary_ops->netlink_send(sk, skb);
      (not exactly using netlink_recv)
return secondary_ops->task_post_setuid(id0,id1,id2,flags);
secondary_ops->task_reparent_to_init(p);

not using cap:
cap_settime (obviously)
cap_inode_setxattr (makes sense)
cap_inode_removexattr (makes sense)
cap_vm_enough_memory (makes sense)

These are no-ops in dummy or capabilities, but don't represent the rest
of the hooks, or the rest in a single object area (i.e. task or inode).
Looks like (from discussion in early May) they were added to help stack
with grsec and something else?

rc = secondary_ops->sysctl(table, op);
return secondary_ops->bprm_check_security(bprm);
rc = secondary_ops->sb_mount(dev_name, nd, type, flags, data);
rc = secondary_ops->sb_umount(mnt, flags);
rc = secondary_ops->inode_link(old_dentry,dir,new_dentry);  /* maybe owlsm? */
rc = secondary_ops->inode_unlink(dir, dentry);
rc = secondary_ops->inode_mknod(dir, dentry, mode, dev);
rc = secondary_ops->inode_follow_link(dentry,nameidata);    /* maybe owlsm? */
rc = secondary_ops->inode_permission(inode, mask, nd);
rc = secondary_ops->inode_setattr(dentry, iattr);
rc = secondary_ops->file_mmap(file, reqprot, prot, flags);  /* maybe digsig? */
rc = secondary_ops->file_mprotect(vma, reqprot, prot);      /* maybe digsig? */
rc = secondary_ops->task_create(clone_flags);
rc = secondary_ops->task_setnice(p, nice);
rc = secondary_ops->task_setrlimit(resource, new_rlim);
rc = secondary_ops->task_kill(p, info, sig);
err = secondary_ops->unix_stream_connect(sock, other, newsk);
rc = secondary_ops->shm_shmat(shp, shmaddr, shmflg);

Would it make more sense to directly use cap_* as library functions,
and drop the whole ad-hoc internal stacking?

--
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.

[PATCH] remove selinux stacked ops

[Chris Wright]

* Chris Wright (chrisw@osdl.org) wrote:
> Would it make more sense to directly use cap_* as library functions,
> and drop the whole ad-hoc internal stacking?

Something like this.  Granted, any new stacking would require external
support...

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -102,14 +102,8 @@ static int __init selinux_enabled_setup(
 __setup("selinux=", selinux_enabled_setup);
 #endif
 
-/* Original (dummy) security module. */
-static struct security_operations *original_ops = NULL;
-
-/* Minimal support for a secondary security module,
-   just to allow the use of the dummy or capability modules.
-   The owlsm module can alternatively be used as a secondary
-   module as long as CONFIG_OWLSM_FD is not enabled. */
-static struct security_operations *secondary_ops = NULL;
+/* Original (default) security module. */
+static struct security_operations *original_ops;
 
 /* Lists of inode and superblock security structures initialized
    before the policy was loaded. */
@@ -1352,7 +1346,7 @@ static int selinux_ptrace(struct task_st
 	struct task_security_struct *csec = child->security;
 	int rc;
 
-	rc = secondary_ops->ptrace(parent,child);
+	rc = cap_ptrace(parent,child);
 	if (rc)
 		return rc;
 
@@ -1372,7 +1366,7 @@ static int selinux_capget(struct task_st
 	if (error)
 		return error;
 
-	return secondary_ops->capget(target, effective, inheritable, permitted);
+	return cap_capget(target, effective, inheritable, permitted);
 }
 
 static int selinux_capset_check(struct task_struct *target, kernel_cap_t *effective,
@@ -1380,7 +1374,7 @@ static int selinux_capset_check(struct t
 {
 	int error;
 
-	error = secondary_ops->capset_check(target, effective, inheritable, permitted);
+	error = cap_capset_check(target, effective, inheritable, permitted);
 	if (error)
 		return error;
 
@@ -1390,14 +1384,14 @@ static int selinux_capset_check(struct t
 static void selinux_capset_set(struct task_struct *target, kernel_cap_t *effective,
                                kernel_cap_t *inheritable, kernel_cap_t *permitted)
 {
-	secondary_ops->capset_set(target, effective, inheritable, permitted);
+	cap_capset_set(target, effective, inheritable, permitted);
 }
 
 static int selinux_capable(struct task_struct *tsk, int cap)
 {
 	int rc;
 
-	rc = secondary_ops->capable(tsk, cap);
+	rc = cap_capable(tsk, cap);
 	if (rc)
 		return rc;
 
@@ -1412,10 +1406,6 @@ 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) ?
@@ -1484,7 +1474,7 @@ static int selinux_syslog(int type)
 {
 	int rc;
 
-	rc = secondary_ops->syslog(type);
+	rc = cap_syslog(type);
 	if (rc)
 		return rc;
 
@@ -1515,7 +1505,7 @@ static int selinux_syslog(int type)
  * mapping. 0 means there is enough memory for the allocation to
  * succeed and -ENOMEM implies there is not.
  *
- * Note that secondary_ops->capable and task_has_perm_noaudit return 0
+ * Note that cap_capable and task_has_perm_noaudit return 0
  * if the capability is granted, but __vm_enough_memory requires 1 if
  * the capability is granted.
  *
@@ -1527,7 +1517,7 @@ static int selinux_vm_enough_memory(long
 	int rc, cap_sys_admin = 0;
 	struct task_security_struct *tsec = current->security;
 
-	rc = secondary_ops->capable(current, CAP_SYS_ADMIN);
+	rc = cap_capable(current, CAP_SYS_ADMIN);
 	if (rc == 0)
 		rc = avc_has_perm_noaudit(tsec->sid, tsec->sid,
 					SECCLASS_CAPABILITY,
@@ -1570,7 +1560,7 @@ static int selinux_bprm_set_security(str
 	struct avc_audit_data ad;
 	int rc;
 
-	rc = secondary_ops->bprm_set_security(bprm);
+	rc = cap_bprm_set_security(bprm);
 	if (rc)
 		return rc;
 
@@ -1635,12 +1625,6 @@ static int selinux_bprm_set_security(str
 	return 0;
 }
 
-static int selinux_bprm_check_security (struct linux_binprm *bprm)
-{
-	return secondary_ops->bprm_check_security(bprm);
-}
-
-
 static int selinux_bprm_secureexec (struct linux_binprm *bprm)
 {
 	struct task_security_struct *tsec = current->security;
@@ -1655,7 +1639,7 @@ static int selinux_bprm_secureexec (stru
 					 PROCESS__NOATSECURE, NULL);
 	}
 
-	return (atsecure || secondary_ops->bprm_secureexec(bprm));
+	return (atsecure || cap_bprm_secureexec(bprm));
 }
 
 static void selinux_bprm_free_security(struct linux_binprm *bprm)
@@ -1756,7 +1740,7 @@ static void selinux_bprm_apply_creds(str
 	u32 sid;
 	int rc;
 
-	secondary_ops->bprm_apply_creds(bprm, unsafe);
+	cap_bprm_apply_creds(bprm, unsafe);
 
 	tsec = current->security;
 
@@ -1980,12 +1964,6 @@ 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);
@@ -1996,12 +1974,6 @@ 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);
 }
@@ -2030,11 +2002,6 @@ static void selinux_inode_post_create(st
 
 static int selinux_inode_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
 {
-	int rc;
-
-	rc = secondary_ops->inode_link(old_dentry,dir,new_dentry);
-	if (rc)
-		return rc;
 	return may_link(dir, old_dentry, MAY_LINK);
 }
 
@@ -2045,11 +2012,6 @@ static void selinux_inode_post_link(stru
 
 static int selinux_inode_unlink(struct inode *dir, struct dentry *dentry)
 {
-	int rc;
-
-	rc = secondary_ops->inode_unlink(dir, dentry);
-	if (rc)
-		return rc;
 	return may_link(dir, dentry, MAY_UNLINK);
 }
 
@@ -2080,12 +2042,6 @@ 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));
 }
 
@@ -2113,23 +2069,12 @@ static int selinux_inode_readlink(struct
 
 static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *nameidata)
 {
-	int rc;
-
-	rc = secondary_ops->inode_follow_link(dentry,nameidata);
-	if (rc)
-		return rc;
 	return dentry_has_perm(current, NULL, dentry, FILE__READ);
 }
 
 static int selinux_inode_permission(struct inode *inode, int mask,
 				    struct nameidata *nd)
 {
-	int rc;
-
-	rc = secondary_ops->inode_permission(inode, mask, nd);
-	if (rc)
-		return rc;
-
 	if (!mask) {
 		/* No permission to check.  Existence test. */
 		return 0;
@@ -2141,12 +2086,6 @@ static int selinux_inode_permission(stru
 
 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_FORCE)
 		return 0;
 
@@ -2451,12 +2390,6 @@ static int file_map_prot_check(struct fi
 static int selinux_file_mmap(struct file *file, unsigned long reqprot,
 			     unsigned long prot, unsigned long flags)
 {
-	int rc;
-
-	rc = secondary_ops->file_mmap(file, reqprot, prot, flags);
-	if (rc)
-		return rc;
-
 	if (selinux_checkreqprot)
 		prot = reqprot;
 
@@ -2470,10 +2403,6 @@ static int selinux_file_mprotect(struct 
 {
 	int rc;
 
-	rc = secondary_ops->file_mprotect(vma, reqprot, prot);
-	if (rc)
-		return rc;
-
 	if (selinux_checkreqprot)
 		prot = reqprot;
 
@@ -2608,12 +2537,6 @@ 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);
 }
 
@@ -2662,7 +2585,7 @@ static int selinux_task_setuid(uid_t id0
 
 static int selinux_task_post_setuid(uid_t id0, uid_t id1, uid_t id2, int flags)
 {
-	return secondary_ops->task_post_setuid(id0,id1,id2,flags);
+	return cap_task_post_setuid(id0,id1,id2,flags);
 }
 
 static int selinux_task_setgid(gid_t id0, gid_t id1, gid_t id2, int flags)
@@ -2694,23 +2617,12 @@ 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->signal->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
@@ -2735,11 +2647,6 @@ 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)))
@@ -2778,7 +2685,7 @@ static void selinux_task_reparent_to_ini
 {
   	struct task_security_struct *tsec;
 
-	secondary_ops->task_reparent_to_init(p);
+	cap_task_reparent_to_init(p);
 
 	tsec = p->security;
 	tsec->osid = tsec->sid;
@@ -3227,10 +3134,6 @@ 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;
 
@@ -3603,7 +3506,7 @@ static int selinux_netlink_send(struct s
 	struct av_decision avd;
 	int err;
 
-	err = secondary_ops->netlink_send(sk, skb);
+	err = cap_netlink_send(sk, skb);
 	if (err)
 		return err;
 
@@ -3620,13 +3523,6 @@ static int selinux_netlink_send(struct s
 	return err;
 }
 
-static int selinux_netlink_recv(struct sk_buff *skb)
-{
-	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
-		return -EPERM;
-	return 0;
-}
-
 static int ipc_alloc_security(struct task_struct *task,
 			      struct kern_ipc_perm *perm,
 			      u16 sclass)
@@ -3947,11 +3843,6 @@ static int selinux_shm_shmat(struct shmi
 			     char __user *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,37 +3971,6 @@ static int selinux_ipc_permission(struct
 	return ipc_has_perm(ipcp, av);
 }
 
-/* module stacking operations */
-static int selinux_register_security (const char *name, struct security_operations *ops)
-{
-	if (secondary_ops != original_ops) {
-		printk(KERN_INFO "%s:  There is already a secondary security "
-		       "module registered.\n", __FUNCTION__);
-		return -EINVAL;
- 	}
-
-	secondary_ops = ops;
-
-	printk(KERN_INFO "%s:  Registering secondary module %s\n",
-	       __FUNCTION__,
-	       name);
-
-	return 0;
-}
-
-static int selinux_unregister_security (const char *name, struct security_operations *ops)
-{
-	if (ops != secondary_ops) {
-		printk (KERN_INFO "%s:  trying to unregister a security module "
-		        "that is not registered.\n", __FUNCTION__);
-		return -EINVAL;
-	}
-
-	secondary_ops = original_ops;
-
-	return 0;
-}
-
 static void selinux_d_instantiate (struct dentry *dentry, struct inode *inode)
 {
 	if (inode)
@@ -4278,14 +4138,12 @@ static struct security_operations selinu
 	.vm_enough_memory =		selinux_vm_enough_memory,
 
 	.netlink_send =			selinux_netlink_send,
-        .netlink_recv =			selinux_netlink_recv,
 
 	.bprm_alloc_security =		selinux_bprm_alloc_security,
 	.bprm_free_security =		selinux_bprm_free_security,
 	.bprm_apply_creds =		selinux_bprm_apply_creds,
 	.bprm_post_apply_creds =	selinux_bprm_post_apply_creds,
 	.bprm_set_security =		selinux_bprm_set_security,
-	.bprm_check_security =		selinux_bprm_check_security,
 	.bprm_secureexec =		selinux_bprm_secureexec,
 
 	.sb_alloc_security =		selinux_sb_alloc_security,
@@ -4382,9 +4240,6 @@ static struct security_operations selinu
 	.sem_semctl =			selinux_sem_semctl,
 	.sem_semop =			selinux_sem_semop,
 
-	.register_security =		selinux_register_security,
-	.unregister_security =		selinux_unregister_security,
-
 	.d_instantiate =                selinux_d_instantiate,
 
 	.getprocattr =                  selinux_getprocattr,
@@ -4433,9 +4288,7 @@ static __init int selinux_init(void)
 
 	avc_init();
 
-	original_ops = secondary_ops = security_ops;
-	if (!secondary_ops)
-		panic ("SELinux: No initial security operations\n");
+	original_ops = security_ops;
 	if (register_security (&selinux_ops))
 		panic("SELinux: Unable to register with kernel.\n");
 
@@ -4568,8 +4421,8 @@ int selinux_disable(void)
 
 	selinux_disabled = 1;
 
-	/* Reset security_ops to the secondary module, dummy or capability. */
-	security_ops = secondary_ops;
+	/* Reset security_ops to the default */
+	security_ops = original_ops;
 
 	/* Unregister netfilter hooks. */
 	selinux_nf_ip_exit();

--
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: selinux stacked ops

[Stephen Smalley]

On Thu, 2005-08-25 at 15:17 -0700, Chris Wright wrote:
> These are no-ops in dummy or capabilities, but don't represent the rest
> of the hooks, or the rest in a single object area (i.e. task or inode).
> Looks like (from discussion in early May) they were added to help stack
> with grsec and something else?

Yes, they were added at the request of various people who wanted to
stack digsig, owlsm, or custom LSMs.  Not clear that they are important
anymore given the generic LSM stacking support.

> Would it make more sense to directly use cap_* as library functions,
> and drop the whole ad-hoc internal stacking?

Yes, if dummy is going to be killed entirely (as per the recently posted
patches), then it makes sense to hardwire to using the cap_ functions.
But I recall there are some issues with regard to properly falling back
to just capabilities upon a runtime disable of SELinux - I'll look at
your patch.

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[Stephen Smalley]

On Thu, 2005-08-25 at 15:51 -0700, Chris Wright wrote:
> @@ -3620,13 +3523,6 @@ static int selinux_netlink_send(struct s
>  	return err;
>  }
>  
> -static int selinux_netlink_recv(struct sk_buff *skb)
> -{
> -	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
> -		return -EPERM;
> -	return 0;
> -}
> -

Hmm...is removing this entirely safe?  Given that dummy is being
removed, and null ops are no longer being populated, don't we have to
ensure that we define a SELinux hook function for every cap_ function
and explicitly call that function?  Likewise, don't we now have to add a
settime hook to SELinux to preserve behavior?  Previously, it was
defaulting to the dummy hook, which conveniently called capable(), so it
ended up working regardless of whether SELinux was stacked with dummy or
capability.

> @@ -4278,14 +4138,12 @@ static struct security_operations selinu
>  	.vm_enough_memory =		selinux_vm_enough_memory,
>  
>  	.netlink_send =			selinux_netlink_send,
> -        .netlink_recv =			selinux_netlink_recv,

Ditto.

> @@ -4433,9 +4288,7 @@ static __init int selinux_init(void)
>  
>  	avc_init();
>  
> -	original_ops = secondary_ops = security_ops;
> -	if (!secondary_ops)
> -		panic ("SELinux: No initial security operations\n");
> +	original_ops = security_ops;
>  	if (register_security (&selinux_ops))
>  		panic("SELinux: Unable to register with kernel.\n")

At the point this is executed, security_ops == &default_security_ops;
hence...

> @@ -4568,8 +4421,8 @@ int selinux_disable(void)
>  
>  	selinux_disabled = 1;
>  
> -	/* Reset security_ops to the secondary module, dummy or capability. */
> -	security_ops = secondary_ops;
> +	/* Reset security_ops to the default */
> +	security_ops = original_ops;
>  
>  	/* Unregister netfilter hooks. */
>  	selinux_nf_ip_exit();

a runtime disable of SELinux by /sbin/init (upon finding
SELINUX=disabled in /etc/selinux/config) ends up leaving us with null
ops rather than the capability or dummy ops.  Thus, SELINUX=disabled
turns off all security ;(

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[Stephen Smalley]

On Fri, 2005-08-26 at 07:48 -0400, Stephen Smalley wrote:
> On Thu, 2005-08-25 at 15:51 -0700, Chris Wright wrote:
> > @@ -3620,13 +3523,6 @@ static int selinux_netlink_send(struct s
> >  	return err;
> >  }
> >  
> > -static int selinux_netlink_recv(struct sk_buff *skb)
> > -{
> > -	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
> > -		return -EPERM;
> > -	return 0;
> > -}
> > -
> 
> Hmm...is removing this entirely safe?  Given that dummy is being
> removed, and null ops are no longer being populated, don't we have to
> ensure that we define a SELinux hook function for every cap_ function
> and explicitly call that function?  Likewise, don't we now have to add a
> settime hook to SELinux to preserve behavior?  Previously, it was
> defaulting to the dummy hook, which conveniently called capable(), so it
> ended up working regardless of whether SELinux was stacked with dummy or
> capability.

Ah, never mind - I forgot that you are falling back to the cap_
functions in your static inlines if the operation is null.  So you could
also remove selinux_capset_set and selinux_task_post_setuid.

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[Stephen Smalley]

On Fri, 2005-08-26 at 07:48 -0400, Stephen Smalley wrote:
> > @@ -4568,8 +4421,8 @@ int selinux_disable(void)
> >  
> >  	selinux_disabled = 1;
> >  
> > -	/* Reset security_ops to the secondary module, dummy or capability. */
> > -	security_ops = secondary_ops;
> > +	/* Reset security_ops to the default */
> > +	security_ops = original_ops;
> >  
> >  	/* Unregister netfilter hooks. */
> >  	selinux_nf_ip_exit();
> 
> a runtime disable of SELinux by /sbin/init (upon finding
> SELINUX=disabled in /etc/selinux/config) ends up leaving us with null
> ops rather than the capability or dummy ops.  Thus, SELINUX=disabled
> turns off all security ;(

Ok, as with my prior comment, this one is also invalidated by the fact
that the static inlines fall back to the cap_ functions if the operation
is NULL.  So I suppose this would work.

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[Stephen Smalley]

On Fri, 2005-08-26 at 07:58 -0400, Stephen Smalley wrote:
> Ok, as with my prior comment, this one is also invalidated by the fact
> that the static inlines fall back to the cap_ functions if the operation
> is NULL.  So I suppose this would work.

Given these changes, what purpose does the capability module and the
CONFIG_SECURITY_CAPABILITIES option serve anymore?  Should capability.c
be removed entirely?

Also seems to obsolete the cap_stack module in the stacking patch set.

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[serue]

Quoting Tony Jones (tonyj@suse.de):
> On Fri, Aug 26, 2005 at 08:29:58AM -0400, Stephen Smalley wrote:
> > On Fri, 2005-08-26 at 07:58 -0400, Stephen Smalley wrote:
> > > Ok, as with my prior comment, this one is also invalidated by the fact
> > > that the static inlines fall back to the cap_ functions if the operation
> > > is NULL.  So I suppose this would work.
> > 
> > Given these changes, what purpose does the capability module and the
> > CONFIG_SECURITY_CAPABILITIES option serve anymore?  Should capability.c
> > be removed entirely?
> 
> Since stacker will implement every hook (preventing the static inline
> falling thru) wouldn't retaining capability as a module for composition 
> be useful?

For conceptual simplicity I think keeping an actual module for it around
will be best.  Then other module can either stack with it, or not,
however they prefer.

> Of course I can see alternate methods for implementing this.  At the very least,
> as this thread demonstrates, the current stacker approach of calling dummy
> when no submodule implements a hook will need some rework.

Actually that's not quite the way it works under stacker right now.
If no module is loaded, then dummy is used, but if a module is loaded,
then stacker doesn't call dummy__hook if the module doesn't define that
hook.  (Though there are a few hooks which are specially handled, ie
__vm_enough)

So switching from having dummy be the default module when nothing is
stacked, to having capability, is simple enough.

-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: [PATCH] remove selinux stacked ops

[Chris Wright]

* Stephen Smalley (sds@tycho.nsa.gov) wrote:
> On Fri, 2005-08-26 at 07:58 -0400, Stephen Smalley wrote:
> > Ok, as with my prior comment, this one is also invalidated by the fact
> > that the static inlines fall back to the cap_ functions if the operation
> > is NULL.  So I suppose this would work.
> 
> Given these changes, what purpose does the capability module and the
> CONFIG_SECURITY_CAPABILITIES option serve anymore?  Should capability.c
> be removed entirely?

I left it for two reasons.  Making it built-in will mean you can't
load another security module.  Some distros use the module already, so
it's compatibility.  These aren't the best reasons to keep it long term.

> Also seems to obsolete the cap_stack module in the stacking patch set.

I believe so.

thanks,
-chris

--
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: [PATCH] remove selinux stacked ops

[Stephen Smalley]

On Fri, 2005-08-26 at 09:22 -0700, Chris Wright wrote:
> I left it for two reasons.  Making it built-in will mean you can't
> load another security module.

CONFIG_SECURITY=n achieves the same effect.

>   Some distros use the module already, so
> it's compatibility.  

I'd assume that they only do that so that they can load something else
as primary, and then optionally stack capability under it.  Which they
can still achieve (just by modifying their primary to use the commoncap
functions directly).

> These aren't the best reasons to keep it long term.

Yes, it seems confusing to leave it.  I can easily see people leaving it
enabled as long as it remains without realizing that it is no longer
serving any purpose.  And it will definitely kick out an error message
if you leave SELinux+capability enabled together due to the failed
registration.

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[Stephen Smalley]

On Fri, 2005-08-26 at 07:54 -0500, serue@us.ibm.com wrote:
> For conceptual simplicity I think keeping an actual module for it around
> will be best.  Then other module can either stack with it, or not,
> however they prefer.

It seems like the direct use of the cap functions would be the preferred
way of combining other modules with the capability logic given that the
cap functions will now always be built-in.  Not using stacker at all for
that purpose.

> Actually that's not quite the way it works under stacker right now.
> If no module is loaded, then dummy is used, but if a module is loaded,
> then stacker doesn't call dummy__hook if the module doesn't define that
> hook.  (Though there are a few hooks which are specially handled, ie
> __vm_enough)
> 
> So switching from having dummy be the default module when nothing is
> stacked, to having capability, is simple enough.

Hmm...well, with these changes, we are looking at dropping the SELinux
hooks that only call the corresponding cap function, thereby falling
back to the default path in the static inlines (without stacker).  So
we'd expect the same behavior with stacker.

-- 
Stephen Smalley
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: [PATCH] remove selinux stacked ops

[Chris Wright]

* Stephen Smalley (sds@tycho.nsa.gov) wrote:
> On Fri, 2005-08-26 at 09:22 -0700, Chris Wright wrote:
> > I left it for two reasons.  Making it built-in will mean you can't
> > load another security module.
> 
> CONFIG_SECURITY=n achieves the same effect.

Yup, agreed (and if we could reduce performance impact that config could
go away ;-)

> >   Some distros use the module already, so
> > it's compatibility.  
> 
> I'd assume that they only do that so that they can load something else
> as primary, and then optionally stack capability under it.  Which they
> can still achieve (just by modifying their primary to use the commoncap
> functions directly).

No, they do it because they want capabilities, but for some strange
reason prefer to do everythign with modules.

> > These aren't the best reasons to keep it long term.
> 
> Yes, it seems confusing to leave it.  I can easily see people leaving it
> enabled as long as it remains without realizing that it is no longer
> serving any purpose.  And it will definitely kick out an error message
> if you leave SELinux+capability enabled together due to the failed
> registration.

True.  I'd prefer to have the whole of capabilities to simply be default
functionality always enabled.  Leaving it in there is not required for
that.

--
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: [PATCH] remove selinux stacked ops

[Chris Wright]

* Stephen Smalley (sds@tycho.nsa.gov) wrote:
> On Fri, 2005-08-26 at 07:48 -0400, Stephen Smalley wrote:
> > On Thu, 2005-08-25 at 15:51 -0700, Chris Wright wrote:
> > > @@ -3620,13 +3523,6 @@ static int selinux_netlink_send(struct s
> > >  	return err;
> > >  }
> > >  
> > > -static int selinux_netlink_recv(struct sk_buff *skb)
> > > -{
> > > -	if (!cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN))
> > > -		return -EPERM;
> > > -	return 0;
> > > -}
> > > -
> > 
> > Hmm...is removing this entirely safe?  Given that dummy is being
> > removed, and null ops are no longer being populated, don't we have to
> > ensure that we define a SELinux hook function for every cap_ function
> > and explicitly call that function?  Likewise, don't we now have to add a
> > settime hook to SELinux to preserve behavior?  Previously, it was
> > defaulting to the dummy hook, which conveniently called capable(), so it
> > ended up working regardless of whether SELinux was stacked with dummy or
> > capability.
> 
> Ah, never mind - I forgot that you are falling back to the cap_
> functions in your static inlines if the operation is null.  So you could
> also remove selinux_capset_set and selinux_task_post_setuid.

Thanks, I had done selinux_capset_set locally, but missed
selinux_task_post_setuid.

--
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.