Proposing SELinux support in strace

[Renaud Métrich]

Shame on me, definitely, your idea just works fine and is robust!

Tested on chroots, bind mounts and systemd-nspawns. We get even more 
contexts since Unix sockets are now displayed as well.

I was concentrating on how I could rebuild the fs tree from mountinfo 
instead of just following symlinks in /proc as you proposed ...

So what's next now? Since contexts always display now, there is no real 
need for dedicated *--secontext test suite, all this can be integrated 
into the standard test suite (we just need to add "%s" almost everywhere 
for tests that we want to re-run with "strace --secontext").

Shall I do this or just stick to the updated test suite?

Renaud.


On 3/19/21 2:41 AM, Dmitry V. Levin wrote:
> On Fri, Mar 19, 2021 at 02:18:35AM +0300, Dmitry V. Levin wrote:
>> On Thu, Mar 18, 2021 at 04:55:13PM +0100, Renaud Métrich wrote:
>>> Hello, I've just realized that the limitation on the mount namespace is
>>> a real blocker in real world:
>>>
>>> it appears that on systemd systems, all systemd services making use of
>>> PrivateTmp get their own namespace (due to having their own /var/tmp),
>>> which prevents the --secontext option from working for all files.
>> The very minimal fix that's readily available is for printing selinux
>> context associated with descriptors.  Since it can be obtained right from
>> procfs without falling back to path names, it should not be affected by
>> any differences in mountns or rootfs between strace and its tracee.
> Assuming this works, let's see what could be done with file names:
> - if the filename starts with "/", we can construct a new name as
>    /proc/pid/root/filename and getfilecon it;
> - otherwise, if last_dirfd == AT_FDCWD, we can construct a new name as
>    /proc/pid/cwd/filename and getfilecon it;
> - otherwise, we can construct a new name as
>    /proc/pid/fd/last_dirfd/filename and getfilecon it.
>
> Could you check whether this approach would work good enough, please?
>
> diff --git a/doc/strace.1.in b/doc/strace.1.in
> index e6ad7a9c0..15d8e2531 100644
> --- a/doc/strace.1.in
> +++ b/doc/strace.1.in
> @@ -1093,11 +1093,6 @@ strace's namespace, too.
>   .if '@USE_SELINUX_FALSE@'#' .B full
>   .if '@USE_SELINUX_FALSE@'#' is specified, print the complete context (user,
>   .if '@USE_SELINUX_FALSE@'#' role, type and category) instead of just the type.
> -.if '@USE_SELINUX_FALSE@'#' The current implementation has limitations, in
> -.if '@USE_SELINUX_FALSE@'#' particular, SELinux contexts cannot be printed for
> -.if '@USE_SELINUX_FALSE@'#' file names when
> -.if '@USE_SELINUX_FALSE@'#' .B strace
> -.if '@USE_SELINUX_FALSE@'#' and the tracee belong to different mount namespaces.
>   .SS Statistics
>   .TP 12
>   .B \-c
> diff --git a/src/selinux.c b/src/selinux.c
> index 0f1edfe28..b62d40324 100644
> --- a/src/selinux.c
> +++ b/src/selinux.c
> @@ -103,52 +103,27 @@ selinux_getfilecon(struct tcb *tcp, const char *path, char **result)
>   	if (!selinux_context)
>   		return -1;
>   
> -	/*
> -	 * Current limitation: we cannot query the path if we are in different
> -	 * mount namespaces.
> -	 */
> -	if (get_mnt_ns(tcp) != get_our_mnt_ns())
> +	int proc_pid = 0;
> +	translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
> +	if (!proc_pid)
>   		return -1;
>   
> -	char *secontext;
> +	int ret = -1;
> +	char fname[PATH_MAX];
>   
>   	if (path[0] == '/')
> -		return getcontext(getfilecon(path, &secontext), &secontext, result);
> -
> -	char resolved[PATH_MAX + 1];
> -
> -	/*
> -	 * If we have a relative pathname and 'last_dirfd' == AT_FDCWD, we need
> -	 * to prepend by the CWD.
> -	 */
> -	if (tcp->last_dirfd == AT_FDCWD) {
> -		int proc_pid = 0;
> -		translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
> -		if (!proc_pid)
> -			return -1;
> -
> -		char linkpath[sizeof("/proc/%u/cwd") + sizeof(int)*3];
> -		xsprintf(linkpath, "/proc/%u/cwd", proc_pid);
> -
> -		ssize_t n = readlink(linkpath, resolved, sizeof(resolved) - 1);
> -		/*
> -		 * NB: if buffer is too small, readlink doesn't fail,
> -		 * it returns truncated result.
> -		 */
> -		if (n == sizeof(resolved) - 1)
> -			return -1;
> -		resolved[n] = '\0';
> -	} else {
> -		if (getfdpath_pid(tcp->pid, tcp->last_dirfd,
> -				  resolved, sizeof(resolved)) < 0)
> -			return -1;
> -	}
> -
> -	if (resolved[0] != '/')
> +		ret = snprintf(fname, sizeof(fname), "/proc/%u/root%s",
> +			       proc_pid, path);
> +	else if (tcp->last_dirfd == AT_FDCWD)
> +		ret = snprintf(fname, sizeof(fname), "/proc/%u/cwd/%s",
> +			       proc_pid, path);
> +	else if (tcp->last_dirfd >= 0 )
> +		ret = snprintf(fname, sizeof(fname), "/proc/%u/fd/%u/%s",
> +			       proc_pid, tcp->last_dirfd, path);
> +
> +	if ((unsigned int) ret >= sizeof(fname))
>   		return -1;
>   
> -	char pathtoresolve[2 * PATH_MAX + 2];
> -	xsprintf(pathtoresolve, "%s/%s", resolved, path);
> -
> -	return getcontext(getfilecon(pathtoresolve, &secontext), &secontext, result);
> +	char *secontext;
> +	return getcontext(getfilecon(fname, &secontext), &secontext, result);
>   }
> diff --git a/tests/fchmod--secontext.c b/tests/fchmod--secontext.c
> index 13434544c..3b6681153 100644
> --- a/tests/fchmod--secontext.c
> +++ b/tests/fchmod--secontext.c
> @@ -37,19 +37,20 @@ main(void)
>   	if (fname_realpath == NULL)
>   		perror_msg_and_fail("realpath");
>   
> +	const char *fname_context = SELINUX_FILECONTEXT(fname);
>   	long rc = syscall(__NR_fchmod, fd, 0600);
>   	printf("%sfchmod(%d<%s>%s, 0600) = %s\n",
>   	       SELINUX_MYCONTEXT(),
> -	       fd, fname_realpath, SELINUX_FILECONTEXT(fname),
> +	       fd, fname_realpath, fname_context,
>   	       sprintrc(rc));
>   
>   	if (unlink(fname))
>   		perror_msg_and_fail("unlink");
>   
>   	rc = syscall(__NR_fchmod, fd, 0600);
> -	printf("%sfchmod(%d<%s (deleted)>, 0600) = %s\n",
> +	printf("%sfchmod(%d<%s (deleted)>%s, 0600) = %s\n",
>   	       SELINUX_MYCONTEXT(),
> -	       fd, fname_realpath,
> +	       fd, fname_realpath, fname_context,
>   	       sprintrc(rc));
>   
>   	puts("+++ exited with 0 +++");
>

-------------- 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/20210319/f4d70956/attachment.bin>