Re: [PATCH 2/5] Rework stubs in security.h

2005-08-26 Thread Tony Jones
On Fri, Aug 26, 2005 at 02:00:56PM -0400, Stephen Smalley wrote:
> 
> That makes capability part of the core kernel again, just like DAC,
> which means that you can never override a capability denial in your
> module.  We sometimes want to override the capability implementation,
> not just apply further restrictions after it.  cap_inode_setxattr and
> cap_inode_removexattr are examples; they prohibit any access to _all_

Right, the rationale behind cap_stack.c.  Good point.  I'd forgotten that.

I guess selective internal composition is the way to go.

Tony
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] Rework stubs in security.h

2005-08-26 Thread Chris Wright
* Stephen Smalley ([EMAIL PROTECTED]) wrote:
> That one isn't so much an issue as the xattr ones and vm_enough_memory
> case.  But more generally, if you think about moving toward a place
> where one can grant privileges to processes based solely on their
> role/domain, you'll need the same ability for capable and other hooks
> too.  Naturally, that can't be done safely without a lot more work on
> userspace and policy, but it is a long term goal.

Right, you've mentioned that before, thanks for bringing that up.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] Rework stubs in security.h

2005-08-26 Thread Tony Jones
On Fri, Aug 26, 2005 at 10:59:52AM -0700, Chris Wright wrote:
> * Tony Jones ([EMAIL PROTECTED]) wrote:
> > The discussion about composing with commoncap made me think about whether
> > this is the best way to do this.   It seems that we're heading towards a
> > requirement that every module internally compose with commoncap.  
> 
> Not a requirement, it's a choice ATM.

Can you think of a case where the choice (on the part of the module author)
to not do so makes any sense?

> Heh, this was next on my list.  I just wanted to separate the changes to
> one at a time so we can easily measure the impact.  This becomes another
> policy shift.

I understand the advantages of clearly measuring the impact of each change
independantly but with SELinux having to change how they use their secondaries 
at the same time, I'm not sure there is a lot of difference between the 
two phases.

> > If every module is already internally composing, there shouldn't be a 
> > performance cost for the additional branch inside the #ifdef.
> 
> This needs measurement to verify.

Agreed.  So does the current proposal :-)

Tony
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] Rework stubs in security.h

2005-08-26 Thread Stephen Smalley
On Fri, 2005-08-26 at 10:31 -0700, Tony Jones wrote:
> On Wed, Aug 24, 2005 at 06:20:30PM -0700, Chris Wright wrote:
> 
> >  static inline int security_ptrace (struct task_struct * parent, struct 
> > task_struct * child)
> >  {
> > +#ifdef CONFIG_SECURITY
> > return security_ops->ptrace (parent, child);
> > +#else
> > +   return cap_ptrace (parent, child);
> > +#endif
> > +
> >  }

With the third patch applied, it looks like this instead:
static inline int security_ptrace (struct task_struct * parent, struct 
task_struct * child)
{
#ifdef CONFIG_SECURITY
if (security_ops->ptrace)
return security_ops->ptrace(parent, child);
#endif
return cap_ptrace (parent, child);

}

> The discussion about composing with commoncap made me think about whether
> this is the best way to do this.   It seems that we're heading towards a
> requirement that every module internally compose with commoncap.
>   
> 
> If so (apart from the obvious correctness issues when they don't) it's work
> for each module and composing N of them under stacker obviously creates 
> overhead.

Only matters if there are two or more modules that need to be used
together and both need to override/supplement the capability logic for a
given hook.  

> Would the following not be a better approach?
> 
> static inline int security_ptrace (struct task_struct * parent, struct 
> task_struct * child)
> {
> int ret;
>   ret=cap_ptrace (parent, child);
> #ifdef CONFIG_SECURITY
>   if (!ret && security_ops->ptrace)
>   ret=security_ops->ptrace(parent, child);
> #endif
>   return ret;
> }

That makes capability part of the core kernel again, just like DAC,
which means that you can never override a capability denial in your
module.  We sometimes want to override the capability implementation,
not just apply further restrictions after it.  cap_inode_setxattr and
cap_inode_removexattr are examples; they prohibit any access to _all_
security attributes without CAP_SYS_ADMIN, whereas SELinux wants to
allow access to security.selinux if you pass a certain set of its own
permission checks.  vm_enough_memory is another problem area due to vm
accounting handled internally.

> If every module is already internally composing, there shouldn't be a 
> performance cost for the additional branch inside the #ifdef.
> 
> I havn't looked at every single hook and it's users to see if this would
> cause a problem.  I noticed SELinux calls sec->capget() post rather than pre 
> it's processing which may be an issue.

That one isn't so much an issue as the xattr ones and vm_enough_memory
case.  But more generally, if you think about moving toward a place
where one can grant privileges to processes based solely on their
role/domain, you'll need the same ability for capable and other hooks
too.  Naturally, that can't be done safely without a lot more work on
userspace and policy, but it is a long term goal.

-- 
Stephen Smalley
National Security Agency

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] Rework stubs in security.h

2005-08-26 Thread Chris Wright
* Tony Jones ([EMAIL PROTECTED]) wrote:
> The discussion about composing with commoncap made me think about whether
> this is the best way to do this.   It seems that we're heading towards a
> requirement that every module internally compose with commoncap.  

Not a requirement, it's a choice ATM.

> If so (apart from the obvious correctness issues when they don't) it's work
> for each module and composing N of them under stacker obviously creates 
> overhead.
> 
> Would the following not be a better approach?
> 
> static inline int security_ptrace (struct task_struct * parent, struct 
> task_struct * child)
> {
> int ret;
>   ret=cap_ptrace (parent, child);
> #ifdef CONFIG_SECURITY
>   if (!ret && security_ops->ptrace)
>   ret=security_ops->ptrace(parent, child);
> #endif
>   return ret;
> }

Heh, this was next on my list.  I just wanted to separate the changes to
one at a time so we can easily measure the impact.  This becomes another
policy shift.

> If every module is already internally composing, there shouldn't be a 
> performance cost for the additional branch inside the #ifdef.

This needs measurement to verify.

> I havn't looked at every single hook and it's users to see if this would
> cause a problem.  I noticed SELinux calls sec->capget() post rather than pre 
> it's processing which may be an issue.

Yes, that need careful inspection.

thanks,
-chris
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] Rework stubs in security.h

2005-08-26 Thread Tony Jones
On Wed, Aug 24, 2005 at 06:20:30PM -0700, Chris Wright wrote:

>  static inline int security_ptrace (struct task_struct * parent, struct 
> task_struct * child)
>  {
> +#ifdef CONFIG_SECURITY
>   return security_ops->ptrace (parent, child);
> +#else
> + return cap_ptrace (parent, child);
> +#endif
> +
>  }

The discussion about composing with commoncap made me think about whether
this is the best way to do this.   It seems that we're heading towards a
requirement that every module internally compose with commoncap.  

If so (apart from the obvious correctness issues when they don't) it's work
for each module and composing N of them under stacker obviously creates 
overhead.

Would the following not be a better approach?

static inline int security_ptrace (struct task_struct * parent, struct 
task_struct * child)
{
int ret;
ret=cap_ptrace (parent, child);
#ifdef CONFIG_SECURITY
if (!ret && security_ops->ptrace)
ret=security_ops->ptrace(parent, child);
#endif
return ret;
}

If every module is already internally composing, there shouldn't be a 
performance cost for the additional branch inside the #ifdef.

I havn't looked at every single hook and it's users to see if this would
cause a problem.  I noticed SELinux calls sec->capget() post rather than pre 
it's processing which may be an issue.

Tony
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/5] Rework stubs in security.h

2005-08-24 Thread Chris Wright
Collapse security stubs so that the def'n is done in one spot with ifdef
in function body rather than two separately defined functions.

Patch from Kurt Garloff <[EMAIL PROTECTED]>, and slightly altered by me to
make all ifdef sites consistent and move the prototype decl's to a sane
spot.

Signed-off-by: Kurt Garloff <[EMAIL PROTECTED]>
Signed-off-by: Chris Wright <[EMAIL PROTECTED]>
---

 include/linux/security.h | 1410 ---
 1 files changed, 609 insertions(+), 801 deletions(-)

Index: lsm-hooks-2.6/include/linux/security.h
===
--- lsm-hooks-2.6.orig/include/linux/security.h
+++ lsm-hooks-2.6/include/linux/security.h
@@ -1248,10 +1248,27 @@ struct security_operations {
 /* global variables */
 extern struct security_operations *security_ops;
 
-/* inline stuff */
+/* prototypes */
+extern int security_init   (void);
+extern int register_security   (struct security_operations *ops);
+extern int unregister_security (struct security_operations *ops);
+extern int mod_reg_security(const char *name, struct security_operations 
*ops);
+extern int mod_unreg_security  (const char *name, struct security_operations 
*ops);
+#else
+static inline int security_init(void)
+{
+   return 0;
+}
+#endif /* CONFIG_SECURITY */
+
 static inline int security_ptrace (struct task_struct * parent, struct 
task_struct * child)
 {
+#ifdef CONFIG_SECURITY
return security_ops->ptrace (parent, child);
+#else
+   return cap_ptrace (parent, child);
+#endif
+
 }
 
 static inline int security_capget (struct task_struct *target,
@@ -1259,7 +1281,11 @@ static inline int security_capget (struc
   kernel_cap_t *inheritable,
   kernel_cap_t *permitted)
 {
+#ifdef CONFIG_SECURITY
return security_ops->capget (target, effective, inheritable, permitted);
+#else
+   return cap_capget (target, effective, inheritable, permitted);
+#endif
 }
 
 static inline int security_capset_check (struct task_struct *target,
@@ -1267,7 +1293,11 @@ static inline int security_capset_check 
 kernel_cap_t *inheritable,
 kernel_cap_t *permitted)
 {
+#ifdef CONFIG_SECURITY
return security_ops->capset_check (target, effective, inheritable, 
permitted);
+#else
+   return cap_capset_check (target, effective, inheritable, permitted);
+#endif 
 }
 
 static inline void security_capset_set (struct task_struct *target,
@@ -1275,278 +1305,457 @@ static inline void security_capset_set (
kernel_cap_t *inheritable,
kernel_cap_t *permitted)
 {
+#ifdef CONFIG_SECURITY
security_ops->capset_set (target, effective, inheritable, permitted);
+#else
+   cap_capset_set (target, effective, inheritable, permitted);
+#endif
 }
 
 static inline int security_acct (struct file *file)
 {
+#ifdef CONFIG_SECURITY
return security_ops->acct (file);
+#else
+   return 0;
+#endif
 }
 
 static inline int security_sysctl(struct ctl_table *table, int op)
 {
+#ifdef CONFIG_SECURITY
return security_ops->sysctl(table, op);
+#else
+   return 0;
+#endif
 }
 
 static inline int security_quotactl (int cmds, int type, int id,
 struct super_block *sb)
 {
+#ifdef CONFIG_SECURITY
return security_ops->quotactl (cmds, type, id, sb);
+#else
+   return 0;
+#endif
 }
 
 static inline int security_quota_on (struct dentry * dentry)
 {
+#ifdef CONFIG_SECURITY
return security_ops->quota_on (dentry);
+#else
+   return 0;
+#endif
 }
 
 static inline int security_syslog(int type)
 {
+#ifdef CONFIG_SECURITY
return security_ops->syslog(type);
+#else
+   return cap_syslog(type);
+#endif
 }
 
 static inline int security_settime(struct timespec *ts, struct timezone *tz)
 {
+#ifdef CONFIG_SECURITY
return security_ops->settime(ts, tz);
+#else
+   return cap_settime(ts, tz);
+#endif
 }
 
-
 static inline int security_vm_enough_memory(long pages)
 {
+#ifdef CONFIG_SECURITY
return security_ops->vm_enough_memory(pages);
+#else
+   return cap_vm_enough_memory(pages);
+#endif
 }
 
 static inline int security_bprm_alloc (struct linux_binprm *bprm)
 {
+#ifdef CONFIG_SECURITY
return security_ops->bprm_alloc_security (bprm);
+#else
+   return 0;
+#endif
 }
+
 static inline void security_bprm_free (struct linux_binprm *bprm)
 {
+#ifdef CONFIG_SECURITY
security_ops->bprm_free_security (bprm);
+#else
+   return;
+#endif
 }
+
 static inline void security_bprm_apply_creds (struct linux_binprm *bprm, int 
unsafe)
 {
+#ifdef CONFIG_SECURITY
security_ops->bprm_apply_creds (bprm, unsafe);
+#else
+   cap_bprm_apply_creds (bprm, unsafe);
+#endif
 }
+
 static inline void security_bprm_post_apply_creds (struct linux_binprm