* [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants
@ 2021-03-18 20:42 Paul Moore
2021-03-18 20:42 ` [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants Paul Moore
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Paul Moore @ 2021-03-18 20:42 UTC (permalink / raw)
To: linux-security-module, linux-audit, selinux
Cc: Richard Guy Briggs, John Johansen
An update on the previous RFC patchset found here:
https://lore.kernel.org/linux-security-module/161377712068.87807.12246856567527156637.stgit@sifl/
Aside from being rebased to the current SELinux next branch (which
in turn is based on v5.12-rc2), this revision changes the binder
related code to always use the objective credentials as discussed
in the thread above. I've also dropped the AppArmor patch as John
has a better version in progress; in the meantime AppArmor should
continue to work exactly as it did before this patchset so there
is no harm in merging this without the AppArmor patch.
Casey, John, and Richard; I dropped your ACKs, Reviewed-by, etc.
tags as the binder changes seemed substantial enough to not
carryover your tags. I would appreciate it if you could re-review
this revision; the changes should be minimal. I did leave Mimi's
tag on this revision as she qualified it for IMA and that code
didn't change.
---
Paul Moore (3):
lsm: separate security_task_getsecid() into subjective and objective variants
selinux: clarify task subjective and objective credentials
smack: differentiate between subjective and objective task credentials
security/selinux/hooks.c | 112 ++++++++++++++++++++++++-------------
security/smack/smack.h | 18 +++++-
security/smack/smack_lsm.c | 40 ++++++++-----
3 files changed, 117 insertions(+), 53 deletions(-)
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants
2021-03-18 20:42 [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
@ 2021-03-18 20:42 ` Paul Moore
2021-03-18 20:57 ` Casey Schaufler
2021-03-19 13:35 ` Richard Guy Briggs
2021-03-18 20:42 ` [PATCH v2 2/3] selinux: clarify task subjective and objective credentials Paul Moore
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Paul Moore @ 2021-03-18 20:42 UTC (permalink / raw)
To: linux-security-module, linux-audit, selinux
Cc: Richard Guy Briggs, John Johansen
Of the three LSMs that implement the security_task_getsecid() LSM
hook, all three LSMs provide the task's objective security
credentials. This turns out to be unfortunate as most of the hook's
callers seem to expect the task's subjective credentials, although
a small handful of callers do correctly expect the objective
credentials.
This patch is the first step towards fixing the problem: it splits
the existing security_task_getsecid() hook into two variants, one
for the subjective creds, one for the objective creds.
void security_task_getsecid_subj(struct task_struct *p,
u32 *secid);
void security_task_getsecid_obj(struct task_struct *p,
u32 *secid);
While this patch does fix all of the callers to use the correct
variant, in order to keep this patch focused on the callers and to
ease review, the LSMs continue to use the same implementation for
both hooks. The net effect is that this patch should not change
the behavior of the kernel in any way, it will be up to the latter
LSM specific patches in this series to change the hook
implementations and return the correct credentials.
Acked-by: Mimi Zohar <zohar@linux.ibm.com> (IMA)
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
drivers/android/binder.c | 11 ++++++++++-
include/linux/cred.h | 2 +-
include/linux/lsm_hook_defs.h | 5 ++++-
include/linux/lsm_hooks.h | 12 +++++++++---
include/linux/security.h | 10 ++++++++--
kernel/audit.c | 4 ++--
kernel/auditfilter.c | 3 ++-
kernel/auditsc.c | 8 ++++----
kernel/bpf/bpf_lsm.c | 3 ++-
net/netlabel/netlabel_unlabeled.c | 2 +-
net/netlabel/netlabel_user.h | 2 +-
security/apparmor/lsm.c | 3 ++-
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_main.c | 14 +++++++-------
security/security.c | 13 ++++++++++---
security/selinux/hooks.c | 3 ++-
security/smack/smack_lsm.c | 3 ++-
17 files changed, 68 insertions(+), 32 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c119736ca56a..61d235b6ccd8 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2700,7 +2700,16 @@ static void binder_transaction(struct binder_proc *proc,
u32 secid;
size_t added_size;
- security_task_getsecid(proc->tsk, &secid);
+ /*
+ * Arguably this should be the task's subjective LSM secid but
+ * we can't reliably access the subjective creds of a task
+ * other than our own so we must use the objective creds, which
+ * are safe to access. The downside is that if a task is
+ * temporarily overriding it's creds it will not be reflected
+ * here; however, it isn't clear that binder would handle that
+ * case well anyway.
+ */
+ security_task_getsecid_obj(proc->tsk, &secid);
ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
if (ret) {
return_error = BR_FAILED_REPLY;
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 4c6350503697..ac0e5f97d7d8 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -140,7 +140,7 @@ struct cred {
struct key *request_key_auth; /* assumed request_key authority */
#endif
#ifdef CONFIG_SECURITY
- void *security; /* subjective LSM security */
+ void *security; /* LSM security */
#endif
struct user_struct *user; /* real user ID subscription */
struct user_namespace *user_ns; /* user_ns the caps and keyrings are relative to. */
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 477a597db013..3ad8085e85e1 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -203,7 +203,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new, const struct cred * old,
LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
-LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32 *secid)
+LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
+ struct task_struct *p, u32 *secid)
+LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
+ struct task_struct *p, u32 *secid)
LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)
LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio)
LSM_HOOK(int, 0, task_getioprio, struct task_struct *p)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index fb7f3193753d..e25d31fe787e 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -707,9 +707,15 @@
* @p.
* @p contains the task_struct for the process.
* Return 0 if permission is granted.
- * @task_getsecid:
- * Retrieve the security identifier of the process @p.
- * @p contains the task_struct for the process and place is into @secid.
+ * @task_getsecid_subj:
+ * Retrieve the subjective security identifier of the task_struct in @p
+ * and return it in @secid. Special care must be taken to ensure that @p
+ * is the either the "current" task, or the caller has exclusive access
+ * to @p.
+ * In case of failure, @secid will be set to zero.
+ * @task_getsecid_obj:
+ * Retrieve the objective security identifier of the task_struct in @p
+ * and return it in @secid.
* In case of failure, @secid will be set to zero.
*
* @task_setnice:
diff --git a/include/linux/security.h b/include/linux/security.h
index 8aeebd6646dc..9c490bc437f5 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -414,7 +414,8 @@ int security_task_fix_setgid(struct cred *new, const struct cred *old,
int security_task_setpgid(struct task_struct *p, pid_t pgid);
int security_task_getpgid(struct task_struct *p);
int security_task_getsid(struct task_struct *p);
-void security_task_getsecid(struct task_struct *p, u32 *secid);
+void security_task_getsecid_subj(struct task_struct *p, u32 *secid);
+void security_task_getsecid_obj(struct task_struct *p, u32 *secid);
int security_task_setnice(struct task_struct *p, int nice);
int security_task_setioprio(struct task_struct *p, int ioprio);
int security_task_getioprio(struct task_struct *p);
@@ -1098,7 +1099,12 @@ static inline int security_task_getsid(struct task_struct *p)
return 0;
}
-static inline void security_task_getsecid(struct task_struct *p, u32 *secid)
+static inline void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
+{
+ *secid = 0;
+}
+
+static inline void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
{
*secid = 0;
}
diff --git a/kernel/audit.c b/kernel/audit.c
index 551a394bc8f4..121d37e700a6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2132,7 +2132,7 @@ int audit_log_task_context(struct audit_buffer *ab)
int error;
u32 sid;
- security_task_getsecid(current, &sid);
+ security_task_getsecid_subj(current, &sid);
if (!sid)
return 0;
@@ -2353,7 +2353,7 @@ int audit_signal_info(int sig, struct task_struct *t)
audit_sig_uid = auid;
else
audit_sig_uid = uid;
- security_task_getsecid(current, &audit_sig_sid);
+ security_task_getsecid_subj(current, &audit_sig_sid);
}
return audit_signal_info_syscall(t);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 333b3bcfc545..db2c6b59dfc3 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1359,7 +1359,8 @@ int audit_filter(int msgtype, unsigned int listtype)
case AUDIT_SUBJ_SEN:
case AUDIT_SUBJ_CLR:
if (f->lsm_rule) {
- security_task_getsecid(current, &sid);
+ security_task_getsecid_subj(current,
+ &sid);
result = security_audit_rule_match(sid,
f->type, f->op, f->lsm_rule);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 47fb48f42c93..9973865cbf13 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -667,7 +667,7 @@ static int audit_filter_rules(struct task_struct *tsk,
logged upon error */
if (f->lsm_rule) {
if (need_sid) {
- security_task_getsecid(tsk, &sid);
+ security_task_getsecid_subj(tsk, &sid);
need_sid = 0;
}
result = security_audit_rule_match(sid, f->type,
@@ -2400,7 +2400,7 @@ void __audit_ptrace(struct task_struct *t)
context->target_auid = audit_get_loginuid(t);
context->target_uid = task_uid(t);
context->target_sessionid = audit_get_sessionid(t);
- security_task_getsecid(t, &context->target_sid);
+ security_task_getsecid_obj(t, &context->target_sid);
memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
}
@@ -2427,7 +2427,7 @@ int audit_signal_info_syscall(struct task_struct *t)
ctx->target_auid = audit_get_loginuid(t);
ctx->target_uid = t_uid;
ctx->target_sessionid = audit_get_sessionid(t);
- security_task_getsecid(t, &ctx->target_sid);
+ security_task_getsecid_obj(t, &ctx->target_sid);
memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
return 0;
}
@@ -2448,7 +2448,7 @@ int audit_signal_info_syscall(struct task_struct *t)
axp->target_auid[axp->pid_count] = audit_get_loginuid(t);
axp->target_uid[axp->pid_count] = t_uid;
axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
- security_task_getsecid(t, &axp->target_sid[axp->pid_count]);
+ security_task_getsecid_obj(t, &axp->target_sid[axp->pid_count]);
memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
axp->pid_count++;
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 1622a44d1617..0ff58259ccf8 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -209,7 +209,8 @@ BTF_ID(func, bpf_lsm_socket_socketpair)
BTF_ID(func, bpf_lsm_syslog)
BTF_ID(func, bpf_lsm_task_alloc)
-BTF_ID(func, bpf_lsm_task_getsecid)
+BTF_ID(func, bpf_lsm_task_getsecid_subj)
+BTF_ID(func, bpf_lsm_task_getsecid_obj)
BTF_ID(func, bpf_lsm_task_prctl)
BTF_ID(func, bpf_lsm_task_setscheduler)
BTF_ID(func, bpf_lsm_task_to_inode)
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index ccb491642811..3e6ac9b790b1 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1539,7 +1539,7 @@ int __init netlbl_unlabel_defconf(void)
/* Only the kernel is allowed to call this function and the only time
* it is called is at bootup before the audit subsystem is reporting
* messages so don't worry to much about these values. */
- security_task_getsecid(current, &audit_info.secid);
+ security_task_getsecid_subj(current, &audit_info.secid);
audit_info.loginuid = GLOBAL_ROOT_UID;
audit_info.sessionid = 0;
diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h
index 3c67afce64f1..b9ba8112b3c5 100644
--- a/net/netlabel/netlabel_user.h
+++ b/net/netlabel/netlabel_user.h
@@ -34,7 +34,7 @@
static inline void netlbl_netlink_auditinfo(struct sk_buff *skb,
struct netlbl_audit *audit_info)
{
- security_task_getsecid(current, &audit_info->secid);
+ security_task_getsecid_subj(current, &audit_info->secid);
audit_info->loginuid = audit_get_loginuid(current);
audit_info->sessionid = audit_get_sessionid(current);
}
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 240a53387e6b..f72406fe1bf2 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1252,7 +1252,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_free, apparmor_task_free),
LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
- LSM_HOOK_INIT(task_getsecid, apparmor_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_subj, apparmor_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid),
LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
LSM_HOOK_INIT(task_kill, apparmor_task_kill),
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 565e33ff19d0..4e5eb0236278 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -76,7 +76,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
if (!ima_appraise)
return 0;
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
return ima_match_policy(mnt_userns, inode, current_cred(), secid, func,
mask, IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL);
}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 9ef748ea829f..b85d9e429426 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -391,7 +391,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
u32 secid;
if (file && (prot & PROT_EXEC)) {
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
return process_measurement(file, current_cred(), secid, NULL,
0, MAY_EXEC, MMAP_CHECK);
}
@@ -429,7 +429,7 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
!(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC))
return 0;
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
inode = file_inode(vma->vm_file);
action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode,
current_cred(), secid, MAY_EXEC, MMAP_CHECK,
@@ -470,7 +470,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
int ret;
u32 secid;
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
MAY_EXEC, BPRM_CHECK);
if (ret)
@@ -495,7 +495,7 @@ int ima_file_check(struct file *file, int mask)
{
u32 secid;
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
return process_measurement(file, current_cred(), secid, NULL, 0,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
MAY_APPEND), FILE_CHECK);
@@ -686,7 +686,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
/* Read entire file for all partial reads. */
func = read_idmap[read_id] ?: FILE_CHECK;
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
return process_measurement(file, current_cred(), secid, NULL,
0, MAY_READ, func);
}
@@ -729,7 +729,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
}
func = read_idmap[read_id] ?: FILE_CHECK;
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
return process_measurement(file, current_cred(), secid, buf, size,
MAY_READ, func);
}
@@ -872,7 +872,7 @@ void process_buffer_measurement(struct user_namespace *mnt_userns,
* buffer measurements.
*/
if (func) {
- security_task_getsecid(current, &secid);
+ security_task_getsecid_subj(current, &secid);
action = ima_get_action(mnt_userns, inode, current_cred(),
secid, 0, func, &pcr, &template,
func_data);
diff --git a/security/security.c b/security/security.c
index 5ac96b16f8fa..ee76d6c2f852 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1762,12 +1762,19 @@ int security_task_getsid(struct task_struct *p)
return call_int_hook(task_getsid, 0, p);
}
-void security_task_getsecid(struct task_struct *p, u32 *secid)
+void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
{
*secid = 0;
- call_void_hook(task_getsecid, p, secid);
+ call_void_hook(task_getsecid_subj, p, secid);
}
-EXPORT_SYMBOL(security_task_getsecid);
+EXPORT_SYMBOL(security_task_getsecid_subj);
+
+void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
+{
+ *secid = 0;
+ call_void_hook(task_getsecid_obj, p, secid);
+}
+EXPORT_SYMBOL(security_task_getsecid_obj);
int security_task_setnice(struct task_struct *p, int nice)
{
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index eca9fc0ba764..24ca545c6e1b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7149,7 +7149,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
LSM_HOOK_INIT(task_getsid, selinux_task_getsid),
- LSM_HOOK_INIT(task_getsecid, selinux_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid),
LSM_HOOK_INIT(task_setnice, selinux_task_setnice),
LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio),
LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio),
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 12a45e61c1a5..f546fb832f30 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4759,7 +4759,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
LSM_HOOK_INIT(task_getpgid, smack_task_getpgid),
LSM_HOOK_INIT(task_getsid, smack_task_getsid),
- LSM_HOOK_INIT(task_getsecid, smack_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid),
LSM_HOOK_INIT(task_setnice, smack_task_setnice),
LSM_HOOK_INIT(task_setioprio, smack_task_setioprio),
LSM_HOOK_INIT(task_getioprio, smack_task_getioprio),
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] selinux: clarify task subjective and objective credentials
2021-03-18 20:42 [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
2021-03-18 20:42 ` [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants Paul Moore
@ 2021-03-18 20:42 ` Paul Moore
2021-03-19 13:36 ` Richard Guy Briggs
2021-03-18 20:42 ` [PATCH v2 3/3] smack: differentiate between subjective and objective task credentials Paul Moore
2021-03-22 19:29 ` [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
3 siblings, 1 reply; 9+ messages in thread
From: Paul Moore @ 2021-03-18 20:42 UTC (permalink / raw)
To: linux-security-module, linux-audit, selinux
Cc: Richard Guy Briggs, John Johansen
SELinux has a function, task_sid(), which returns the task's
objective credentials, but unfortunately is used in a few places
where the subjective task credentials should be used. Most notably
in the new security_task_getsecid_subj() LSM hook.
This patch fixes this and attempts to make things more obvious by
introducing a new function, task_sid_subj(), and renaming the
existing task_sid() function to task_sid_obj().
This patch also adds an interesting function in task_sid_binder().
The task_sid_binder() function has a comment which hopefully
describes it's reason for being, but it basically boils down to the
simple fact that we can't safely access another task's subjective
credentials so in the case of binder we need to stick with the
objective credentials regardless.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/selinux/hooks.c | 112 ++++++++++++++++++++++++++++++----------------
1 file changed, 73 insertions(+), 39 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 24ca545c6e1b..901a3ce453f1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -229,10 +229,23 @@ static inline u32 cred_sid(const struct cred *cred)
return tsec->sid;
}
+/*
+ * get the subjective security ID of a task
+ */
+static inline u32 task_sid_subj(const struct task_struct *task)
+{
+ u32 sid;
+
+ rcu_read_lock();
+ sid = cred_sid(rcu_dereference(task->cred));
+ rcu_read_unlock();
+ return sid;
+}
+
/*
* get the objective security ID of a task
*/
-static inline u32 task_sid(const struct task_struct *task)
+static inline u32 task_sid_obj(const struct task_struct *task)
{
u32 sid;
@@ -242,6 +255,29 @@ static inline u32 task_sid(const struct task_struct *task)
return sid;
}
+/*
+ * get the security ID of a task for use with binder
+ */
+static inline u32 task_sid_binder(const struct task_struct *task)
+{
+ /*
+ * In many case where this function is used we should be using the
+ * task's subjective SID, but we can't reliably access the subjective
+ * creds of a task other than our own so we must use the objective
+ * creds/SID, which are safe to access. The downside is that if a task
+ * is temporarily overriding it's creds it will not be reflected here;
+ * however, it isn't clear that binder would handle that case well
+ * anyway.
+ *
+ * If this ever changes and we can safely reference the subjective
+ * creds/SID of another task, this function will make it easier to
+ * identify the various places where we make use of the task SIDs in
+ * the binder code. It is also likely that we will need to adjust
+ * the main drivers/android binder code as well.
+ */
+ return task_sid_obj(task);
+}
+
static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry);
/*
@@ -2035,11 +2071,8 @@ static inline u32 open_file_to_av(struct file *file)
static int selinux_binder_set_context_mgr(struct task_struct *mgr)
{
- u32 mysid = current_sid();
- u32 mgrsid = task_sid(mgr);
-
return avc_has_perm(&selinux_state,
- mysid, mgrsid, SECCLASS_BINDER,
+ current_sid(), task_sid_binder(mgr), SECCLASS_BINDER,
BINDER__SET_CONTEXT_MGR, NULL);
}
@@ -2047,8 +2080,7 @@ static int selinux_binder_transaction(struct task_struct *from,
struct task_struct *to)
{
u32 mysid = current_sid();
- u32 fromsid = task_sid(from);
- u32 tosid = task_sid(to);
+ u32 fromsid = task_sid_binder(from);
int rc;
if (mysid != fromsid) {
@@ -2059,19 +2091,16 @@ static int selinux_binder_transaction(struct task_struct *from,
return rc;
}
- return avc_has_perm(&selinux_state,
- fromsid, tosid, SECCLASS_BINDER, BINDER__CALL,
- NULL);
+ return avc_has_perm(&selinux_state, fromsid, task_sid_binder(to),
+ SECCLASS_BINDER, BINDER__CALL, NULL);
}
static int selinux_binder_transfer_binder(struct task_struct *from,
struct task_struct *to)
{
- u32 fromsid = task_sid(from);
- u32 tosid = task_sid(to);
-
return avc_has_perm(&selinux_state,
- fromsid, tosid, SECCLASS_BINDER, BINDER__TRANSFER,
+ task_sid_binder(from), task_sid_binder(to),
+ SECCLASS_BINDER, BINDER__TRANSFER,
NULL);
}
@@ -2079,7 +2108,7 @@ static int selinux_binder_transfer_file(struct task_struct *from,
struct task_struct *to,
struct file *file)
{
- u32 sid = task_sid(to);
+ u32 sid = task_sid_binder(to);
struct file_security_struct *fsec = selinux_file(file);
struct dentry *dentry = file->f_path.dentry;
struct inode_security_struct *isec;
@@ -2115,10 +2144,10 @@ static int selinux_binder_transfer_file(struct task_struct *from,
}
static int selinux_ptrace_access_check(struct task_struct *child,
- unsigned int mode)
+ unsigned int mode)
{
u32 sid = current_sid();
- u32 csid = task_sid(child);
+ u32 csid = task_sid_obj(child);
if (mode & PTRACE_MODE_READ)
return avc_has_perm(&selinux_state,
@@ -2131,15 +2160,15 @@ static int selinux_ptrace_access_check(struct task_struct *child,
static int selinux_ptrace_traceme(struct task_struct *parent)
{
return avc_has_perm(&selinux_state,
- task_sid(parent), current_sid(), SECCLASS_PROCESS,
- PROCESS__PTRACE, NULL);
+ task_sid_subj(parent), task_sid_obj(current),
+ SECCLASS_PROCESS, PROCESS__PTRACE, NULL);
}
static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(target), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(target), SECCLASS_PROCESS,
PROCESS__GETCAP, NULL);
}
@@ -2264,7 +2293,7 @@ static u32 ptrace_parent_sid(void)
rcu_read_lock();
tracer = ptrace_parent(current);
if (tracer)
- sid = task_sid(tracer);
+ sid = task_sid_obj(tracer);
rcu_read_unlock();
return sid;
@@ -3921,7 +3950,7 @@ static int selinux_file_send_sigiotask(struct task_struct *tsk,
struct fown_struct *fown, int signum)
{
struct file *file;
- u32 sid = task_sid(tsk);
+ u32 sid = task_sid_obj(tsk);
u32 perm;
struct file_security_struct *fsec;
@@ -4140,47 +4169,52 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
static int selinux_task_setpgid(struct task_struct *p, pid_t pgid)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__SETPGID, NULL);
}
static int selinux_task_getpgid(struct task_struct *p)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__GETPGID, NULL);
}
static int selinux_task_getsid(struct task_struct *p)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__GETSESSION, NULL);
}
-static void selinux_task_getsecid(struct task_struct *p, u32 *secid)
+static void selinux_task_getsecid_subj(struct task_struct *p, u32 *secid)
+{
+ *secid = task_sid_subj(p);
+}
+
+static void selinux_task_getsecid_obj(struct task_struct *p, u32 *secid)
{
- *secid = task_sid(p);
+ *secid = task_sid_obj(p);
}
static int selinux_task_setnice(struct task_struct *p, int nice)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__SETSCHED, NULL);
}
static int selinux_task_setioprio(struct task_struct *p, int ioprio)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__SETSCHED, NULL);
}
static int selinux_task_getioprio(struct task_struct *p)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__GETSCHED, NULL);
}
@@ -4211,7 +4245,7 @@ static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource,
upon context transitions. See selinux_bprm_committing_creds. */
if (old_rlim->rlim_max != new_rlim->rlim_max)
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p),
+ current_sid(), task_sid_obj(p),
SECCLASS_PROCESS, PROCESS__SETRLIMIT, NULL);
return 0;
@@ -4220,21 +4254,21 @@ static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource,
static int selinux_task_setscheduler(struct task_struct *p)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__SETSCHED, NULL);
}
static int selinux_task_getscheduler(struct task_struct *p)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__GETSCHED, NULL);
}
static int selinux_task_movememory(struct task_struct *p)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid(p), SECCLASS_PROCESS,
+ current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
PROCESS__SETSCHED, NULL);
}
@@ -4253,14 +4287,14 @@ static int selinux_task_kill(struct task_struct *p, struct kernel_siginfo *info,
else
secid = cred_sid(cred);
return avc_has_perm(&selinux_state,
- secid, task_sid(p), SECCLASS_PROCESS, perm, NULL);
+ secid, task_sid_obj(p), SECCLASS_PROCESS, perm, NULL);
}
static void selinux_task_to_inode(struct task_struct *p,
struct inode *inode)
{
struct inode_security_struct *isec = selinux_inode(inode);
- u32 sid = task_sid(p);
+ u32 sid = task_sid_obj(p);
spin_lock(&isec->lock);
isec->sclass = inode_mode_to_security_class(inode->i_mode);
@@ -6153,7 +6187,7 @@ static int selinux_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *m
struct ipc_security_struct *isec;
struct msg_security_struct *msec;
struct common_audit_data ad;
- u32 sid = task_sid(target);
+ u32 sid = task_sid_subj(target);
int rc;
isec = selinux_ipc(msq);
@@ -7149,8 +7183,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
LSM_HOOK_INIT(task_getsid, selinux_task_getsid),
- LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid),
- LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid_subj),
+ LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid_obj),
LSM_HOOK_INIT(task_setnice, selinux_task_setnice),
LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio),
LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio),
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] smack: differentiate between subjective and objective task credentials
2021-03-18 20:42 [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
2021-03-18 20:42 ` [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants Paul Moore
2021-03-18 20:42 ` [PATCH v2 2/3] selinux: clarify task subjective and objective credentials Paul Moore
@ 2021-03-18 20:42 ` Paul Moore
2021-03-22 19:29 ` [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
3 siblings, 0 replies; 9+ messages in thread
From: Paul Moore @ 2021-03-18 20:42 UTC (permalink / raw)
To: linux-security-module, linux-audit, selinux
Cc: Richard Guy Briggs, John Johansen
With the split of the security_task_getsecid() into subjective and
objective variants it's time to update Smack to ensure it is using
the correct task creds.
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/smack/smack.h | 18 +++++++++++++++++-
security/smack/smack_lsm.c | 40 +++++++++++++++++++++++++++-------------
2 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/security/smack/smack.h b/security/smack/smack.h
index a9768b12716b..08f9cb80655c 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -383,7 +383,23 @@ static inline struct smack_known *smk_of_task(const struct task_smack *tsp)
return tsp->smk_task;
}
-static inline struct smack_known *smk_of_task_struct(
+static inline struct smack_known *smk_of_task_struct_subj(
+ const struct task_struct *t)
+{
+ struct smack_known *skp;
+ const struct cred *cred;
+
+ rcu_read_lock();
+
+ cred = rcu_dereference(t->cred);
+ skp = smk_of_task(smack_cred(cred));
+
+ rcu_read_unlock();
+
+ return skp;
+}
+
+static inline struct smack_known *smk_of_task_struct_obj(
const struct task_struct *t)
{
struct smack_known *skp;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index f546fb832f30..cd14bec4ad80 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -159,7 +159,7 @@ static int smk_bu_current(char *note, struct smack_known *oskp,
static int smk_bu_task(struct task_struct *otp, int mode, int rc)
{
struct task_smack *tsp = smack_cred(current_cred());
- struct smack_known *smk_task = smk_of_task_struct(otp);
+ struct smack_known *smk_task = smk_of_task_struct_obj(otp);
char acc[SMK_NUM_ACCESS_TYPE + 1];
if (rc <= 0)
@@ -479,7 +479,7 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
{
struct smack_known *skp;
- skp = smk_of_task_struct(ctp);
+ skp = smk_of_task_struct_obj(ctp);
return smk_ptrace_rule_check(current, skp, mode, __func__);
}
@@ -2033,7 +2033,7 @@ static int smk_curacc_on_task(struct task_struct *p, int access,
const char *caller)
{
struct smk_audit_info ad;
- struct smack_known *skp = smk_of_task_struct(p);
+ struct smack_known *skp = smk_of_task_struct_subj(p);
int rc;
smk_ad_init(&ad, caller, LSM_AUDIT_DATA_TASK);
@@ -2078,15 +2078,29 @@ static int smack_task_getsid(struct task_struct *p)
}
/**
- * smack_task_getsecid - get the secid of the task
- * @p: the object task
+ * smack_task_getsecid_subj - get the subjective secid of the task
+ * @p: the task
* @secid: where to put the result
*
- * Sets the secid to contain a u32 version of the smack label.
+ * Sets the secid to contain a u32 version of the task's subjective smack label.
+ */
+static void smack_task_getsecid_subj(struct task_struct *p, u32 *secid)
+{
+ struct smack_known *skp = smk_of_task_struct_subj(p);
+
+ *secid = skp->smk_secid;
+}
+
+/**
+ * smack_task_getsecid_obj - get the objective secid of the task
+ * @p: the task
+ * @secid: where to put the result
+ *
+ * Sets the secid to contain a u32 version of the task's objective smack label.
*/
-static void smack_task_getsecid(struct task_struct *p, u32 *secid)
+static void smack_task_getsecid_obj(struct task_struct *p, u32 *secid)
{
- struct smack_known *skp = smk_of_task_struct(p);
+ struct smack_known *skp = smk_of_task_struct_obj(p);
*secid = skp->smk_secid;
}
@@ -2174,7 +2188,7 @@ static int smack_task_kill(struct task_struct *p, struct kernel_siginfo *info,
{
struct smk_audit_info ad;
struct smack_known *skp;
- struct smack_known *tkp = smk_of_task_struct(p);
+ struct smack_known *tkp = smk_of_task_struct_obj(p);
int rc;
if (!sig)
@@ -2212,7 +2226,7 @@ static int smack_task_kill(struct task_struct *p, struct kernel_siginfo *info,
static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
{
struct inode_smack *isp = smack_inode(inode);
- struct smack_known *skp = smk_of_task_struct(p);
+ struct smack_known *skp = smk_of_task_struct_obj(p);
isp->smk_inode = skp;
isp->smk_flags |= SMK_INODE_INSTANT;
@@ -3483,7 +3497,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
*/
static int smack_getprocattr(struct task_struct *p, char *name, char **value)
{
- struct smack_known *skp = smk_of_task_struct(p);
+ struct smack_known *skp = smk_of_task_struct_subj(p);
char *cp;
int slen;
@@ -4759,8 +4773,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
LSM_HOOK_INIT(task_getpgid, smack_task_getpgid),
LSM_HOOK_INIT(task_getsid, smack_task_getsid),
- LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid),
- LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid),
+ LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid_subj),
+ LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid_obj),
LSM_HOOK_INIT(task_setnice, smack_task_setnice),
LSM_HOOK_INIT(task_setioprio, smack_task_setioprio),
LSM_HOOK_INIT(task_getioprio, smack_task_getioprio),
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants
2021-03-18 20:42 ` [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants Paul Moore
@ 2021-03-18 20:57 ` Casey Schaufler
2021-03-19 3:44 ` Paul Moore
2021-03-19 13:35 ` Richard Guy Briggs
1 sibling, 1 reply; 9+ messages in thread
From: Casey Schaufler @ 2021-03-18 20:57 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-audit, selinux
Cc: Richard Guy Briggs, John Johansen
On 3/18/2021 1:42 PM, Paul Moore wrote:
> Of the three LSMs that implement the security_task_getsecid() LSM
> hook, all three LSMs provide the task's objective security
> credentials. This turns out to be unfortunate as most of the hook's
> callers seem to expect the task's subjective credentials, although
> a small handful of callers do correctly expect the objective
> credentials.
>
> This patch is the first step towards fixing the problem: it splits
> the existing security_task_getsecid() hook into two variants, one
> for the subjective creds, one for the objective creds.
>
> void security_task_getsecid_subj(struct task_struct *p,
> u32 *secid);
> void security_task_getsecid_obj(struct task_struct *p,
> u32 *secid);
>
> While this patch does fix all of the callers to use the correct
> variant, in order to keep this patch focused on the callers and to
> ease review, the LSMs continue to use the same implementation for
> both hooks. The net effect is that this patch should not change
> the behavior of the kernel in any way, it will be up to the latter
> LSM specific patches in this series to change the hook
> implementations and return the correct credentials.
>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com> (IMA)
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> drivers/android/binder.c | 11 ++++++++++-
> include/linux/cred.h | 2 +-
> include/linux/lsm_hook_defs.h | 5 ++++-
> include/linux/lsm_hooks.h | 12 +++++++++---
> include/linux/security.h | 10 ++++++++--
> kernel/audit.c | 4 ++--
> kernel/auditfilter.c | 3 ++-
> kernel/auditsc.c | 8 ++++----
> kernel/bpf/bpf_lsm.c | 3 ++-
> net/netlabel/netlabel_unlabeled.c | 2 +-
> net/netlabel/netlabel_user.h | 2 +-
> security/apparmor/lsm.c | 3 ++-
> security/integrity/ima/ima_appraise.c | 2 +-
> security/integrity/ima/ima_main.c | 14 +++++++-------
> security/security.c | 13 ++++++++++---
> security/selinux/hooks.c | 3 ++-
> security/smack/smack_lsm.c | 3 ++-
> 17 files changed, 68 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56a..61d235b6ccd8 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2700,7 +2700,16 @@ static void binder_transaction(struct binder_proc *proc,
> u32 secid;
> size_t added_size;
>
> - security_task_getsecid(proc->tsk, &secid);
> + /*
> + * Arguably this should be the task's subjective LSM secid but
> + * we can't reliably access the subjective creds of a task
> + * other than our own so we must use the objective creds, which
> + * are safe to access. The downside is that if a task is
> + * temporarily overriding it's creds it will not be reflected
> + * here; however, it isn't clear that binder would handle that
> + * case well anyway.
> + */
> + security_task_getsecid_obj(proc->tsk, &secid);
> ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 4c6350503697..ac0e5f97d7d8 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -140,7 +140,7 @@ struct cred {
> struct key *request_key_auth; /* assumed request_key authority */
> #endif
> #ifdef CONFIG_SECURITY
> - void *security; /* subjective LSM security */
> + void *security; /* LSM security */
> #endif
> struct user_struct *user; /* real user ID subscription */
> struct user_namespace *user_ns; /* user_ns the caps and keyrings are relative to. */
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 477a597db013..3ad8085e85e1 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -203,7 +203,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new, const struct cred * old,
> LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
> LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
> LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
> -LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
> + struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
> + struct task_struct *p, u32 *secid)
> LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)
> LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio)
> LSM_HOOK(int, 0, task_getioprio, struct task_struct *p)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index fb7f3193753d..e25d31fe787e 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -707,9 +707,15 @@
> * @p.
> * @p contains the task_struct for the process.
> * Return 0 if permission is granted.
> - * @task_getsecid:
> - * Retrieve the security identifier of the process @p.
> - * @p contains the task_struct for the process and place is into @secid.
> + * @task_getsecid_subj:
> + * Retrieve the subjective security identifier of the task_struct in @p
> + * and return it in @secid. Special care must be taken to ensure that @p
> + * is the either the "current" task, or the caller has exclusive access
> + * to @p.
> + * In case of failure, @secid will be set to zero.
> + * @task_getsecid_obj:
> + * Retrieve the objective security identifier of the task_struct in @p
> + * and return it in @secid.
> * In case of failure, @secid will be set to zero.
> *
> * @task_setnice:
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 8aeebd6646dc..9c490bc437f5 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -414,7 +414,8 @@ int security_task_fix_setgid(struct cred *new, const struct cred *old,
> int security_task_setpgid(struct task_struct *p, pid_t pgid);
> int security_task_getpgid(struct task_struct *p);
> int security_task_getsid(struct task_struct *p);
> -void security_task_getsecid(struct task_struct *p, u32 *secid);
> +void security_task_getsecid_subj(struct task_struct *p, u32 *secid);
> +void security_task_getsecid_obj(struct task_struct *p, u32 *secid);
> int security_task_setnice(struct task_struct *p, int nice);
> int security_task_setioprio(struct task_struct *p, int ioprio);
> int security_task_getioprio(struct task_struct *p);
> @@ -1098,7 +1099,12 @@ static inline int security_task_getsid(struct task_struct *p)
> return 0;
> }
>
> -static inline void security_task_getsecid(struct task_struct *p, u32 *secid)
> +static inline void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
> +{
> + *secid = 0;
> +}
> +
> +static inline void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
> {
> *secid = 0;
> }
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 551a394bc8f4..121d37e700a6 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2132,7 +2132,7 @@ int audit_log_task_context(struct audit_buffer *ab)
> int error;
> u32 sid;
>
> - security_task_getsecid(current, &sid);
> + security_task_getsecid_subj(current, &sid);
> if (!sid)
> return 0;
>
> @@ -2353,7 +2353,7 @@ int audit_signal_info(int sig, struct task_struct *t)
> audit_sig_uid = auid;
> else
> audit_sig_uid = uid;
> - security_task_getsecid(current, &audit_sig_sid);
> + security_task_getsecid_subj(current, &audit_sig_sid);
> }
>
> return audit_signal_info_syscall(t);
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 333b3bcfc545..db2c6b59dfc3 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1359,7 +1359,8 @@ int audit_filter(int msgtype, unsigned int listtype)
> case AUDIT_SUBJ_SEN:
> case AUDIT_SUBJ_CLR:
> if (f->lsm_rule) {
> - security_task_getsecid(current, &sid);
> + security_task_getsecid_subj(current,
> + &sid);
> result = security_audit_rule_match(sid,
> f->type, f->op, f->lsm_rule);
> }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 47fb48f42c93..9973865cbf13 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -667,7 +667,7 @@ static int audit_filter_rules(struct task_struct *tsk,
> logged upon error */
> if (f->lsm_rule) {
> if (need_sid) {
> - security_task_getsecid(tsk, &sid);
> + security_task_getsecid_subj(tsk, &sid);
> need_sid = 0;
> }
> result = security_audit_rule_match(sid, f->type,
> @@ -2400,7 +2400,7 @@ void __audit_ptrace(struct task_struct *t)
> context->target_auid = audit_get_loginuid(t);
> context->target_uid = task_uid(t);
> context->target_sessionid = audit_get_sessionid(t);
> - security_task_getsecid(t, &context->target_sid);
> + security_task_getsecid_obj(t, &context->target_sid);
> memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
> }
>
> @@ -2427,7 +2427,7 @@ int audit_signal_info_syscall(struct task_struct *t)
> ctx->target_auid = audit_get_loginuid(t);
> ctx->target_uid = t_uid;
> ctx->target_sessionid = audit_get_sessionid(t);
> - security_task_getsecid(t, &ctx->target_sid);
> + security_task_getsecid_obj(t, &ctx->target_sid);
> memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
> return 0;
> }
> @@ -2448,7 +2448,7 @@ int audit_signal_info_syscall(struct task_struct *t)
> axp->target_auid[axp->pid_count] = audit_get_loginuid(t);
> axp->target_uid[axp->pid_count] = t_uid;
> axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
> - security_task_getsecid(t, &axp->target_sid[axp->pid_count]);
> + security_task_getsecid_obj(t, &axp->target_sid[axp->pid_count]);
> memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
> axp->pid_count++;
>
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 1622a44d1617..0ff58259ccf8 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -209,7 +209,8 @@ BTF_ID(func, bpf_lsm_socket_socketpair)
>
> BTF_ID(func, bpf_lsm_syslog)
> BTF_ID(func, bpf_lsm_task_alloc)
> -BTF_ID(func, bpf_lsm_task_getsecid)
> +BTF_ID(func, bpf_lsm_task_getsecid_subj)
> +BTF_ID(func, bpf_lsm_task_getsecid_obj)
> BTF_ID(func, bpf_lsm_task_prctl)
> BTF_ID(func, bpf_lsm_task_setscheduler)
> BTF_ID(func, bpf_lsm_task_to_inode)
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index ccb491642811..3e6ac9b790b1 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1539,7 +1539,7 @@ int __init netlbl_unlabel_defconf(void)
> /* Only the kernel is allowed to call this function and the only time
> * it is called is at bootup before the audit subsystem is reporting
> * messages so don't worry to much about these values. */
> - security_task_getsecid(current, &audit_info.secid);
> + security_task_getsecid_subj(current, &audit_info.secid);
> audit_info.loginuid = GLOBAL_ROOT_UID;
> audit_info.sessionid = 0;
>
> diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h
> index 3c67afce64f1..b9ba8112b3c5 100644
> --- a/net/netlabel/netlabel_user.h
> +++ b/net/netlabel/netlabel_user.h
> @@ -34,7 +34,7 @@
> static inline void netlbl_netlink_auditinfo(struct sk_buff *skb,
> struct netlbl_audit *audit_info)
> {
> - security_task_getsecid(current, &audit_info->secid);
> + security_task_getsecid_subj(current, &audit_info->secid);
> audit_info->loginuid = audit_get_loginuid(current);
> audit_info->sessionid = audit_get_sessionid(current);
> }
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 240a53387e6b..f72406fe1bf2 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1252,7 +1252,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>
> LSM_HOOK_INIT(task_free, apparmor_task_free),
> LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
> - LSM_HOOK_INIT(task_getsecid, apparmor_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, apparmor_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid),
> LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
> LSM_HOOK_INIT(task_kill, apparmor_task_kill),
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 565e33ff19d0..4e5eb0236278 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -76,7 +76,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
> if (!ima_appraise)
> return 0;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return ima_match_policy(mnt_userns, inode, current_cred(), secid, func,
> mask, IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL);
> }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 9ef748ea829f..b85d9e429426 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -391,7 +391,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
> u32 secid;
>
> if (file && (prot & PROT_EXEC)) {
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, NULL,
> 0, MAY_EXEC, MMAP_CHECK);
> }
> @@ -429,7 +429,7 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
> !(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC))
> return 0;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> inode = file_inode(vma->vm_file);
> action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode,
> current_cred(), secid, MAY_EXEC, MMAP_CHECK,
> @@ -470,7 +470,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
> int ret;
> u32 secid;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
> MAY_EXEC, BPRM_CHECK);
> if (ret)
> @@ -495,7 +495,7 @@ int ima_file_check(struct file *file, int mask)
> {
> u32 secid;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, NULL, 0,
> mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
> MAY_APPEND), FILE_CHECK);
> @@ -686,7 +686,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
>
> /* Read entire file for all partial reads. */
> func = read_idmap[read_id] ?: FILE_CHECK;
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, NULL,
> 0, MAY_READ, func);
> }
> @@ -729,7 +729,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
> }
>
> func = read_idmap[read_id] ?: FILE_CHECK;
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, buf, size,
> MAY_READ, func);
> }
> @@ -872,7 +872,7 @@ void process_buffer_measurement(struct user_namespace *mnt_userns,
> * buffer measurements.
> */
> if (func) {
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> action = ima_get_action(mnt_userns, inode, current_cred(),
> secid, 0, func, &pcr, &template,
> func_data);
> diff --git a/security/security.c b/security/security.c
> index 5ac96b16f8fa..ee76d6c2f852 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1762,12 +1762,19 @@ int security_task_getsid(struct task_struct *p)
> return call_int_hook(task_getsid, 0, p);
> }
>
> -void security_task_getsecid(struct task_struct *p, u32 *secid)
> +void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
> {
> *secid = 0;
> - call_void_hook(task_getsecid, p, secid);
> + call_void_hook(task_getsecid_subj, p, secid);
> }
> -EXPORT_SYMBOL(security_task_getsecid);
> +EXPORT_SYMBOL(security_task_getsecid_subj);
> +
> +void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
> +{
> + *secid = 0;
> + call_void_hook(task_getsecid_obj, p, secid);
> +}
> +EXPORT_SYMBOL(security_task_getsecid_obj);
>
> int security_task_setnice(struct task_struct *p, int nice)
> {
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index eca9fc0ba764..24ca545c6e1b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7149,7 +7149,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
> LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> LSM_HOOK_INIT(task_getsid, selinux_task_getsid),
> - LSM_HOOK_INIT(task_getsecid, selinux_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid),
> LSM_HOOK_INIT(task_setnice, selinux_task_setnice),
> LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio),
> LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio),
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 12a45e61c1a5..f546fb832f30 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4759,7 +4759,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
> LSM_HOOK_INIT(task_getpgid, smack_task_getpgid),
> LSM_HOOK_INIT(task_getsid, smack_task_getsid),
> - LSM_HOOK_INIT(task_getsecid, smack_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid),
> LSM_HOOK_INIT(task_setnice, smack_task_setnice),
> LSM_HOOK_INIT(task_setioprio, smack_task_setioprio),
> LSM_HOOK_INIT(task_getioprio, smack_task_getioprio),
>
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants
2021-03-18 20:57 ` Casey Schaufler
@ 2021-03-19 3:44 ` Paul Moore
0 siblings, 0 replies; 9+ messages in thread
From: Paul Moore @ 2021-03-19 3:44 UTC (permalink / raw)
To: Casey Schaufler
Cc: selinux, Richard Guy Briggs, linux-security-module, linux-audit,
John Johansen
On Thu, Mar 18, 2021 at 4:57 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 3/18/2021 1:42 PM, Paul Moore wrote:
> > Of the three LSMs that implement the security_task_getsecid() LSM
> > hook, all three LSMs provide the task's objective security
> > credentials. This turns out to be unfortunate as most of the hook's
> > callers seem to expect the task's subjective credentials, although
> > a small handful of callers do correctly expect the objective
> > credentials.
> >
> > This patch is the first step towards fixing the problem: it splits
> > the existing security_task_getsecid() hook into two variants, one
> > for the subjective creds, one for the objective creds.
> >
> > void security_task_getsecid_subj(struct task_struct *p,
> > u32 *secid);
> > void security_task_getsecid_obj(struct task_struct *p,
> > u32 *secid);
> >
> > While this patch does fix all of the callers to use the correct
> > variant, in order to keep this patch focused on the callers and to
> > ease review, the LSMs continue to use the same implementation for
> > both hooks. The net effect is that this patch should not change
> > the behavior of the kernel in any way, it will be up to the latter
> > LSM specific patches in this series to change the hook
> > implementations and return the correct credentials.
> >
> > Acked-by: Mimi Zohar <zohar@linux.ibm.com> (IMA)
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Thanks Casey.
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants
2021-03-18 20:42 ` [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants Paul Moore
2021-03-18 20:57 ` Casey Schaufler
@ 2021-03-19 13:35 ` Richard Guy Briggs
1 sibling, 0 replies; 9+ messages in thread
From: Richard Guy Briggs @ 2021-03-19 13:35 UTC (permalink / raw)
To: Paul Moore; +Cc: selinux, linux-security-module, linux-audit, John Johansen
On 2021-03-18 16:42, Paul Moore wrote:
> Of the three LSMs that implement the security_task_getsecid() LSM
> hook, all three LSMs provide the task's objective security
> credentials. This turns out to be unfortunate as most of the hook's
> callers seem to expect the task's subjective credentials, although
> a small handful of callers do correctly expect the objective
> credentials.
>
> This patch is the first step towards fixing the problem: it splits
> the existing security_task_getsecid() hook into two variants, one
> for the subjective creds, one for the objective creds.
>
> void security_task_getsecid_subj(struct task_struct *p,
> u32 *secid);
> void security_task_getsecid_obj(struct task_struct *p,
> u32 *secid);
>
> While this patch does fix all of the callers to use the correct
> variant, in order to keep this patch focused on the callers and to
> ease review, the LSMs continue to use the same implementation for
> both hooks. The net effect is that this patch should not change
> the behavior of the kernel in any way, it will be up to the latter
> LSM specific patches in this series to change the hook
> implementations and return the correct credentials.
>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com> (IMA)
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Audit: Acked-by: Richard Guy Briggs <rgb@redhat.com>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> drivers/android/binder.c | 11 ++++++++++-
> include/linux/cred.h | 2 +-
> include/linux/lsm_hook_defs.h | 5 ++++-
> include/linux/lsm_hooks.h | 12 +++++++++---
> include/linux/security.h | 10 ++++++++--
> kernel/audit.c | 4 ++--
> kernel/auditfilter.c | 3 ++-
> kernel/auditsc.c | 8 ++++----
> kernel/bpf/bpf_lsm.c | 3 ++-
> net/netlabel/netlabel_unlabeled.c | 2 +-
> net/netlabel/netlabel_user.h | 2 +-
> security/apparmor/lsm.c | 3 ++-
> security/integrity/ima/ima_appraise.c | 2 +-
> security/integrity/ima/ima_main.c | 14 +++++++-------
> security/security.c | 13 ++++++++++---
> security/selinux/hooks.c | 3 ++-
> security/smack/smack_lsm.c | 3 ++-
> 17 files changed, 68 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56a..61d235b6ccd8 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2700,7 +2700,16 @@ static void binder_transaction(struct binder_proc *proc,
> u32 secid;
> size_t added_size;
>
> - security_task_getsecid(proc->tsk, &secid);
> + /*
> + * Arguably this should be the task's subjective LSM secid but
> + * we can't reliably access the subjective creds of a task
> + * other than our own so we must use the objective creds, which
> + * are safe to access. The downside is that if a task is
> + * temporarily overriding it's creds it will not be reflected
> + * here; however, it isn't clear that binder would handle that
> + * case well anyway.
> + */
> + security_task_getsecid_obj(proc->tsk, &secid);
> ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 4c6350503697..ac0e5f97d7d8 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -140,7 +140,7 @@ struct cred {
> struct key *request_key_auth; /* assumed request_key authority */
> #endif
> #ifdef CONFIG_SECURITY
> - void *security; /* subjective LSM security */
> + void *security; /* LSM security */
> #endif
> struct user_struct *user; /* real user ID subscription */
> struct user_namespace *user_ns; /* user_ns the caps and keyrings are relative to. */
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 477a597db013..3ad8085e85e1 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -203,7 +203,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new, const struct cred * old,
> LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
> LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
> LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
> -LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
> + struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
> + struct task_struct *p, u32 *secid)
> LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)
> LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio)
> LSM_HOOK(int, 0, task_getioprio, struct task_struct *p)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index fb7f3193753d..e25d31fe787e 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -707,9 +707,15 @@
> * @p.
> * @p contains the task_struct for the process.
> * Return 0 if permission is granted.
> - * @task_getsecid:
> - * Retrieve the security identifier of the process @p.
> - * @p contains the task_struct for the process and place is into @secid.
> + * @task_getsecid_subj:
> + * Retrieve the subjective security identifier of the task_struct in @p
> + * and return it in @secid. Special care must be taken to ensure that @p
> + * is the either the "current" task, or the caller has exclusive access
> + * to @p.
> + * In case of failure, @secid will be set to zero.
> + * @task_getsecid_obj:
> + * Retrieve the objective security identifier of the task_struct in @p
> + * and return it in @secid.
> * In case of failure, @secid will be set to zero.
> *
> * @task_setnice:
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 8aeebd6646dc..9c490bc437f5 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -414,7 +414,8 @@ int security_task_fix_setgid(struct cred *new, const struct cred *old,
> int security_task_setpgid(struct task_struct *p, pid_t pgid);
> int security_task_getpgid(struct task_struct *p);
> int security_task_getsid(struct task_struct *p);
> -void security_task_getsecid(struct task_struct *p, u32 *secid);
> +void security_task_getsecid_subj(struct task_struct *p, u32 *secid);
> +void security_task_getsecid_obj(struct task_struct *p, u32 *secid);
> int security_task_setnice(struct task_struct *p, int nice);
> int security_task_setioprio(struct task_struct *p, int ioprio);
> int security_task_getioprio(struct task_struct *p);
> @@ -1098,7 +1099,12 @@ static inline int security_task_getsid(struct task_struct *p)
> return 0;
> }
>
> -static inline void security_task_getsecid(struct task_struct *p, u32 *secid)
> +static inline void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
> +{
> + *secid = 0;
> +}
> +
> +static inline void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
> {
> *secid = 0;
> }
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 551a394bc8f4..121d37e700a6 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2132,7 +2132,7 @@ int audit_log_task_context(struct audit_buffer *ab)
> int error;
> u32 sid;
>
> - security_task_getsecid(current, &sid);
> + security_task_getsecid_subj(current, &sid);
> if (!sid)
> return 0;
>
> @@ -2353,7 +2353,7 @@ int audit_signal_info(int sig, struct task_struct *t)
> audit_sig_uid = auid;
> else
> audit_sig_uid = uid;
> - security_task_getsecid(current, &audit_sig_sid);
> + security_task_getsecid_subj(current, &audit_sig_sid);
> }
>
> return audit_signal_info_syscall(t);
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 333b3bcfc545..db2c6b59dfc3 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1359,7 +1359,8 @@ int audit_filter(int msgtype, unsigned int listtype)
> case AUDIT_SUBJ_SEN:
> case AUDIT_SUBJ_CLR:
> if (f->lsm_rule) {
> - security_task_getsecid(current, &sid);
> + security_task_getsecid_subj(current,
> + &sid);
> result = security_audit_rule_match(sid,
> f->type, f->op, f->lsm_rule);
> }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 47fb48f42c93..9973865cbf13 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -667,7 +667,7 @@ static int audit_filter_rules(struct task_struct *tsk,
> logged upon error */
> if (f->lsm_rule) {
> if (need_sid) {
> - security_task_getsecid(tsk, &sid);
> + security_task_getsecid_subj(tsk, &sid);
> need_sid = 0;
> }
> result = security_audit_rule_match(sid, f->type,
> @@ -2400,7 +2400,7 @@ void __audit_ptrace(struct task_struct *t)
> context->target_auid = audit_get_loginuid(t);
> context->target_uid = task_uid(t);
> context->target_sessionid = audit_get_sessionid(t);
> - security_task_getsecid(t, &context->target_sid);
> + security_task_getsecid_obj(t, &context->target_sid);
> memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
> }
>
> @@ -2427,7 +2427,7 @@ int audit_signal_info_syscall(struct task_struct *t)
> ctx->target_auid = audit_get_loginuid(t);
> ctx->target_uid = t_uid;
> ctx->target_sessionid = audit_get_sessionid(t);
> - security_task_getsecid(t, &ctx->target_sid);
> + security_task_getsecid_obj(t, &ctx->target_sid);
> memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
> return 0;
> }
> @@ -2448,7 +2448,7 @@ int audit_signal_info_syscall(struct task_struct *t)
> axp->target_auid[axp->pid_count] = audit_get_loginuid(t);
> axp->target_uid[axp->pid_count] = t_uid;
> axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
> - security_task_getsecid(t, &axp->target_sid[axp->pid_count]);
> + security_task_getsecid_obj(t, &axp->target_sid[axp->pid_count]);
> memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
> axp->pid_count++;
>
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 1622a44d1617..0ff58259ccf8 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -209,7 +209,8 @@ BTF_ID(func, bpf_lsm_socket_socketpair)
>
> BTF_ID(func, bpf_lsm_syslog)
> BTF_ID(func, bpf_lsm_task_alloc)
> -BTF_ID(func, bpf_lsm_task_getsecid)
> +BTF_ID(func, bpf_lsm_task_getsecid_subj)
> +BTF_ID(func, bpf_lsm_task_getsecid_obj)
> BTF_ID(func, bpf_lsm_task_prctl)
> BTF_ID(func, bpf_lsm_task_setscheduler)
> BTF_ID(func, bpf_lsm_task_to_inode)
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index ccb491642811..3e6ac9b790b1 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1539,7 +1539,7 @@ int __init netlbl_unlabel_defconf(void)
> /* Only the kernel is allowed to call this function and the only time
> * it is called is at bootup before the audit subsystem is reporting
> * messages so don't worry to much about these values. */
> - security_task_getsecid(current, &audit_info.secid);
> + security_task_getsecid_subj(current, &audit_info.secid);
> audit_info.loginuid = GLOBAL_ROOT_UID;
> audit_info.sessionid = 0;
>
> diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h
> index 3c67afce64f1..b9ba8112b3c5 100644
> --- a/net/netlabel/netlabel_user.h
> +++ b/net/netlabel/netlabel_user.h
> @@ -34,7 +34,7 @@
> static inline void netlbl_netlink_auditinfo(struct sk_buff *skb,
> struct netlbl_audit *audit_info)
> {
> - security_task_getsecid(current, &audit_info->secid);
> + security_task_getsecid_subj(current, &audit_info->secid);
> audit_info->loginuid = audit_get_loginuid(current);
> audit_info->sessionid = audit_get_sessionid(current);
> }
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 240a53387e6b..f72406fe1bf2 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1252,7 +1252,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>
> LSM_HOOK_INIT(task_free, apparmor_task_free),
> LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
> - LSM_HOOK_INIT(task_getsecid, apparmor_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, apparmor_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid),
> LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
> LSM_HOOK_INIT(task_kill, apparmor_task_kill),
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 565e33ff19d0..4e5eb0236278 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -76,7 +76,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
> if (!ima_appraise)
> return 0;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return ima_match_policy(mnt_userns, inode, current_cred(), secid, func,
> mask, IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL);
> }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 9ef748ea829f..b85d9e429426 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -391,7 +391,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
> u32 secid;
>
> if (file && (prot & PROT_EXEC)) {
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, NULL,
> 0, MAY_EXEC, MMAP_CHECK);
> }
> @@ -429,7 +429,7 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
> !(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC))
> return 0;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> inode = file_inode(vma->vm_file);
> action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode,
> current_cred(), secid, MAY_EXEC, MMAP_CHECK,
> @@ -470,7 +470,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
> int ret;
> u32 secid;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
> MAY_EXEC, BPRM_CHECK);
> if (ret)
> @@ -495,7 +495,7 @@ int ima_file_check(struct file *file, int mask)
> {
> u32 secid;
>
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, NULL, 0,
> mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
> MAY_APPEND), FILE_CHECK);
> @@ -686,7 +686,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
>
> /* Read entire file for all partial reads. */
> func = read_idmap[read_id] ?: FILE_CHECK;
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, NULL,
> 0, MAY_READ, func);
> }
> @@ -729,7 +729,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
> }
>
> func = read_idmap[read_id] ?: FILE_CHECK;
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> return process_measurement(file, current_cred(), secid, buf, size,
> MAY_READ, func);
> }
> @@ -872,7 +872,7 @@ void process_buffer_measurement(struct user_namespace *mnt_userns,
> * buffer measurements.
> */
> if (func) {
> - security_task_getsecid(current, &secid);
> + security_task_getsecid_subj(current, &secid);
> action = ima_get_action(mnt_userns, inode, current_cred(),
> secid, 0, func, &pcr, &template,
> func_data);
> diff --git a/security/security.c b/security/security.c
> index 5ac96b16f8fa..ee76d6c2f852 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1762,12 +1762,19 @@ int security_task_getsid(struct task_struct *p)
> return call_int_hook(task_getsid, 0, p);
> }
>
> -void security_task_getsecid(struct task_struct *p, u32 *secid)
> +void security_task_getsecid_subj(struct task_struct *p, u32 *secid)
> {
> *secid = 0;
> - call_void_hook(task_getsecid, p, secid);
> + call_void_hook(task_getsecid_subj, p, secid);
> }
> -EXPORT_SYMBOL(security_task_getsecid);
> +EXPORT_SYMBOL(security_task_getsecid_subj);
> +
> +void security_task_getsecid_obj(struct task_struct *p, u32 *secid)
> +{
> + *secid = 0;
> + call_void_hook(task_getsecid_obj, p, secid);
> +}
> +EXPORT_SYMBOL(security_task_getsecid_obj);
>
> int security_task_setnice(struct task_struct *p, int nice)
> {
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index eca9fc0ba764..24ca545c6e1b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7149,7 +7149,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
> LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> LSM_HOOK_INIT(task_getsid, selinux_task_getsid),
> - LSM_HOOK_INIT(task_getsecid, selinux_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid),
> LSM_HOOK_INIT(task_setnice, selinux_task_setnice),
> LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio),
> LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio),
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 12a45e61c1a5..f546fb832f30 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4759,7 +4759,8 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
> LSM_HOOK_INIT(task_getpgid, smack_task_getpgid),
> LSM_HOOK_INIT(task_getsid, smack_task_getsid),
> - LSM_HOOK_INIT(task_getsecid, smack_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, smack_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_obj, smack_task_getsecid),
> LSM_HOOK_INIT(task_setnice, smack_task_setnice),
> LSM_HOOK_INIT(task_setioprio, smack_task_setioprio),
> LSM_HOOK_INIT(task_getioprio, smack_task_getioprio),
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] selinux: clarify task subjective and objective credentials
2021-03-18 20:42 ` [PATCH v2 2/3] selinux: clarify task subjective and objective credentials Paul Moore
@ 2021-03-19 13:36 ` Richard Guy Briggs
0 siblings, 0 replies; 9+ messages in thread
From: Richard Guy Briggs @ 2021-03-19 13:36 UTC (permalink / raw)
To: Paul Moore; +Cc: selinux, linux-security-module, linux-audit, John Johansen
On 2021-03-18 16:42, Paul Moore wrote:
> SELinux has a function, task_sid(), which returns the task's
> objective credentials, but unfortunately is used in a few places
> where the subjective task credentials should be used. Most notably
> in the new security_task_getsecid_subj() LSM hook.
>
> This patch fixes this and attempts to make things more obvious by
> introducing a new function, task_sid_subj(), and renaming the
> existing task_sid() function to task_sid_obj().
>
> This patch also adds an interesting function in task_sid_binder().
> The task_sid_binder() function has a comment which hopefully
> describes it's reason for being, but it basically boils down to the
> simple fact that we can't safely access another task's subjective
> credentials so in the case of binder we need to stick with the
> objective credentials regardless.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> security/selinux/hooks.c | 112 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 73 insertions(+), 39 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 24ca545c6e1b..901a3ce453f1 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -229,10 +229,23 @@ static inline u32 cred_sid(const struct cred *cred)
> return tsec->sid;
> }
>
> +/*
> + * get the subjective security ID of a task
> + */
> +static inline u32 task_sid_subj(const struct task_struct *task)
> +{
> + u32 sid;
> +
> + rcu_read_lock();
> + sid = cred_sid(rcu_dereference(task->cred));
> + rcu_read_unlock();
> + return sid;
> +}
> +
> /*
> * get the objective security ID of a task
> */
> -static inline u32 task_sid(const struct task_struct *task)
> +static inline u32 task_sid_obj(const struct task_struct *task)
> {
> u32 sid;
>
> @@ -242,6 +255,29 @@ static inline u32 task_sid(const struct task_struct *task)
> return sid;
> }
>
> +/*
> + * get the security ID of a task for use with binder
> + */
> +static inline u32 task_sid_binder(const struct task_struct *task)
> +{
> + /*
> + * In many case where this function is used we should be using the
> + * task's subjective SID, but we can't reliably access the subjective
> + * creds of a task other than our own so we must use the objective
> + * creds/SID, which are safe to access. The downside is that if a task
> + * is temporarily overriding it's creds it will not be reflected here;
> + * however, it isn't clear that binder would handle that case well
> + * anyway.
> + *
> + * If this ever changes and we can safely reference the subjective
> + * creds/SID of another task, this function will make it easier to
> + * identify the various places where we make use of the task SIDs in
> + * the binder code. It is also likely that we will need to adjust
> + * the main drivers/android binder code as well.
> + */
> + return task_sid_obj(task);
> +}
> +
> static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry);
>
> /*
> @@ -2035,11 +2071,8 @@ static inline u32 open_file_to_av(struct file *file)
>
> static int selinux_binder_set_context_mgr(struct task_struct *mgr)
> {
> - u32 mysid = current_sid();
> - u32 mgrsid = task_sid(mgr);
> -
> return avc_has_perm(&selinux_state,
> - mysid, mgrsid, SECCLASS_BINDER,
> + current_sid(), task_sid_binder(mgr), SECCLASS_BINDER,
> BINDER__SET_CONTEXT_MGR, NULL);
> }
>
> @@ -2047,8 +2080,7 @@ static int selinux_binder_transaction(struct task_struct *from,
> struct task_struct *to)
> {
> u32 mysid = current_sid();
> - u32 fromsid = task_sid(from);
> - u32 tosid = task_sid(to);
> + u32 fromsid = task_sid_binder(from);
> int rc;
>
> if (mysid != fromsid) {
> @@ -2059,19 +2091,16 @@ static int selinux_binder_transaction(struct task_struct *from,
> return rc;
> }
>
> - return avc_has_perm(&selinux_state,
> - fromsid, tosid, SECCLASS_BINDER, BINDER__CALL,
> - NULL);
> + return avc_has_perm(&selinux_state, fromsid, task_sid_binder(to),
> + SECCLASS_BINDER, BINDER__CALL, NULL);
> }
>
> static int selinux_binder_transfer_binder(struct task_struct *from,
> struct task_struct *to)
> {
> - u32 fromsid = task_sid(from);
> - u32 tosid = task_sid(to);
> -
> return avc_has_perm(&selinux_state,
> - fromsid, tosid, SECCLASS_BINDER, BINDER__TRANSFER,
> + task_sid_binder(from), task_sid_binder(to),
> + SECCLASS_BINDER, BINDER__TRANSFER,
> NULL);
> }
>
> @@ -2079,7 +2108,7 @@ static int selinux_binder_transfer_file(struct task_struct *from,
> struct task_struct *to,
> struct file *file)
> {
> - u32 sid = task_sid(to);
> + u32 sid = task_sid_binder(to);
> struct file_security_struct *fsec = selinux_file(file);
> struct dentry *dentry = file->f_path.dentry;
> struct inode_security_struct *isec;
> @@ -2115,10 +2144,10 @@ static int selinux_binder_transfer_file(struct task_struct *from,
> }
>
> static int selinux_ptrace_access_check(struct task_struct *child,
> - unsigned int mode)
> + unsigned int mode)
> {
> u32 sid = current_sid();
> - u32 csid = task_sid(child);
> + u32 csid = task_sid_obj(child);
>
> if (mode & PTRACE_MODE_READ)
> return avc_has_perm(&selinux_state,
> @@ -2131,15 +2160,15 @@ static int selinux_ptrace_access_check(struct task_struct *child,
> static int selinux_ptrace_traceme(struct task_struct *parent)
> {
> return avc_has_perm(&selinux_state,
> - task_sid(parent), current_sid(), SECCLASS_PROCESS,
> - PROCESS__PTRACE, NULL);
> + task_sid_subj(parent), task_sid_obj(current),
> + SECCLASS_PROCESS, PROCESS__PTRACE, NULL);
> }
>
> static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
> kernel_cap_t *inheritable, kernel_cap_t *permitted)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(target), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(target), SECCLASS_PROCESS,
> PROCESS__GETCAP, NULL);
> }
>
> @@ -2264,7 +2293,7 @@ static u32 ptrace_parent_sid(void)
> rcu_read_lock();
> tracer = ptrace_parent(current);
> if (tracer)
> - sid = task_sid(tracer);
> + sid = task_sid_obj(tracer);
> rcu_read_unlock();
>
> return sid;
> @@ -3921,7 +3950,7 @@ static int selinux_file_send_sigiotask(struct task_struct *tsk,
> struct fown_struct *fown, int signum)
> {
> struct file *file;
> - u32 sid = task_sid(tsk);
> + u32 sid = task_sid_obj(tsk);
> u32 perm;
> struct file_security_struct *fsec;
>
> @@ -4140,47 +4169,52 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
> static int selinux_task_setpgid(struct task_struct *p, pid_t pgid)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__SETPGID, NULL);
> }
>
> static int selinux_task_getpgid(struct task_struct *p)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__GETPGID, NULL);
> }
>
> static int selinux_task_getsid(struct task_struct *p)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__GETSESSION, NULL);
> }
>
> -static void selinux_task_getsecid(struct task_struct *p, u32 *secid)
> +static void selinux_task_getsecid_subj(struct task_struct *p, u32 *secid)
> +{
> + *secid = task_sid_subj(p);
> +}
> +
> +static void selinux_task_getsecid_obj(struct task_struct *p, u32 *secid)
> {
> - *secid = task_sid(p);
> + *secid = task_sid_obj(p);
> }
>
> static int selinux_task_setnice(struct task_struct *p, int nice)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__SETSCHED, NULL);
> }
>
> static int selinux_task_setioprio(struct task_struct *p, int ioprio)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__SETSCHED, NULL);
> }
>
> static int selinux_task_getioprio(struct task_struct *p)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__GETSCHED, NULL);
> }
>
> @@ -4211,7 +4245,7 @@ static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource,
> upon context transitions. See selinux_bprm_committing_creds. */
> if (old_rlim->rlim_max != new_rlim->rlim_max)
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p),
> + current_sid(), task_sid_obj(p),
> SECCLASS_PROCESS, PROCESS__SETRLIMIT, NULL);
>
> return 0;
> @@ -4220,21 +4254,21 @@ static int selinux_task_setrlimit(struct task_struct *p, unsigned int resource,
> static int selinux_task_setscheduler(struct task_struct *p)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__SETSCHED, NULL);
> }
>
> static int selinux_task_getscheduler(struct task_struct *p)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__GETSCHED, NULL);
> }
>
> static int selinux_task_movememory(struct task_struct *p)
> {
> return avc_has_perm(&selinux_state,
> - current_sid(), task_sid(p), SECCLASS_PROCESS,
> + current_sid(), task_sid_obj(p), SECCLASS_PROCESS,
> PROCESS__SETSCHED, NULL);
> }
>
> @@ -4253,14 +4287,14 @@ static int selinux_task_kill(struct task_struct *p, struct kernel_siginfo *info,
> else
> secid = cred_sid(cred);
> return avc_has_perm(&selinux_state,
> - secid, task_sid(p), SECCLASS_PROCESS, perm, NULL);
> + secid, task_sid_obj(p), SECCLASS_PROCESS, perm, NULL);
> }
>
> static void selinux_task_to_inode(struct task_struct *p,
> struct inode *inode)
> {
> struct inode_security_struct *isec = selinux_inode(inode);
> - u32 sid = task_sid(p);
> + u32 sid = task_sid_obj(p);
>
> spin_lock(&isec->lock);
> isec->sclass = inode_mode_to_security_class(inode->i_mode);
> @@ -6153,7 +6187,7 @@ static int selinux_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *m
> struct ipc_security_struct *isec;
> struct msg_security_struct *msec;
> struct common_audit_data ad;
> - u32 sid = task_sid(target);
> + u32 sid = task_sid_subj(target);
> int rc;
>
> isec = selinux_ipc(msq);
> @@ -7149,8 +7183,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid),
> LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid),
> LSM_HOOK_INIT(task_getsid, selinux_task_getsid),
> - LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid),
> - LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid),
> + LSM_HOOK_INIT(task_getsecid_subj, selinux_task_getsecid_subj),
> + LSM_HOOK_INIT(task_getsecid_obj, selinux_task_getsecid_obj),
> LSM_HOOK_INIT(task_setnice, selinux_task_setnice),
> LSM_HOOK_INIT(task_setioprio, selinux_task_setioprio),
> LSM_HOOK_INIT(task_getioprio, selinux_task_getioprio),
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants
2021-03-18 20:42 [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
` (2 preceding siblings ...)
2021-03-18 20:42 ` [PATCH v2 3/3] smack: differentiate between subjective and objective task credentials Paul Moore
@ 2021-03-22 19:29 ` Paul Moore
3 siblings, 0 replies; 9+ messages in thread
From: Paul Moore @ 2021-03-22 19:29 UTC (permalink / raw)
To: linux-security-module, linux-audit, selinux
Cc: Richard Guy Briggs, John Johansen
On Thu, Mar 18, 2021 at 4:42 PM Paul Moore <paul@paul-moore.com> wrote:
>
> An update on the previous RFC patchset found here:
>
> https://lore.kernel.org/linux-security-module/161377712068.87807.12246856567527156637.stgit@sifl/
>
> Aside from being rebased to the current SELinux next branch (which
> in turn is based on v5.12-rc2), this revision changes the binder
> related code to always use the objective credentials as discussed
> in the thread above. I've also dropped the AppArmor patch as John
> has a better version in progress; in the meantime AppArmor should
> continue to work exactly as it did before this patchset so there
> is no harm in merging this without the AppArmor patch.
>
> Casey, John, and Richard; I dropped your ACKs, Reviewed-by, etc.
> tags as the binder changes seemed substantial enough to not
> carryover your tags. I would appreciate it if you could re-review
> this revision; the changes should be minimal. I did leave Mimi's
> tag on this revision as she qualified it for IMA and that code
> didn't change.
>
> ---
>
> Paul Moore (3):
> lsm: separate security_task_getsecid() into subjective and objective variants
> selinux: clarify task subjective and objective credentials
> smack: differentiate between subjective and objective task credentials
>
>
> security/selinux/hooks.c | 112 ++++++++++++++++++++++++-------------
> security/smack/smack.h | 18 +++++-
> security/smack/smack_lsm.c | 40 ++++++++-----
> 3 files changed, 117 insertions(+), 53 deletions(-)
All three patches have been merged into selinux/next, thanks for the
help/review everyone!
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-22 19:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 20:42 [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
2021-03-18 20:42 ` [PATCH v2 1/3] lsm: separate security_task_getsecid() into subjective and objective variants Paul Moore
2021-03-18 20:57 ` Casey Schaufler
2021-03-19 3:44 ` Paul Moore
2021-03-19 13:35 ` Richard Guy Briggs
2021-03-18 20:42 ` [PATCH v2 2/3] selinux: clarify task subjective and objective credentials Paul Moore
2021-03-19 13:36 ` Richard Guy Briggs
2021-03-18 20:42 ` [PATCH v2 3/3] smack: differentiate between subjective and objective task credentials Paul Moore
2021-03-22 19:29 ` [PATCH v2 0/3] Split security_task_getsecid() into subj and obj variants Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).