From 3f0e5340b651da98251a58cc7923525d69f96032 Mon Sep 17 00:00:00 2001

From: Eugene Syromyatnikov <evgsyr@gmail.com>

Date: Fri, 1 Jul 2022 10:45:48 +0200

Subject: [PATCH] secontext: fix expected SELinux context check for unlinked

 FDs

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

selinux_getfdcon open-coded a part of getfdpath_pid since it tries

to do the same job, figure out a path associated with an FD, for slightly

different purpose: to get the expected SELinux context for it.  As the previous

commit shows, it's a bit more complicated in cases when the path ends

with the " (deleted)" string, which is also used for designated unlinked paths

in procfs.  Otherwise, it may manifest in test failures such as this:

     [unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023] fchmod(4</root/rpmbuild/BUILD/strace-5.13/tests/fchmod-y--secontext_full_mismatch.dir/fchmod_subdir/fchmod_sample_file> [unconfined_u:object_r:admin_home_t:s0!!system_u:object_r:admin_home_t:s0], 0600) = 0

    -[unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023] fchmod(4</root/rpmbuild/BUILD/strace-5.13/tests/fchmod-y--secontext_full_mismatch.dir/fchmod_subdir/fchmod_sample_file (deleted)> [unconfined_u:object_r:admin_home_t:s0!!system_u:object_r:admin_home_t:s0], 051) = 0

    -[unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023] fchmod(4</root/rpmbuild/BUILD/strace-5.13/tests/fchmod-y--secontext_full_mismatch.dir/fchmod_subdir/fchmod_sample_file (deleted)> [unconfined_u:object_r:admin_home_t:s0!!system_u:object_r:admin_home_t:s0], 004) = 0

    +[unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023] fchmod(4</root/rpmbuild/BUILD/strace-5.13/tests/fchmod-y--secontext_full_mismatch.dir/fchmod_subdir/fchmod_sample_file (deleted)> [unconfined_u:object_r:admin_home_t:s0], 051) = 0

    +[unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023] fchmod(4</root/rpmbuild/BUILD/strace-5.13/tests/fchmod-y--secontext_full_mismatch.dir/fchmod_subdir/fchmod_sample_file (deleted)> [unconfined_u:object_r:admin_home_t:s0], 004) = 0

     +++ exited with 0 +++

    + fail_ '../../src/strace -a15 -y --secontext=full,mismatch -e trace=fchmod ../fchmod-y--secontext_full_mismatch output mismatch'

    + warn_ 'fchmod-y--secontext_full_mismatch.gen.test: failed test: ../../src/strace -a15 -y --secontext=full,mismatch -e trace=fchmod ../fchmod-y--secontext_full_mismatch output mismatch'

    + printf '%s\n' 'fchmod-y--secontext_full_mismatch.gen.test: failed test: ../../src/strace -a15 -y --secontext=full,mismatch -e trace=fchmod ../fchmod-y--secontext_full_mismatch output mismatch'

    fchmod-y--secontext_full_mismatch.gen.test: failed test: ../../src/strace -a15 -y --secontext=full,mismatch -e trace=fchmod ../fchmod-y--secontext_full_mismatch output mismatch

    + exit 1

    FAIL fchmod-y--secontext_full_mismatch.gen.test (exit status: 1)

that happens due to the fact that the get_expected_filecontext() call

is made against the path with the " (deleted)" part, which is wrong (it

is more wrong than shown above when a file with the path that ends with

" (deleted)" exists).  Moreover, it would be incorrect to call stat()

on that path.

Let's factor out the common part of the code and simply call it

from selinux_getfdcon, then use the st_mode from the procfs link.

* src/defs.h (get_proc_pid_fd_path): New declaration.

* src/pathtrace.c (get)proc_pid_fd_path): New function, part

of getfdpath_pid that performs link resolution and processing

of the result.

(getfdpath_pid): Call get_proc_pid_fd_path after PID resolution.

* src/secontext.c (get_expected_filecontext): Add mode parameter, use

it in selabel_lookup call instead of retrieveing file mode using stat()

if it is not -1.

(selinux_getfdcon): Call get_proc_pid_fd_path instead

of open-coding path resolution code, call stat() on the procfs link

and pass the retrieved st_mode to the get_expected_filecontext call.

(selinux_getfilecon): Pass -1 as mode in the get_expected_filecontext

call.

Reported-by: Václav Kadlčík <vkadlcik@redhat.com>

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2087693

---

 src/defs.h      | 15 +++++++++++++++

 src/pathtrace.c | 26 ++++++++++++++++++--------

 src/secontext.c | 35 +++++++++++++++++++++--------------

 3 files changed, 54 insertions(+), 22 deletions(-)

Index: strace-5.18/src/defs.h

===================================================================

--- strace-5.18.orig/src/defs.h	2022-07-12 18:22:01.563254140 +0200

+++ strace-5.18/src/defs.h	2022-07-12 18:22:06.202199392 +0200

@@ -785,6 +785,21 @@

 	return pathtrace_match_set(tcp, &global_path_set);

 }

+/**

+ * Resolves a path for a fd procfs PID proc_pid (the one got from

+ * get_proc_pid()).

+ *

+ * @param proc_pid PID number in /proc, obtained with get_proc_pid().

+ * @param fd       FD to resolve path for.

+ * @param buf      Buffer to store the resolved path in.

+ * @param bufsize  The size of buf.

+ * @param deleted  If non-NULL, set to true if the path associated with the FD

+ *                 seems to have been unlinked and to false otherwise.

+ * @return         Number of bytes written including terminating '\0'.

+ */

+extern int get_proc_pid_fd_path(int proc_pid, int fd, char *buf,

+				unsigned bufsize, bool *deleted);

+

 extern int getfdpath_pid(pid_t pid, int fd, char *buf, unsigned bufsize,

 			 bool *deleted);

Index: strace-5.18/src/pathtrace.c

===================================================================

--- strace-5.18.orig/src/pathtrace.c	2022-07-12 18:22:01.532254506 +0200

+++ strace-5.18/src/pathtrace.c	2022-07-12 18:22:06.202199392 +0200

@@ -77,11 +77,9 @@

 	set->paths_selected[set->num_selected++] = path;

 }

-/*

- * Get path associated with fd of a process with pid.

- */

 int

-getfdpath_pid(pid_t pid, int fd, char *buf, unsigned bufsize, bool *deleted)

+get_proc_pid_fd_path(int proc_pid, int fd, char *buf, unsigned bufsize,

+		     bool *deleted)

 {

 	char linkpath[sizeof("/proc/%u/fd/%u") + 2 * sizeof(int)*3];

 	ssize_t n;

@@ -89,10 +87,6 @@

 	if (fd < 0)

 		return -1;

-	int proc_pid = get_proc_pid(pid);

-	if (!proc_pid)

-		return -1;

-

 	xsprintf(linkpath, "/proc/%u/fd/%u", proc_pid, fd);

 	n = readlink(linkpath, buf, bufsize - 1);

 	if (n < 0)

@@ -143,6 +137,22 @@

 }

 /*

+ * Get path associated with fd of a process with pid.

+ */

+int

+getfdpath_pid(pid_t pid, int fd, char *buf, unsigned bufsize, bool *deleted)

+{

+	if (fd < 0)

+		return -1;

+

+	int proc_pid = get_proc_pid(pid);

+	if (!proc_pid)

+		return -1;

+

+	return get_proc_pid_fd_path(proc_pid, fd, buf, bufsize, deleted);

+}

+

+/*

  * Add a path to the set we're tracing.  Also add the canonicalized

  * version of the path.  Specifying NULL will delete all paths.

  */

Index: strace-5.18/src/secontext.c

===================================================================

--- strace-5.18.orig/src/secontext.c	2022-07-12 18:22:01.564254128 +0200

+++ strace-5.18/src/secontext.c	2022-07-12 18:22:06.203199380 +0200

@@ -62,7 +62,7 @@

 }

 static int

-get_expected_filecontext(const char *path, char **secontext)

+get_expected_filecontext(const char *path, char **secontext, int mode)

 {

 	static struct selabel_handle *hdl;

@@ -80,12 +80,7 @@

 		}

 	}

-	strace_stat_t stb;

-	if (stat_file(path, &stb) < 0) {

-		return -1;

-	}

-

-	return selabel_lookup(hdl, secontext, path, stb.st_mode);

+	return selabel_lookup(hdl, secontext, path, mode);

 }

 /*

@@ -130,16 +125,22 @@

 	/*

 	 * We need to resolve the path, because selabel_lookup() doesn't

-	 * resolve anything.  Using readlink() is sufficient here.

+	 * resolve anything.

 	 */

+	char buf[PATH_MAX + 1];

+	ssize_t n = get_proc_pid_fd_path(proc_pid, fd, buf, sizeof(buf), NULL);

+	if ((size_t) n >= (sizeof(buf) - 1))

+		return 0;

-	char buf[PATH_MAX];

-	ssize_t n = readlink(linkpath, buf, sizeof(buf));

-	if ((size_t) n >= sizeof(buf))

+	/*

+	 * We retrieve stat() here since the path the procfs link resolves into

+	 * may be reused by a different file with different context.

+	 */

+	strace_stat_t st;

+	if (stat_file(linkpath, &st))

 		return 0;

-	buf[n] = '\0';

-	get_expected_filecontext(buf, expected);

+	get_expected_filecontext(buf, expected, st.st_mode);

 	return 0;

 }

@@ -190,7 +191,13 @@

 	if (!resolved)

 		return 0;

-	get_expected_filecontext(resolved, expected);

+	strace_stat_t st;

+	if (stat_file(resolved, &st) < 0)

+		goto out;

+

+	get_expected_filecontext(resolved, expected, st.st_mode);

+

+out:

 	free(resolved);

 	return 0;