* [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
[not found] <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcas5p3.samsung.com>
@ 2021-12-20 10:13 ` Vishal Goel
2021-12-20 16:41 ` Casey Schaufler
2022-01-28 16:24 ` Casey Schaufler
0 siblings, 2 replies; 19+ messages in thread
From: Vishal Goel @ 2021-12-20 10:13 UTC (permalink / raw)
To: casey, linux-security-module, linux-kernel
Cc: a.sahrawat, v.narang, Vishal Goel
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(-)
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)
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2021-12-20 10:13 ` [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs Vishal Goel
@ 2021-12-20 16:41 ` Casey Schaufler
2022-01-28 16:24 ` Casey Schaufler
1 sibling, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2021-12-20 16:41 UTC (permalink / raw)
To: Vishal Goel, linux-security-module, linux-kernel
Cc: a.sahrawat, v.narang, linux-audit
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)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
@ 2021-12-20 16:41 ` Casey Schaufler
0 siblings, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2021-12-20 16:41 UTC (permalink / raw)
To: Vishal Goel, linux-security-module, linux-kernel
Cc: v.narang, a.sahrawat, linux-audit
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)
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2021-12-20 16:41 ` Casey Schaufler
@ 2021-12-21 1:19 ` Casey Schaufler
-1 siblings, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2021-12-21 1:19 UTC (permalink / raw)
To: Vishal Goel, linux-security-module, linux-kernel
Cc: a.sahrawat, v.narang, linux-audit, 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)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
@ 2021-12-21 1:19 ` Casey Schaufler
0 siblings, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2021-12-21 1:19 UTC (permalink / raw)
To: Vishal Goel, linux-security-module, linux-kernel
Cc: v.narang, a.sahrawat, linux-audit
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)
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p2>
@ 2021-12-21 13:12 ` Vishal Goel
0 siblings, 0 replies; 19+ messages in thread
From: Vishal Goel @ 2021-12-21 13:12 UTC (permalink / raw)
To: Casey Schaufler, linux-security-module, linux-kernel
Cc: AMIT SAHRAWAT, Vaneet Narang, linux-audit
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
@ 2021-12-21 13:12 ` Vishal Goel
0 siblings, 0 replies; 19+ messages in thread
From: Vishal Goel @ 2021-12-21 13:12 UTC (permalink / raw)
To: Casey Schaufler, linux-security-module, linux-kernel
Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2021-12-21 13:12 ` Vishal Goel
@ 2021-12-21 17:01 ` Casey Schaufler
-1 siblings, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2021-12-21 17:01 UTC (permalink / raw)
To: vishal.goel, linux-security-module, linux-kernel
Cc: AMIT SAHRAWAT, Vaneet Narang, linux-audit, Casey Schaufler
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
@ 2021-12-21 17:01 ` Casey Schaufler
0 siblings, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2021-12-21 17:01 UTC (permalink / raw)
To: vishal.goel, linux-security-module, linux-kernel
Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
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
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p6>
@ 2021-12-24 5:04 ` Vishal Goel
0 siblings, 0 replies; 19+ messages in thread
From: Vishal Goel @ 2021-12-24 5:04 UTC (permalink / raw)
To: Casey Schaufler, linux-security-module, linux-kernel
Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
[-- Attachment #1.1: Type: text/html, Size: 6766 bytes --]
[-- Attachment #1.2: Type: image/gif, Size: 13402 bytes --]
[-- Attachment #2: Type: text/plain, Size: 106 bytes --]
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2021-12-20 10:13 ` [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs Vishal Goel
2021-12-20 16:41 ` Casey Schaufler
@ 2022-01-28 16:24 ` Casey Schaufler
2022-01-28 18:00 ` Paul Moore
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p8>
1 sibling, 2 replies; 19+ messages in thread
From: Casey Schaufler @ 2022-01-28 16:24 UTC (permalink / raw)
To: linux-audit; +Cc: v.narang, a.sahrawat, Vishal Goel
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>
Does anyone from the audit side object to my taking this
in the Smack tree?
> ---
> 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)
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2022-01-28 16:24 ` Casey Schaufler
@ 2022-01-28 18:00 ` Paul Moore
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p8>
1 sibling, 0 replies; 19+ messages in thread
From: Paul Moore @ 2022-01-28 18:00 UTC (permalink / raw)
To: Casey Schaufler; +Cc: v.narang, a.sahrawat, linux-audit, Vishal Goel
On Fri, Jan 28, 2022 at 11:25 AM Casey Schaufler <casey@schaufler-ca.com> 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>
>
> Does anyone from the audit side object to my taking this
> in the Smack tree?
The audit subsystem already has the "opid" and "ocomm" fields for
reporting on the object task info and this is even available in
dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
that can't be used instead?
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p8>
@ 2022-02-01 7:54 ` Vishal Goel
2022-02-01 14:05 ` Paul Moore
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p4>
0 siblings, 2 replies; 19+ messages in thread
From: Vishal Goel @ 2022-02-01 7:54 UTC (permalink / raw)
To: Paul Moore, Casey Schaufler; +Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
>>> 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>
>>
>> Does anyone from the audit side object to my taking this
>> in the Smack tree?
> The audit subsystem already has the "opid" and "ocomm" fields for
> reporting on the object task info and this is even available in
> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
> that can't be used instead?
That info is not sufficient for debugging smack issues in ptrace calls.
Tracee information is not printed in the logs. For eg. in below log-
[ 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"
There is no information of the tracee process.
So to debug such ptrace issues, both tracer and tracee information is needed.
That's why added new type to print both info specifically for ptrace scenarios.
Thanks & Regards
Vishal Goel
--------- Original Message ---------
Sender : Paul Moore <paul@paul-moore.com>
Date : 2022-01-29 03:00 (GMT+9)
Title : Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
On Fri, Jan 28, 2022 at 11:25 AM Casey Schaufler <casey@schaufler-ca.com> 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>
>
> Does anyone from the audit side object to my taking this
> in the Smack tree?
The audit subsystem already has the "opid" and "ocomm" fields for
reporting on the object task info and this is even available in
dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
that can't be used instead?
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2022-02-01 7:54 ` Vishal Goel
@ 2022-02-01 14:05 ` Paul Moore
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p4>
1 sibling, 0 replies; 19+ messages in thread
From: Paul Moore @ 2022-02-01 14:05 UTC (permalink / raw)
To: vishal.goel; +Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
On Tue, Feb 1, 2022 at 2:55 AM Vishal Goel <vishal.goel@samsung.com> 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>
> >>
> >> Does anyone from the audit side object to my taking this
> >> in the Smack tree?
>
> > The audit subsystem already has the "opid" and "ocomm" fields for
> > reporting on the object task info and this is even available in
> > dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
> > that can't be used instead?
>
> That info is not sufficient for debugging smack issues in ptrace calls.
> Tracee information is not printed in the logs. For eg. in below log-
> [ 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"
>
> There is no information of the tracee process.
> So to debug such ptrace issues, both tracer and tracee information is needed.
> That's why added new type to print both info specifically for ptrace scenarios.
[NOTE: please only send plaintext email to the lists]
>From what I saw you are trying to record information about the tracer
and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
should be used instead of adding new fields.
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Re: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p4>
@ 2022-02-02 10:37 ` Vishal Goel
2022-02-02 15:20 ` Paul Moore
0 siblings, 1 reply; 19+ messages in thread
From: Vishal Goel @ 2022-02-02 10:37 UTC (permalink / raw)
To: Paul Moore; +Cc: Vaneet Narang, SAHRAWAT, linux-audit, AMIT
>>>>> 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>
>>>>
>>>> Does anyone from the audit side object to my taking this
>>>> in the Smack tree?
>>
>>> The audit subsystem already has the "opid" and "ocomm" fields for
>>> reporting on the object task info and this is even available in
>>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
>>> that can't be used instead?
>>
>> That info is not sufficient for debugging smack issues in ptrace calls.
>> Tracee information is not printed in the logs. For eg. in below log-
>> [ 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"
>>
>> There is no information of the tracee process.
>> So to debug such ptrace issues, both tracer and tracee information is needed.
>> That's why added new type to print both info specifically for ptrace scenarios.
> From what I saw you are trying to record information about the tracer
> and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
> should be used instead of adding new fields.
Actually in smack_ptrace_access_check() function, tracer process is current process.
While some other process is object process(tracee).
But in case of smack_ptrace_traceme() function, tracer process is parent process.
While current process is object process(tracee). So in this case, both pid/comm
and opid/ocomm will print current process info only i.e tracess process.
So tracer process info is not getting printed.
Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
And current process is tracee process.
So that's why we need to print separately tracer and tracee process info
without any confusion.
--------- Original Message ---------
Sender : Paul Moore <paul@paul-moore.com>
Date : 2022-02-01 23:05 (GMT+9)
Title : Re: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
On Tue, Feb 1, 2022 at 2:55 AM Vishal Goel <vishal.goel@samsung.com> 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>
> >>
> >> Does anyone from the audit side object to my taking this
> >> in the Smack tree?
>
> > The audit subsystem already has the "opid" and "ocomm" fields for
> > reporting on the object task info and this is even available in
> > dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
> > that can't be used instead?
>
> That info is not sufficient for debugging smack issues in ptrace calls.
> Tracee information is not printed in the logs. For eg. in below log-
> [ 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"
>
> There is no information of the tracee process.
> So to debug such ptrace issues, both tracer and tracee information is needed.
> That's why added new type to print both info specifically for ptrace scenarios.
[NOTE: please only send plaintext email to the lists]
From what I saw you are trying to record information about the tracer
and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
should be used instead of adding new fields.
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2022-02-02 10:37 ` Vishal Goel
@ 2022-02-02 15:20 ` Paul Moore
2022-02-02 18:12 ` Casey Schaufler
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p7>
0 siblings, 2 replies; 19+ messages in thread
From: Paul Moore @ 2022-02-02 15:20 UTC (permalink / raw)
To: vishal.goel; +Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
On Wed, Feb 2, 2022 at 5:38 AM Vishal Goel <vishal.goel@samsung.com> 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>
> >>>>
> >>>> Does anyone from the audit side object to my taking this
> >>>> in the Smack tree?
> >>
> >>> The audit subsystem already has the "opid" and "ocomm" fields for
> >>> reporting on the object task info and this is even available in
> >>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
> >>> that can't be used instead?
> >>
> >> That info is not sufficient for debugging smack issues in ptrace calls.
> >> Tracee information is not printed in the logs. For eg. in below log-
> >> [ 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"
> >>
> >> There is no information of the tracee process.
> >> So to debug such ptrace issues, both tracer and tracee information is needed.
> >> That's why added new type to print both info specifically for ptrace scenarios.
>
>
> > From what I saw you are trying to record information about the tracer
> > and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
> > should be used instead of adding new fields.
>
> Actually in smack_ptrace_access_check() function, tracer process is current process.
> While some other process is object process(tracee).
> But in case of smack_ptrace_traceme() function, tracer process is parent process.
> While current process is object process(tracee). So in this case, both pid/comm
> and opid/ocomm will print current process info only i.e tracess process.
> So tracer process info is not getting printed.
> Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
> And current process is tracee process.
> So that's why we need to print separately tracer and tracee process info
> without any confusion.
The last time I checked, Smack's access controls operated as
subject-verb-object triple, which should map nicely to the
"pid"/"comm" and "opid"/"ocomm" fields; the former pair associated
with the subject, the latter pair associated with the object. That
combined with the "fn" field should give you all of the information
relevant to the access control decision. If you feel that is not the
case, perhaps that is an indicator that the information used in the
access control decision is wrong, or there is a problem with the
implementation.
--
paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2022-02-02 15:20 ` Paul Moore
@ 2022-02-02 18:12 ` Casey Schaufler
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p7>
1 sibling, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2022-02-02 18:12 UTC (permalink / raw)
To: Paul Moore, vishal.goel; +Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
On 2/2/2022 7:20 AM, Paul Moore wrote:
> On Wed, Feb 2, 2022 at 5:38 AM Vishal Goel <vishal.goel@samsung.com> 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>
>>>>>> Does anyone from the audit side object to my taking this
>>>>>> in the Smack tree?
>>>>> The audit subsystem already has the "opid" and "ocomm" fields for
>>>>> reporting on the object task info and this is even available in
>>>>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
>>>>> that can't be used instead?
>>>> That info is not sufficient for debugging smack issues in ptrace calls.
>>>> Tracee information is not printed in the logs. For eg. in below log-
>>>> [ 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"
>>>>
>>>> There is no information of the tracee process.
>>>> So to debug such ptrace issues, both tracer and tracee information is needed.
>>>> That's why added new type to print both info specifically for ptrace scenarios.
>>
>>> From what I saw you are trying to record information about the tracer
>>> and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
>>> should be used instead of adding new fields.
>> Actually in smack_ptrace_access_check() function, tracer process is current process.
>> While some other process is object process(tracee).
>> But in case of smack_ptrace_traceme() function, tracer process is parent process.
>> While current process is object process(tracee). So in this case, both pid/comm
>> and opid/ocomm will print current process info only i.e tracess process.
>> So tracer process info is not getting printed.
>> Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
>> And current process is tracee process.
>> So that's why we need to print separately tracer and tracee process info
>> without any confusion.
> The last time I checked, Smack's access controls operated as
> subject-verb-object triple, which should map nicely to the
> "pid"/"comm" and "opid"/"ocomm" fields; the former pair associated
> with the subject, the latter pair associated with the object. That
> combined with the "fn" field should give you all of the information
> relevant to the access control decision. If you feel that is not the
> case, perhaps that is an indicator that the information used in the
> access control decision is wrong, or there is a problem with the
> implementation.
Sorry that I've been absent from this discussion. Paul's right.
There's a whole lot of unnecessary complexity in this patch.
If you change the 2nd parameter of smk_ptrace_rule_check() to
be the task_struct of the tracee you should be able to do
exactly as Paul suggests.
>
> --
> paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p7>
@ 2022-02-03 13:16 ` Vishal Goel
2022-02-03 18:09 ` Casey Schaufler
0 siblings, 1 reply; 19+ messages in thread
From: Vishal Goel @ 2022-02-03 13:16 UTC (permalink / raw)
To: Casey Schaufler, Paul Moore; +Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
>>>>>>>> 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>
>>>>>>> Does anyone from the audit side object to my taking this
>>>>>>> in the Smack tree?
>>>>>> The audit subsystem already has the "opid" and "ocomm" fields for
>>>>>> reporting on the object task info and this is even available in
>>>>>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
>>>>>> that can't be used instead?
>>>>> That info is not sufficient for debugging smack issues in ptrace calls.
>>>>> Tracee information is not printed in the logs. For eg. in below log-
>>>>> [ 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"
>>>>>
>>>>> There is no information of the tracee process.
>>>>> So to debug such ptrace issues, both tracer and tracee information is needed.
>>>>> That's why added new type to print both info specifically for ptrace scenarios.
>>>
>>>> From what I saw you are trying to record information about the tracer
>>>> and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
>>>> should be used instead of adding new fields.
>>> Actually in smack_ptrace_access_check() function, tracer process is current process.
>>> While some other process is object process(tracee).
>>> But in case of smack_ptrace_traceme() function, tracer process is parent process.
>>> While current process is object process(tracee). So in this case, both pid/comm
>>> and opid/ocomm will print current process info only i.e tracess process.
>>> So tracer process info is not getting printed.
>>> Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
>>> And current process is tracee process.
>>> So that's why we need to print separately tracer and tracee process info
>>> without any confusion.
>> The last time I checked, Smack's access controls operated as
>> subject-verb-object triple, which should map nicely to the
>> "pid"/"comm" and "opid"/"ocomm" fields; the former pair associated
>> with the subject, the latter pair associated with the object. That
>> combined with the "fn" field should give you all of the information
>> relevant to the access control decision. If you feel that is not the
>> case, perhaps that is an indicator that the information used in the
>> access control decision is wrong, or there is a problem with the
>> implementation.
> Sorry that I've been absent from this discussion. Paul's right.
> There's a whole lot of unnecessary complexity in this patch.
> If you change the 2nd parameter of smk_ptrace_rule_check() to
> be the task_struct of the tracee you should be able to do
> exactly as Paul suggests.
Yes,right. We can map the "opid/ocomm" pair to object/tracee process.
But issue is to print subject info. In smack, subject can be current process
or it can be parent of current process in some api.
For eg. in smack_ptrace_access_check() function, subject/tracer process is current process.
While in case of smack_ptrace_traceme() function, subject/tracer process is parent process.
But as you know, that "pid/comm" pair always maps to current process according to below code:-
static void dump_common_audit_data(struct audit_buffer *ab,
struct common_audit_data *a)
{
audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm)));
So in some case, it prints correct info where current process is subject.
But what about smack_ptrace_traceme() api, where subject is parent process not current process?
So wrong subject info is getting printed currently.
How can we map "pid/comm" to parent process which is actual subject in smack_ptrace_traceme() api case?
--------- Original Message ---------
Sender : Casey Schaufler <casey@schaufler-ca.com>
Date : 2022-02-03 03:13 (GMT+9)
Title : Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
On 2/2/2022 7:20 AM, Paul Moore wrote:
> On Wed, Feb 2, 2022 at 5:38 AM Vishal Goel <vishal.goel@samsung.com> 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>
>>>>>> Does anyone from the audit side object to my taking this
>>>>>> in the Smack tree?
>>>>> The audit subsystem already has the "opid" and "ocomm" fields for
>>>>> reporting on the object task info and this is even available in
>>>>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
>>>>> that can't be used instead?
>>>> That info is not sufficient for debugging smack issues in ptrace calls.
>>>> Tracee information is not printed in the logs. For eg. in below log-
>>>> [ 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"
>>>>
>>>> There is no information of the tracee process.
>>>> So to debug such ptrace issues, both tracer and tracee information is needed.
>>>> That's why added new type to print both info specifically for ptrace scenarios.
>>
>>> From what I saw you are trying to record information about the tracer
>>> and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
>>> should be used instead of adding new fields.
>> Actually in smack_ptrace_access_check() function, tracer process is current process.
>> While some other process is object process(tracee).
>> But in case of smack_ptrace_traceme() function, tracer process is parent process.
>> While current process is object process(tracee). So in this case, both pid/comm
>> and opid/ocomm will print current process info only i.e tracess process.
>> So tracer process info is not getting printed.
>> Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
>> And current process is tracee process.
>> So that's why we need to print separately tracer and tracee process info
>> without any confusion.
> The last time I checked, Smack's access controls operated as
> subject-verb-object triple, which should map nicely to the
> "pid"/"comm" and "opid"/"ocomm" fields; the former pair associated
> with the subject, the latter pair associated with the object. That
> combined with the "fn" field should give you all of the information
> relevant to the access control decision. If you feel that is not the
> case, perhaps that is an indicator that the information used in the
> access control decision is wrong, or there is a problem with the
> implementation.
Sorry that I've been absent from this discussion. Paul's right.
There's a whole lot of unnecessary complexity in this patch.
If you change the 2nd parameter of smk_ptrace_rule_check() to
be the task_struct of the tracee you should be able to do
exactly as Paul suggests.
>
> --
> paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
2022-02-03 13:16 ` Vishal Goel
@ 2022-02-03 18:09 ` Casey Schaufler
0 siblings, 0 replies; 19+ messages in thread
From: Casey Schaufler @ 2022-02-03 18:09 UTC (permalink / raw)
To: vishal.goel, Paul Moore; +Cc: Vaneet Narang, AMIT SAHRAWAT, linux-audit
On 2/3/2022 5:16 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>
>>>>>>>> Does anyone from the audit side object to my taking this
>>>>>>>> in the Smack tree?
>>>>>>> The audit subsystem already has the "opid" and "ocomm" fields for
>>>>>>> reporting on the object task info and this is even available in
>>>>>>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
>>>>>>> that can't be used instead?
>>>>>> That info is not sufficient for debugging smack issues in ptrace calls.
>>>>>> Tracee information is not printed in the logs. For eg. in below log-
>>>>>> [ 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"
>>>>>>
>>>>>> There is no information of the tracee process.
>>>>>> So to debug such ptrace issues, both tracer and tracee information is needed.
>>>>>> That's why added new type to print both info specifically for ptrace scenarios.
>>>>> From what I saw you are trying to record information about the tracer
>>>>> and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
>>>>> should be used instead of adding new fields.
>>>> Actually in smack_ptrace_access_check() function, tracer process is current process.
>>>> While some other process is object process(tracee).
>>>> But in case of smack_ptrace_traceme() function, tracer process is parent process.
>>>> While current process is object process(tracee). So in this case, both pid/comm
>>>> and opid/ocomm will print current process info only i.e tracess process.
>>>> So tracer process info is not getting printed.
>>>> Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
>>>> And current process is tracee process.
>>>> So that's why we need to print separately tracer and tracee process info
>>>> without any confusion.
>>> The last time I checked, Smack's access controls operated as
>>> subject-verb-object triple, which should map nicely to the
>>> "pid"/"comm" and "opid"/"ocomm" fields; the former pair associated
>>> with the subject, the latter pair associated with the object. That
>>> combined with the "fn" field should give you all of the information
>>> relevant to the access control decision. If you feel that is not the
>>> case, perhaps that is an indicator that the information used in the
>>> access control decision is wrong, or there is a problem with the
>>> implementation.
>
>> Sorry that I've been absent from this discussion. Paul's right.
>> There's a whole lot of unnecessary complexity in this patch.
>> If you change the 2nd parameter of smk_ptrace_rule_check() to
>> be the task_struct of the tracee you should be able to do
>> exactly as Paul suggests.
> Yes,right. We can map the "opid/ocomm" pair to object/tracee process.
> But issue is to print subject info. In smack, subject can be current process
> or it can be parent of current process in some api.
> For eg. in smack_ptrace_access_check() function, subject/tracer process is current process.
> While in case of smack_ptrace_traceme() function, subject/tracer process is parent process.
> But as you know, that "pid/comm" pair always maps to current process according to below code:-
>
> static void dump_common_audit_data(struct audit_buffer *ab,
> struct common_audit_data *a)
> {
> audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
> audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm)));
>
> So in some case, it prints correct info where current process is subject.
> But what about smack_ptrace_traceme() api, where subject is parent process not current process?
> So wrong subject info is getting printed currently.
> How can we map "pid/comm" to parent process which is actual subject in smack_ptrace_traceme() api case?
I'm not especially happy looking at the code as it is today.
Common code paths and helpers only work when you have commonality.
There have been changes made to the ptrace hooks that didn't take
these important differences into account. It looks like a rewrite
of smack_ptrace_traceme() that doesn't use the common path functions
is going to be necessary. Unfortunately, it's not going to be
elegant.
>
>
>
> --------- Original Message ---------
> Sender : Casey Schaufler <casey@schaufler-ca.com>
> Date : 2022-02-03 03:13 (GMT+9)
> Title : Re: [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs
>
> On 2/2/2022 7:20 AM, Paul Moore wrote:
>> On Wed, Feb 2, 2022 at 5:38 AM Vishal Goel <vishal.goel@samsung.com> 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>
>>>>>>> Does anyone from the audit side object to my taking this
>>>>>>> in the Smack tree?
>>>>>> The audit subsystem already has the "opid" and "ocomm" fields for
>>>>>> reporting on the object task info and this is even available in
>>>>>> dump_common_audit_data() via LSM_AUDIT_DATA_TASK; is there a reason
>>>>>> that can't be used instead?
>>>>> That info is not sufficient for debugging smack issues in ptrace calls.
>>>>> Tracee information is not printed in the logs. For eg. in below log-
>>>>> [ 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"
>>>>>
>>>>> There is no information of the tracee process.
>>>>> So to debug such ptrace issues, both tracer and tracee information is needed.
>>>>> That's why added new type to print both info specifically for ptrace scenarios.
>>>> From what I saw you are trying to record information about the tracer
>>>> and the tracee, yes? The "pid", "comm", "opid", and "ocomm" fields
>>>> should be used instead of adding new fields.
>>> Actually in smack_ptrace_access_check() function, tracer process is current process.
>>> While some other process is object process(tracee).
>>> But in case of smack_ptrace_traceme() function, tracer process is parent process.
>>> While current process is object process(tracee). So in this case, both pid/comm
>>> and opid/ocomm will print current process info only i.e tracess process.
>>> So tracer process info is not getting printed.
>>> Similarly for smack_bprm_creds_for_exec(), tracer process is parent process.
>>> And current process is tracee process.
>>> So that's why we need to print separately tracer and tracee process info
>>> without any confusion.
>> The last time I checked, Smack's access controls operated as
>> subject-verb-object triple, which should map nicely to the
>> "pid"/"comm" and "opid"/"ocomm" fields; the former pair associated
>> with the subject, the latter pair associated with the object. That
>> combined with the "fn" field should give you all of the information
>> relevant to the access control decision. If you feel that is not the
>> case, perhaps that is an indicator that the information used in the
>> access control decision is wrong, or there is a problem with the
>> implementation.
>
> Sorry that I've been absent from this discussion. Paul's right.
> There's a whole lot of unnecessary complexity in this patch.
> If you change the 2nd parameter of smk_ptrace_rule_check() to
> be the task_struct of the tracee you should be able to do
> exactly as Paul suggests.
>
>> --
>> paul-moore.com
>
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-02-03 18:09 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcas5p3.samsung.com>
2021-12-20 10:13 ` [PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs Vishal Goel
2021-12-20 16:41 ` Casey Schaufler
2021-12-20 16:41 ` Casey Schaufler
2021-12-21 1:19 ` Casey Schaufler
2021-12-21 1:19 ` Casey Schaufler
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p2>
2021-12-21 13:12 ` Vishal Goel
2021-12-21 13:12 ` Vishal Goel
2021-12-21 17:01 ` Casey Schaufler
2021-12-21 17:01 ` Casey Schaufler
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p6>
2021-12-24 5:04 ` Vishal Goel
2022-01-28 16:24 ` Casey Schaufler
2022-01-28 18:00 ` Paul Moore
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p8>
2022-02-01 7:54 ` Vishal Goel
2022-02-01 14:05 ` Paul Moore
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p4>
2022-02-02 10:37 ` Vishal Goel
2022-02-02 15:20 ` Paul Moore
2022-02-02 18:12 ` Casey Schaufler
[not found] ` <CGME20211220101352epcas5p3aec72d06d04f71a7c387570957a0f6c7@epcms5p7>
2022-02-03 13:16 ` Vishal Goel
2022-02-03 18:09 ` Casey Schaufler
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.