Re: [PATCH 2/5] Rework stubs in security.h
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
* 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
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
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
* 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
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
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