| Message ID | 20211220101318.3538824-1-vishal.goel@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs | expand |
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> 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)
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)
Hi, >>> Signed-off-by: Vishal Goel <vishal.goel@samsung.com> > > What test case do you have that generates these records? Test case for 1st log:- void main(int argc,char *argv[]) { int pid; if (argc < 2) { printf("enter pid of the tracee process\n"); exit(0); } pid = atoi(argv[1]); fprintf(stderr,"Inside\n"); ptrace(PTRACE_ATTACH, pid,NULL,NULL); while(1) { sleep(10); } } Test case for 2nd log:- void main(int argc,char *argv[]) { int pid; pid = getpid(); fprintf(stderr,"Inside\n"); ptrace(PTRACE_TRACEME, pid,NULL,NULL); while(1) { sleep(10); } } Test case for 3rd log:- void main() { int pid; char *argv[2]; fprintf(stderr,"Inside\n"); pid = fork(); if(pid == 0) { argv[0] = "/tst_pt"; argv[1] = NULL; if(ptrace(PTRACE_TRACEME, pid,NULL,NULL)) printf("attached child\n"); printf("going for exec\n"); execv("/tst_pt",argv); } else { while(1) { sleep(10); } } } >> >> Added linux-audit to the CC list. >> Thanks Vishal Goel
On 12/21/2021 5:12 AM, Vishal Goel wrote: > Hi, > >>>> Signed-off-by: Vishal Goel <vishal.goel@samsung.com> >> What test case do you have that generates these records? Could you include a permissive license with this code? I'd like to add it or a derivative of it to the Smack test suite. > Test case for 1st log:- > void main(int argc,char *argv[]) > { > int pid; > > if (argc < 2) { > printf("enter pid of the tracee process\n"); > exit(0); > } > > pid = atoi(argv[1]); > fprintf(stderr,"Inside\n"); > ptrace(PTRACE_ATTACH, pid,NULL,NULL); > while(1) > { > sleep(10); > } > } > > Test case for 2nd log:- > void main(int argc,char *argv[]) > { > int pid; > > pid = getpid(); > fprintf(stderr,"Inside\n"); > ptrace(PTRACE_TRACEME, pid,NULL,NULL); > while(1) > { > sleep(10); > } > } > > Test case for 3rd log:- > void main() > { > int pid; > char *argv[2]; > > fprintf(stderr,"Inside\n"); > pid = fork(); > if(pid == 0) { > argv[0] = "/tst_pt"; > argv[1] = NULL; > > if(ptrace(PTRACE_TRACEME, pid,NULL,NULL)) > printf("attached child\n"); > > printf("going for exec\n"); > execv("/tst_pt",argv); > } > else > { > while(1) > { > sleep(10); > } > } > } > >>> Added linux-audit to the CC list. >>> > Thanks > Vishal Goel
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)
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> --- 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(-)