[PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
Casey Schaufler
casey at schaufler-ca.com
Tue Dec 21 01:19:14 UTC 2021
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 at 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)
More information about the Linux-security-module-archive
mailing list