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.g...@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 <ca...@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.g...@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.g...@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
