Skip to contentSkip to conversation

selinux stacked ops

PATCH In review Chris Wright 4 participants 13 messages Aug 25, 2005 – Aug 26, 2005 1 message not in the archive SELinux security module

lore
Share & tools
lore

Chris Wright

author
#0 lore raw

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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

author
#1 lore raw

PATCH 1/1 remove selinux stacked ops

Quoted 2 lines · * Chris Wright ([email protected]) 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...

security/selinux/hooks.c +19 -166
@@ -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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

#2 lore raw

Re: [PATCH] remove selinux stacked ops

security/selinux/hooks.c static int selinux_netlink_send(struct s · line 3629

Quoted 11 lines · Chris Wright
security/selinux/hooks.c +0 -7
@@ -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.

security/selinux/hooks.c static struct security_operations selinu · line 4281

Quoted 5 lines
security/selinux/hooks.c +0 -1
@@ -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.

security/selinux/hooks.c static __init int selinux_init(void) · line 4293

Quoted 10 lines
security/selinux/hooks.c +1 -3
@@ -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...

security/selinux/hooks.c int selinux_disable(void) · line 4428

Quoted 11 lines
security/selinux/hooks.c +2 -2
@@ -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 ;(

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#3 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 5 lines · Stephen Smalley
Quoted 4 lines · Chris Wright
security/selinux/hooks.c +0 -0
@@ -3620,13 +3523,6 @@ static int selinux_netlink_send(struct s
 	return err;
 }
Quoted 16 lines

security/selinux/hooks.c static int selinux_netlink_send(struct s · line 3629

Quoted 7 lines
security/selinux/hooks.c +0 -7
-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.

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
author
#4 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 23 lines · * Stephen Smalley ([email protected]) wrote:
Quoted 5 lines · Stephen Smalley
Quoted 4 lines · Chris Wright
security/selinux/hooks.c +0 -0
@@ -3620,13 +3523,6 @@ static int selinux_netlink_send(struct s
 	return err;
 }
Quoted 16 lines

security/selinux/hooks.c static int selinux_netlink_send(struct s · line 3629

Quoted 7 lines
security/selinux/hooks.c +0 -7
-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.

Quoted 3 lines

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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

#5 lore raw

Re: [PATCH] remove selinux stacked ops

In reply to Stephen Smalley
Quoted 9 lines · Stephen Smalley
Quoted 9 lines
security/selinux/hooks.c +2 -2
@@ -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;
Quoted 7 lines

security/selinux/hooks.c int selinux_disable(void) · line 4428

Quoted 2 lines
security/selinux/hooks.c +0 -0
 	/* 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.

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
#6 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 3 lines · Stephen Smalley

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.

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#8 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 6 lines · Tony Jones
Quoted 5 lines · Stephen Smalley
Quoted 3 lines · Stephen Smalley

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.

Quoted 7 lines
Quoted 3 lines

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.

Quoted 3 lines

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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

#9 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 3 lines · On Fri, 2005-08-26 at 07:54 -0500, [email protected] 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.

Quoted 8 lines

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.

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
author
#10 lore raw

Re: [PATCH] remove selinux stacked ops

In reply to Stephen Smalley
Quoted 8 lines · * Stephen Smalley ([email protected]) wrote:
Quoted 3 lines · Stephen Smalley

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.

Quoted 1 line

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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

#11 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 2 lines · Chris Wright

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.

Quoted 2 lines

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

Quoted 1 line

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.

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.
author
#12 lore raw

Re: [PATCH] remove selinux stacked ops

Quoted 5 lines · * Stephen Smalley ([email protected]) wrote:
Quoted 2 lines · Chris Wright

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 ;-)

Quoted 7 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 1 line

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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

#13 lore raw
In reply to Chris Wright
Quoted 4 lines · Chris Wright

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.

Quoted 2 lines

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.

Signature · 8 lines
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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.