Proposing SELinux support in strace

[Dmitry V. Levin]

On Fri, Mar 26, 2021 at 10:16:27AM +0100, Renaud Métrich wrote:
> Thanks, fixed. I additionally handled the buffer too small case, just in 
> case ...
> 
> I didn't merge de commits yet, to keep track of changes, will do once 
> ready to merge.

Thanks.  There are several style issues, see below.

[...]
diff --git a/tests/access--secontext.c b/tests/access--secontext.c
new file mode 100644
index 000000000..01fd12e35
--- /dev/null
+++ b/tests/access--secontext.c
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2021 The strace developers.
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "tests.h"
+
+#define TEST_SECONTEXT
+
+#ifndef HAVE_SELINUX_RUNTIME
+
+SKIP_MAIN_UNDEFINED("HAVE_SELINUX_RUNTIME")
+
+#else
+
+#include "access.c"
+
+#endif

There are several minor style issues here (and in other similar files),
I'd prefer if it was written this way:

#include "tests.h"

#ifdef HAVE_SELINUX_RUNTIME

# define TEST_SECONTEXT
# include "access.c"

#else

SKIP_MAIN_UNDEFINED("HAVE_SELINUX_RUNTIME")

#endif

[...]
diff --git a/tests/dirfd.c b/tests/dirfd.c
new file mode 100644
index 000000000..517ddea0c
--- /dev/null
+++ b/tests/dirfd.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2021 The strace developers.
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "tests.h"
+
+#include <limits.h>
+#include <unistd.h>
+#include <sys/types.h>

I don't think we need to include <sys/types.h> explicitly here.
Note that in man-pages-5.11 a lot of redundant <sys/types.h> were removed.

+#include <dirent.h>
+
+#include "xmalloc.h"
+
+int
+get_dir_fd(const char *dir_path)
+{
+	DIR *dir = NULL;
+	dir = opendir(dir_path);

Let's merge the last two lines.

+	if (dir == NULL)
+		perror_msg_and_fail("opendir");

Let's mention dir_path in the diagnostics.

+	int dfd = dirfd(dir);
+	if (dfd == -1)
+		perror_msg_and_fail("dirfd");
+	return dfd;
+}
+
+int
+get_curdir_fd(char **curdir)
+{
+	int res = get_dir_fd(".");
+
+	if (curdir != NULL) {
+		char *buf = xmalloc(PATH_MAX);
+		ssize_t n = readlink("/proc/self/cwd", buf, PATH_MAX);
+		if (n == -1)
+			perror_msg_and_skip("/proc is not available");

Let's change the last line to
			perror_msg_and_skip("readlink: %s", "/proc/self/cwd");

+		if (n >= PATH_MAX)
+			perror_msg_and_fail("buffer is too small");

There is no errno defined in this case so perror_* is not suitable,
let's change the last line to
			error_msg_and_fail("readlink: %s: %s",
					   "/proc/self/cwd",
					   "symlink value is too long");

[...]
diff --git a/tests/execve--secontext.c.old b/tests/execve--secontext.c.old
new file mode 100644
index 000000000..64c34c7b9
--- /dev/null
+++ b/tests/execve--secontext.c.old
@@ -0,0 +1,42 @@

This looks as a something unintended.

diff --git a/tests/execve--secontext.test b/tests/execve--secontext.test
new file mode 100755
index 000000000..14c9f33de
--- /dev/null
+++ b/tests/execve--secontext.test
@@ -0,0 +1,18 @@
+#!/bin/sh
+#
+# Check execve syscall decoding.
+#
+# Copyright (c) 2021 The strace developers.
+# All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+. "${srcdir=.}/init.sh"
+
+check_prog grep
+run_prog > /dev/null
+run_strace --secontext -eexecve $args > "$EXP"
+
+# Filter out execve() call made by strace.
+grep -F test.execve < "$LOG" > "$OUT"
+match_diff "$OUT" "$EXP"

This looks very similar to tests/execve.test, could you parametrize it
instead, see e.g. tests/ioctl.test as an example.

diff --git a/tests/execve--secontext_full.test b/tests/execve--secontext_full.test
new file mode 100755
index 000000000..4f5ac8c54
--- /dev/null
+++ b/tests/execve--secontext_full.test
@@ -0,0 +1,18 @@
+#!/bin/sh
+#
+# Check execve syscall decoding.
+#
+# Copyright (c) 2021 The strace developers.
+# All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+. "${srcdir=.}/init.sh"
+
+check_prog grep
+run_prog > /dev/null
+run_strace --secontext=full -eexecve $args > "$EXP"
+
+# Filter out execve() call made by strace.
+grep -F test.execve < "$LOG" > "$OUT"
+match_diff "$OUT" "$EXP"

Likewise.


-- 
ldv