[PATCH 1/1] Smack:- Fix the issue of wrong info printed in ptrace error logs

[Casey Schaufler]

On 2/2/2022 7:20 AM, Paul Moore wrote:
> On Wed, Feb 2, 2022 at 5:38 AM Vishal Goel <vishal.goel at 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 at 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