Proposing SELinux support in strace

[Dmitry V. Levin]

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 +++");

-- 
ldv