Messages in this thread | | | Date | Mon, 20 Dec 2021 17:19:14 -0800 | Subject | Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs | From | Casey Schaufler <> |
| |
On 12/20/2021 8:41 AM, Casey Schaufler wrote: > On 12/20/2021 2:13 AM, Vishal Goel wrote: >> Currently tracer process info is printed in object field in >> smack error log for ptrace check which is wrong. >> Object process should print the tracee process info. >> Tracee info is not printed in the smack error logs. >> So it is not possible to debug the ptrace smack issues. >> >> Now changes has been done to print both tracer and tracee >> process info in smack error logs for ptrace scenarios >> >> Old logs:- >> [ 378.098330] audit: type=1400 audit(1637212273.300:2): lsm=SMACK fn=smack_ptrace_access_check action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= pid=9397 comm="tst_pt" opid=9397 ocomm="tst_pt" >> [ 520.261605] audit: type=1400 audit(1637212415.464:3): lsm=SMACK fn=smack_ptrace_traceme action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= pid=12685 comm="tst_pt_me" opid=12563 ocomm="bash" >> [ 1445.259319] audit: type=1400 audit(1637213340.460:5): lsm=SMACK fn=smack_bprm_set_creds action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= pid=1778 comm="tst_bprm" opid=1776 ocomm="tst_bprm" >> >> New logs:- >> [ 378.098330] audit: type=1400 audit(1637212273.300:2): lsm=SMACK fn=smack_ptrace_access_check action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= tracer-pid=5189 tracer-comm="tst_pt" pid=5189 comm="tst_pt" tracee-pid=962 tracee-comm="test_tracee" >> [ 520.261605] audit: type=1400 audit(1637212415.464:3): lsm=SMACK fn=smack_ptrace_traceme action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= tracer-pid=6161 tracer-comm="bash" pid=6310 comm="tst_pt_me" tracee-pid=6310 tracee-comm="tst_pt_me" >> [ 1445.259319] audit: type=1400 audit(1637213340.460:5): lsm=SMACK fn=smack_bprm_set_creds action=denied subject="Tracer_lbl" object="Tracee_lbl" requested= tracer-pid=6435 tracer-comm="tst_bprm" pid=6436 comm="tst_bprm" tracee-pid=6436 tracee-comm="tst_bprm" >> >> Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
What test case do you have that generates these records?
> > Added linux-audit to the CC list. > >> --- >> include/linux/lsm_audit.h | 1 + >> security/lsm_audit.c | 15 +++++++++++++++ >> security/smack/smack.h | 19 +++++++++++++++++++ >> security/smack/smack_access.c | 16 ++++++++++++++++ >> security/smack/smack_lsm.c | 33 ++++++++++++++++++++++----------- >> 5 files changed, 73 insertions(+), 11 deletions(-) >> >> diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h >> index 17d02eda9..6d752ea16 100644 >> --- a/include/linux/lsm_audit.h >> +++ b/include/linux/lsm_audit.h >> @@ -76,6 +76,7 @@ struct common_audit_data { >> #define LSM_AUDIT_DATA_IBENDPORT 14 >> #define LSM_AUDIT_DATA_LOCKDOWN 15 >> #define LSM_AUDIT_DATA_NOTIFICATION 16 >> +#define LSM_AUDIT_DATA_PTRACE 17 >> union { >> struct path path; >> struct dentry *dentry; >> diff --git a/security/lsm_audit.c b/security/lsm_audit.c >> index 1897cbf6f..069e0282c 100644 >> --- a/security/lsm_audit.c >> +++ b/security/lsm_audit.c >> @@ -318,6 +318,21 @@ static void dump_common_audit_data(struct audit_buffer *ab, >> } >> break; >> } >> + case LSM_AUDIT_DATA_PTRACE: { >> + struct task_struct *tsk = a->u.tsk; >> + if (tsk) { >> + pid_t pid = task_tgid_nr(tsk); >> + >> + if (pid) { >> + char comm[sizeof(tsk->comm)]; >> + >> + audit_log_format(ab, " tracee-pid=%d tracee-comm=", pid); >> + audit_log_untrustedstring(ab, >> + memcpy(comm, tsk->comm, sizeof(comm))); >> + } >> + } >> + break; >> + } >> case LSM_AUDIT_DATA_NET: >> if (a->u.net->sk) { >> const struct sock *sk = a->u.net->sk; >> diff --git a/security/smack/smack.h b/security/smack/smack.h >> index 99c342259..901228205 100644 >> --- a/security/smack/smack.h >> +++ b/security/smack/smack.h >> @@ -266,6 +266,7 @@ struct smack_audit_data { >> char *object; >> char *request; >> int result; >> + struct task_struct *tracer_tsk; >> }; >> /* >> @@ -497,6 +498,16 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a, >> { >> a->a.u.net->sk = sk; >> } >> +static inline void smk_ad_setfield_u_tracer(struct smk_audit_info *a, >> + struct task_struct *t) >> +{ >> + a->a.smack_audit_data->tracer_tsk = t; >> +} >> +static inline void smk_ad_setfield_u_tracee(struct smk_audit_info *a, >> + struct task_struct *t) >> +{ >> + a->a.u.tsk = t; >> +} >> #else /* no AUDIT */ >> @@ -524,6 +535,14 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a, >> struct sock *sk) >> { >> } >> +static inline void smk_ad_setfield_u_tracer(struct smk_audit_info *a, >> + struct task_struct *t) >> +{ >> +} >> +static inline void smk_ad_setfield_u_tracee(struct smk_audit_info *a, >> + struct task_struct *t) >> +{ >> +} >> #endif >> #endif /* _SECURITY_SMACK_H */ >> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c >> index d2186e275..f39e3ac92 100644 >> --- a/security/smack/smack_access.c >> +++ b/security/smack/smack_access.c >> @@ -323,6 +323,22 @@ static void smack_log_callback(struct audit_buffer *ab, void *a) >> audit_log_format(ab, " labels_differ"); >> else >> audit_log_format(ab, " requested=%s", sad->request); >> + >> + if (ad->type == LSM_AUDIT_DATA_PTRACE) { >> + struct task_struct *tsk = sad->tracer_tsk; >> + >> + if (tsk) { >> + pid_t pid = task_tgid_nr(tsk); >> + >> + if (pid) { >> + char comm[sizeof(tsk->comm)]; >> + >> + audit_log_format(ab, " tracer-pid=%d tracer-comm=", pid); >> + audit_log_untrustedstring(ab, >> + memcpy(comm, tsk->comm, sizeof(comm))); >> + } >> + } >> + } >> } >> /** >> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c >> index efd35b07c..47e8a9461 100644 >> --- a/security/smack/smack_lsm.c >> +++ b/security/smack/smack_lsm.c >> @@ -416,20 +416,13 @@ static inline unsigned int smk_ptrace_mode(unsigned int mode) >> */ >> static int smk_ptrace_rule_check(struct task_struct *tracer, >> struct smack_known *tracee_known, >> - unsigned int mode, const char *func) >> + unsigned int mode, struct smk_audit_info *saip) >> { >> int rc; >> - struct smk_audit_info ad, *saip = NULL; >> struct task_smack *tsp; >> struct smack_known *tracer_known; >> const struct cred *tracercred; >> - if ((mode & PTRACE_MODE_NOAUDIT) == 0) { >> - smk_ad_init(&ad, func, LSM_AUDIT_DATA_TASK); >> - smk_ad_setfield_u_tsk(&ad, tracer); >> - saip = &ad; >> - } >> - >> rcu_read_lock(); >> tracercred = __task_cred(tracer); >> tsp = smack_cred(tracercred); >> @@ -480,10 +473,17 @@ static int smk_ptrace_rule_check(struct task_struct *tracer, >> static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode) >> { >> struct smack_known *skp; >> + struct smk_audit_info ad, *saip = NULL; >> skp = smk_of_task_struct_obj(ctp); >> + if ((mode & PTRACE_MODE_NOAUDIT) == 0) { >> + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PTRACE); >> + smk_ad_setfield_u_tracer(&ad, current); >> + smk_ad_setfield_u_tracee(&ad, ctp); >> + saip = &ad; >> + } >> - return smk_ptrace_rule_check(current, skp, mode, __func__); >> + return smk_ptrace_rule_check(current, skp, mode, saip); >> } >> /** >> @@ -498,10 +498,15 @@ static int smack_ptrace_traceme(struct task_struct *ptp) >> { >> int rc; >> struct smack_known *skp; >> + struct smk_audit_info ad, *saip = NULL; >> skp = smk_of_task(smack_cred(current_cred())); >> + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PTRACE); >> + smk_ad_setfield_u_tracer(&ad, ptp); >> + smk_ad_setfield_u_tracee(&ad, current); >> + saip = &ad; >> - rc = smk_ptrace_rule_check(ptp, skp, PTRACE_MODE_ATTACH, __func__); >> + rc = smk_ptrace_rule_check(ptp, skp, PTRACE_MODE_ATTACH, saip); >> return rc; >> } >> @@ -897,15 +902,21 @@ static int smack_bprm_creds_for_exec(struct linux_binprm *bprm) >> if (bprm->unsafe & LSM_UNSAFE_PTRACE) { >> struct task_struct *tracer; >> + struct smk_audit_info ad, *saip = NULL; >> rc = 0; >> + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PTRACE); >> + smk_ad_setfield_u_tracee(&ad, current); >> + saip = &ad; >> + >> rcu_read_lock(); >> tracer = ptrace_parent(current); >> + smk_ad_setfield_u_tracer(&ad, tracer); >> if (likely(tracer != NULL)) >> rc = smk_ptrace_rule_check(tracer, >> isp->smk_task, >> PTRACE_MODE_ATTACH, >> - __func__); >> + saip); >> rcu_read_unlock(); >> if (rc != 0)
| |