| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3ccb78ef-905c-4914-c77a-24765c0e6675@schaufler-ca.com>
Date: Mon, 20 Dec 2021 08:41:48 -0800
From: Casey Schaufler <casey@...aufler-ca.com>
To: Vishal Goel <vishal.goel@...sung.com>,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: a.sahrawat@...sung.com, v.narang@...sung.com,
"linux-audit@...hat.com" <linux-audit@...hat.com>
Subject: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace
error logs
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@...sung.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)
Powered by blists - more mailing lists