Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932576Ab1FQP3Y (ORCPT ); Fri, 17 Jun 2011 11:29:24 -0400 Received: from mail-bw0-f46.google.com ([209.85.214.46]:56426 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932492Ab1FQP3U (ORCPT ); Fri, 17 Jun 2011 11:29:20 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:date:from:to:subject:message-id:mime-version:content-type :content-disposition:user-agent; b=vMsi3MmAXPBVM+jQJFOBzTx4CrBQZCNcXxtRq14JT8ShiyZUWefPIpOtGEZLDPmzrb k/hwpwrUjw2UM6l/AT5GZHfzIYZT56jTFGQpkSUmijPwtIAmPGr67Loi1VCubiwZLgv6 IrD6GwJALP4689DHrzPiTRBpMMGVd/Am5aFfI= Date: Fri, 17 Jun 2011 19:29:12 +0400 From: Vasiliy Kulikov To: linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, apparmor@lists.ubuntu.com, "selinux@tycho.nsa.gov Stephen Smalley" , James Morris , Eric Paris , John Johansen , kernel-hardening@lists.openwall.com Subject: [RFC v1] security: introduce ptrace_task_access_check() Message-ID: <20110617152912.GA21885@albatros> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 14306 Lines: 405 Hi, This patch introduces ptrace_task_access_check() to be able to check whether a specific task (not current) is able to ptrace another task (might be current). I need it to call "reversed" ptrace_may_access() with swapped current and target task. Specifically, I need it to filter taskstats and proc connector requests for a restriction of getting other processes' information: http://permalink.gmane.org/gmane.linux.kernel/1155354 Please help me to figure out how such patch should be divided to be applied. I think about such scheme: 1) add generic security/* functions. 2-4) add ptrace_task_access_check() for SMACK, AppArmor and SELinux. 5) change ptrace_access_check() in security ops and all LSMs to ptrace_task_access_check(). But I'd like to hear maintainers' oppinions not to put useless efforts. Thanks, --- include/linux/capability.h | 2 + include/linux/security.h | 20 +++++++++++++++++- kernel/capability.c | 24 ++++++++++++++++++++++ security/apparmor/lsm.c | 10 ++++---- security/capability.c | 2 +- security/commoncap.c | 20 +++++++++++++++++++ security/security.c | 11 +++++++-- security/selinux/hooks.c | 13 +++++------ security/smack/smack.h | 1 + security/smack/smack_access.c | 43 +++++++++++++++++++++++++++++++++++++++++ security/smack/smack_lsm.c | 9 ++++--- 11 files changed, 133 insertions(+), 22 deletions(-) diff --git a/include/linux/capability.h b/include/linux/capability.h index c421123..cc0bcfe 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -544,7 +544,9 @@ extern bool has_ns_capability(struct task_struct *t, struct user_namespace *ns, int cap); extern bool has_capability_noaudit(struct task_struct *t, int cap); extern bool capable(int cap); +extern bool task_capable(struct task_struct *task, int cap); extern bool ns_capable(struct user_namespace *ns, int cap); +extern bool ns_task_capable(struct task_struct *t, struct user_namespace *ns, int cap); extern bool task_ns_capable(struct task_struct *t, int cap); extern bool nsown_capable(int cap); diff --git a/include/linux/security.h b/include/linux/security.h index 8ce59ef..88a2351 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -57,6 +57,8 @@ extern int cap_capable(struct task_struct *tsk, const struct cred *cred, struct user_namespace *ns, int cap, int audit); extern int cap_settime(const struct timespec *ts, const struct timezone *tz); extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode); +extern int cap_ptrace_task_access_check(struct task_struct *task, struct task_struct *child, + unsigned int mode); extern int cap_ptrace_traceme(struct task_struct *parent); extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); extern int cap_capset(struct cred *new, const struct cred *old, @@ -1375,7 +1377,9 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) struct security_operations { char name[SECURITY_NAME_MAX + 1]; - int (*ptrace_access_check) (struct task_struct *child, unsigned int mode); + int (*ptrace_task_access_check) (struct task_struct *task, + struct task_struct *child, + unsigned int mode); int (*ptrace_traceme) (struct task_struct *parent); int (*capget) (struct task_struct *target, kernel_cap_t *effective, @@ -1667,6 +1671,10 @@ int security_capset(struct cred *new, const struct cred *old, const kernel_cap_t *effective, const kernel_cap_t *inheritable, const kernel_cap_t *permitted); +int security_task_capable(struct task_struct *task, + struct user_namespace *ns, + const struct cred *cred, + int cap); int security_capable(struct user_namespace *ns, const struct cred *cred, int cap); int security_real_capable(struct task_struct *tsk, struct user_namespace *ns, @@ -1865,10 +1873,18 @@ static inline int security_capset(struct cred *new, return cap_capset(new, old, effective, inheritable, permitted); } +static inline int security_task_capable(struct task_struct *task, + struct user_namespace *ns, + const struct cred *cred, + int cap) +{ + return cap_capable(task, cred, ns, cap, SECURITY_CAP_AUDIT); +} + static inline int security_capable(struct user_namespace *ns, const struct cred *cred, int cap) { - return cap_capable(current, cred, ns, cap, SECURITY_CAP_AUDIT); + return security_task_capable(current, ns, cred, cap); } static inline int security_real_capable(struct task_struct *tsk, struct user_namespace *ns, int cap) diff --git a/kernel/capability.c b/kernel/capability.c index 283c529..6a2d636 100644 --- a/kernel/capability.c +++ b/kernel/capability.c @@ -356,6 +356,30 @@ bool capable(int cap) } EXPORT_SYMBOL(capable); +bool task_capable(struct task_struct *task, int cap) +{ + return ns_task_capable(task, &init_user_ns, cap); +} +EXPORT_SYMBOL(capable); + +bool ns_task_capable(struct task_struct *task, struct user_namespace *ns, int cap) +{ + if (unlikely(!cap_valid(cap))) { + printk(KERN_CRIT "capable() called with invalid cap=%u\n", cap); + BUG(); + } + + rcu_read_lock(); + if (security_task_capable(task, ns, __task_cred(task), cap) == 0) { + rcu_read_unlock(); + current->flags |= PF_SUPERPRIV; + return true; + } + rcu_read_unlock(); + return false; +} +EXPORT_SYMBOL(ns_task_capable); + /** * ns_capable - Determine if the current task has a superior capability in effect * @ns: The usernamespace we want the capability in diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ec1bcec..707f087 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -93,14 +93,14 @@ static void apparmor_cred_transfer(struct cred *new, const struct cred *old) aa_dup_task_context(new_cxt, old_cxt); } -static int apparmor_ptrace_access_check(struct task_struct *child, - unsigned int mode) +static int apparmor_ptrace_task_access_check(struct task_struct *task, + struct task_struct *child, unsigned int mode) { - int error = cap_ptrace_access_check(child, mode); + int error = cap_ptrace_task_access_check(task, child, mode); if (error) return error; - return aa_ptrace(current, child, mode); + return aa_ptrace(task, child, mode); } static int apparmor_ptrace_traceme(struct task_struct *parent) @@ -624,7 +624,7 @@ static int apparmor_task_setrlimit(struct task_struct *task, static struct security_operations apparmor_ops = { .name = "apparmor", - .ptrace_access_check = apparmor_ptrace_access_check, + .ptrace_task_access_check = apparmor_ptrace_task_access_check, .ptrace_traceme = apparmor_ptrace_traceme, .capget = apparmor_capget, .capable = apparmor_capable, diff --git a/security/capability.c b/security/capability.c index bbb5115..1bdbe44 100644 --- a/security/capability.c +++ b/security/capability.c @@ -874,7 +874,7 @@ static void cap_audit_rule_free(void *lsmrule) void __init security_fixup_ops(struct security_operations *ops) { - set_to_cap_if_null(ops, ptrace_access_check); + set_to_cap_if_null(ops, ptrace_task_access_check); set_to_cap_if_null(ops, ptrace_traceme); set_to_cap_if_null(ops, capget); set_to_cap_if_null(ops, capset); diff --git a/security/commoncap.c b/security/commoncap.c index a93b3b7..aa76791 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -155,6 +155,26 @@ out: return ret; } +int cap_ptrace_task_access_check(struct task_struct *task, struct task_struct *child, + unsigned int mode) +{ + int ret = 0; + const struct cred *cred, *child_cred; + + rcu_read_lock(); + cred = __task_cred(task); + child_cred = __task_cred(child); + if (cred->user->user_ns == child_cred->user->user_ns && + cap_issubset(child_cred->cap_permitted, cred->cap_permitted)) + goto out; + if (ns_task_capable(task, child_cred->user->user_ns, CAP_SYS_PTRACE)) + goto out; + ret = -EPERM; +out: + rcu_read_unlock(); + return ret; +} + /** * cap_ptrace_traceme - Determine whether another process may trace the current * @parent: The task proposed to be the tracer diff --git a/security/security.c b/security/security.c index 4ba6d4c..d51330b 100644 --- a/security/security.c +++ b/security/security.c @@ -129,7 +129,7 @@ int __init register_security(struct security_operations *ops) int security_ptrace_access_check(struct task_struct *child, unsigned int mode) { - return security_ops->ptrace_access_check(child, mode); + return security_ops->ptrace_task_access_check(current, child, mode); } int security_ptrace_traceme(struct task_struct *parent) @@ -154,11 +154,16 @@ int security_capset(struct cred *new, const struct cred *old, effective, inheritable, permitted); } +int security_task_capable(struct task_struct *task, struct user_namespace *ns, + const struct cred *cred, int cap) +{ + return security_ops->capable(task, cred, ns, cap, SECURITY_CAP_AUDIT); +} + int security_capable(struct user_namespace *ns, const struct cred *cred, int cap) { - return security_ops->capable(current, cred, ns, cap, - SECURITY_CAP_AUDIT); + return security_task_capable(current, ns, cred, cap); } int security_real_capable(struct task_struct *tsk, struct user_namespace *ns, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 20219ef..ee963d1 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1804,23 +1804,22 @@ static inline u32 open_file_to_av(struct file *file) } /* Hook functions begin here. */ - -static int selinux_ptrace_access_check(struct task_struct *child, - unsigned int mode) +static int selinux_ptrace_task_access_check(struct task_struct *task, + struct task_struct *child, unsigned int mode) { int rc; - rc = cap_ptrace_access_check(child, mode); + rc = cap_ptrace_task_access_check(task, child, mode); if (rc) return rc; if (mode == PTRACE_MODE_READ) { - u32 sid = current_sid(); + u32 sid = task_sid(task); u32 csid = task_sid(child); return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ, NULL); } - return current_has_perm(child, PROCESS__PTRACE); + return task_has_perm(task, child, PROCESS__PTRACE); } static int selinux_ptrace_traceme(struct task_struct *parent) @@ -5457,7 +5456,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) static struct security_operations selinux_ops = { .name = "selinux", - .ptrace_access_check = selinux_ptrace_access_check, + .ptrace_task_access_check = selinux_ptrace_task_access_check, .ptrace_traceme = selinux_ptrace_traceme, .capget = selinux_capget, .capset = selinux_capset, diff --git a/security/smack/smack.h b/security/smack/smack.h index 2b6c6a5..4d9fb0f 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -199,6 +199,7 @@ struct inode_smack *new_inode_smack(char *); */ int smk_access_entry(char *, char *, struct list_head *); int smk_access(char *, char *, int, struct smk_audit_info *); +int smk_taskacc(struct task_struct *, char *, u32, struct smk_audit_info *); int smk_curacc(char *, u32, struct smk_audit_info *); int smack_to_cipso(const char *, struct smack_cipso *); void smack_from_cipso(u32, char *, char *); diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 9637e10..f6582a7 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -200,6 +200,49 @@ out_audit: return rc; } +int smk_taskacc(struct task_struct *task, char *obj_label, u32 mode, struct smk_audit_info *a) +{ + struct task_smack *tsp = task_cred_xxx(task, security); + char *subject_label = smk_of_task(tsp); + int may; + int rc; + + /* + * Check the global rule list + */ + rc = smk_access(subject_label, obj_label, mode, NULL); + if (rc == 0) { + /* + * If there is an entry in the task's rule list + * it can further restrict access. + */ + may = smk_access_entry(subject_label, obj_label, &tsp->smk_rules); + if (may < 0) + goto out_audit; + if ((mode & may) == mode) + goto out_audit; + rc = -EACCES; + } + + /* + * Return if a specific label has been designated as the + * only one that gets privilege and current does not + * have that label. + */ + if (smack_onlycap != NULL && smack_onlycap != subject_label) + goto out_audit; + + if (task_capable(task, CAP_MAC_OVERRIDE)) + rc = 0; + +out_audit: +#ifdef CONFIG_AUDIT + if (a) + smack_log(subject_label, obj_label, mode, rc, a); +#endif + return rc; +} + /** * smk_curacc - determine if current has a specific access to an object * @obj_label: a pointer to the object's Smack label diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 9831a39..d168974 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -149,13 +149,14 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead, * * Do the capability checks, and require read and write. */ -static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode) +static int smack_ptrace_task_access_check(struct task_struct *task, + struct task_struct *ctp, unsigned int mode) { int rc; struct smk_audit_info ad; char *tsp; - rc = cap_ptrace_access_check(ctp, mode); + rc = cap_ptrace_task_access_check(task, ctp, mode); if (rc != 0) return rc; @@ -163,7 +164,7 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode) smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK); smk_ad_setfield_u_tsk(&ad, ctp); - rc = smk_curacc(tsp, MAY_READWRITE, &ad); + rc = smk_taskacc(task, tsp, MAY_READWRITE, &ad); return rc; } @@ -3395,7 +3396,7 @@ static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) struct security_operations smack_ops = { .name = "smack", - .ptrace_access_check = smack_ptrace_access_check, + .ptrace_task_access_check = smack_ptrace_task_access_check, .ptrace_traceme = smack_ptrace_traceme, .syslog = smack_syslog, --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/