Proposing SELinux support in strace

[Renaud Métrich]

See inline.

On 3/15/21 9:33 PM, Dmitry V. Levin wrote:
>
>>>                  AS_IF([test "x$with_secontexts$st_cv_mpers$st_cv_runtime" = xyesyesyes],
>>>                        [AC_CACHE_CHECK([whether selinux runtime works with mpers_name personality],
>>>                          [st_cv_selinux_runtime],
>>>                          [saved_CPPFLAGS="$CPPFLAGS"
>>>                           saved_LDFLAGS="$LDFLAGS"
>>>                           saved_LIBS="$LIBS"
>>>                           CPPFLAGS="$CPPFLAGS libselinux_CPPFLAGS"
>>>                           LDFLAGS="$LDFLAGS libselinux_LDFLAGS"
>>>                           LIBS="$LIBS libselinux_LIBS"
> Oops, I've made a typo, of course it should be $libselinux_CPPFLAGS,
> $libselinux_LDFLAGS, and $libselinux_LIBS.
My bad, just copy/pasted indeed ...
> [...]
>>>> diff --git a/src/access.c b/src/access.c
>>>> index 2936242f7..81b78889b 100644
>>>> --- a/src/access.c
>>>> +++ b/src/access.c
>>>> @@ -34,8 +34,10 @@ decode_faccessat(struct tcb *tcp)
>>>>    {
>>>>    	/* dirfd */
>>>>    	print_dirfd(tcp, tcp->u_arg[0]);
>>>> +#ifdef USE_SELINUX
>>>> +	tcp->dirfd = (int)(tcp->u_arg[0]);
>>>> +#endif
>>>>    	tprint_arg_next();
>>>> -
>>>>    	decode_access(tcp, 1);
>>>>    }
>>>> [...]
>>>> You've added a lot of such tcp->dirfd assignments.  Apparently,
>>>> they are added after every print_dirfd invocation except one (and that one
>>>> seems to be omitted by mistake).  If you want to cache dirfd processed by
>>>> print_dirfd, it would be much easier if you just moved this assignment
>>>> inside print_dirfd.
>> Initially I had this idea as well but then thought it was ugly to have
>> this state saved in print_dirfd().
>>
>> Additionally we need to really know when to set the dirfd: there may be
>> cases where print_dirfd() is called but the next arguments to the
>> syscall won't use that dirfd as "reference".
>>
>> Hence it's safer to assign dirfd in the syscall handler itself.
> I did "git grep -A1 'print_dirfd(' src/*.c" just to confirm that the only
> place where print_dirfd invocation is not followed by "#ifdef USE_SELINUX"
> is fsconfig where print_dirfd is used to print the last syscall argument.

So you really prefer having this in print_dirfd() even though there 
would be a side effect and we cannot guarantee how print_dirfd() will be 
used in the future?

If so, OK, I'll change this, it simplifies the code for sure.

> [...]
>>>> +	}
>>>> +	xsprintf(pathtoresolve, "%s/%s", resolved, path);
>>>> +	if (realpath(pathtoresolve, resolved) == NULL) {
>>> Interesting.  Why realpath?  Would stat_file be enough?
>> No, because the path to check needs to be absolute, or else cwd of
>> strace will be used for resolving the context, which is wrong.
> realpath is very expensive because it performs canonicalization.
> To check whether resolved/path is an absolute path, all we need is
> to check whether resolved[0] == '/'.
>
Well actually realpath() is not needed at all, getfilecon() just works 
on paths with "/some/absolute/../relative/../" as well.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20210316/efa98eeb/attachment.bin>