[RFC PATCH] libselinux: emulate O_PATH support in fgetfilecon/fsetfilecon

* [RFC PATCH] libselinux: emulate O_PATH support in fgetfilecon/fsetfilecon
@ 2022-05-05 17:44 Christian Göttsche
  2022-05-11 18:42 ` [RFC PATCH 2/4] libselinux: restorecon: misc tweaks Christian Göttsche
  2022-05-11 19:09 ` [RFC PATCH] libselinux: emulate O_PATH support in fgetfilecon/fsetfilecon James Carter
  0 siblings, 2 replies; 14+ messages in thread
From: Christian Göttsche @ 2022-05-05 17:44 UTC (permalink / raw)
  To: selinux

Operating on a file descriptor avoids TOCTOU issues and one opened via
O_PATH avoids the requirement of having read access to the file.  Since
Linux does not natively support file descriptors opened via O_PATH in
fgetxattr(2) and at least glibc and musl does not emulate O_PATH support
in their implementations, fgetfilecon(3) and fsetfilecon(3) also do not
currently support file descriptors opened with O_PATH.

Inspired by CVE-2013-4392: https://github.com/systemd/systemd/pull/8583
Implementation adapted from: https://android.googlesource.com/platform/bionic/+/2825f10b7f61558c264231a536cf3affc0d84204%5E%21/

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/man/man3/getfilecon.3 |  4 +++-
 libselinux/man/man3/setfilecon.3 |  4 +++-
 libselinux/src/fgetfilecon.c     | 28 +++++++++++++++++++++++++---
 libselinux/src/fsetfilecon.c     | 23 ++++++++++++++++++++++-
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/libselinux/man/man3/getfilecon.3 b/libselinux/man/man3/getfilecon.3
index 5bb575b5..c3e92ba1 100644
--- a/libselinux/man/man3/getfilecon.3
+++ b/libselinux/man/man3/getfilecon.3
@@ -33,7 +33,9 @@ is identical to
 .BR getfilecon (),
 only the open file pointed to by filedes (as returned by
 .BR open (2))
-is interrogated in place of path.
+is interrogated in place of path.  Since libselinux 3.4 a file opened via
+.I O_PATH
+is supported.
 
 .BR getfilecon_raw (),
 .BR lgetfilecon_raw ()
diff --git a/libselinux/man/man3/setfilecon.3 b/libselinux/man/man3/setfilecon.3
index 0e9a383f..61f9a806 100644
--- a/libselinux/man/man3/setfilecon.3
+++ b/libselinux/man/man3/setfilecon.3
@@ -29,7 +29,9 @@ link itself has it's context set, not the file that it refers to.
 is identical to setfilecon, only the open file pointed to by filedes (as
 returned by
 .BR open (2))
-has it's context set in place of path.
+has it's context set in place of path.  Since libselinux 3.4 a file opened via
+.I O_PATH
+is supported.
 
 .BR setfilecon_raw (),
 .BR lsetfilecon_raw (),
diff --git a/libselinux/src/fgetfilecon.c b/libselinux/src/fgetfilecon.c
index 8c748f8a..baf38ec1 100644
--- a/libselinux/src/fgetfilecon.c
+++ b/libselinux/src/fgetfilecon.c
@@ -3,10 +3,32 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <stdio.h>
 #include <sys/xattr.h>
 #include "selinux_internal.h"
 #include "policy.h"
 
+static ssize_t fgetxattr_wrapper(int fd, const char *name, void *value, size_t size) {
+	char buf[40];
+	int fd_flag, saved_errno = errno;
+	ssize_t ret;
+
+	ret = fgetxattr(fd, name, value, size);
+	if (ret != -1 || errno != EBADF)
+		return ret;
+
+	/* Emulate O_PATH support */
+	fd_flag = fcntl(fd, F_GETFL);
+	if (fd_flag == -1 || (fd_flag & O_PATH) == 0) {
+		errno = EBADF;
+		return -1;
+	}
+
+	snprintf(buf, sizeof(buf), "/proc/self/fd/%d", fd);
+	errno = saved_errno;
+	return getxattr(buf, name, value, size);
+}
+
 int fgetfilecon_raw(int fd, char ** context)
 {
 	char *buf;
@@ -19,11 +41,11 @@ int fgetfilecon_raw(int fd, char ** context)
 		return -1;
 	memset(buf, 0, size);
 
-	ret = fgetxattr(fd, XATTR_NAME_SELINUX, buf, size - 1);
+	ret = fgetxattr_wrapper(fd, XATTR_NAME_SELINUX, buf, size - 1);
 	if (ret < 0 && errno == ERANGE) {
 		char *newbuf;
 
-		size = fgetxattr(fd, XATTR_NAME_SELINUX, NULL, 0);
+		size = fgetxattr_wrapper(fd, XATTR_NAME_SELINUX, NULL, 0);
 		if (size < 0)
 			goto out;
 
@@ -34,7 +56,7 @@ int fgetfilecon_raw(int fd, char ** context)
 
 		buf = newbuf;
 		memset(buf, 0, size);
-		ret = fgetxattr(fd, XATTR_NAME_SELINUX, buf, size - 1);
+		ret = fgetxattr_wrapper(fd, XATTR_NAME_SELINUX, buf, size - 1);
 	}
       out:
 	if (ret == 0) {
diff --git a/libselinux/src/fsetfilecon.c b/libselinux/src/fsetfilecon.c
index 5cf34e3f..be821c7a 100644
--- a/libselinux/src/fsetfilecon.c
+++ b/libselinux/src/fsetfilecon.c
@@ -3,13 +3,34 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <stdio.h>
 #include <sys/xattr.h>
 #include "selinux_internal.h"
 #include "policy.h"
 
+static int fsetxattr_wrapper(int fd, const char* name, const void* value, size_t size, int flags) {
+	char buf[40];
+	int rc, fd_flag, saved_errno = errno;
+
+	rc = fsetxattr(fd, name, value, size, flags);
+	if (rc == 0 || errno != EBADF)
+		return rc;
+
+	/* Emulate O_PATH support */
+	fd_flag = fcntl(fd, F_GETFL);
+	if (fd_flag == -1 || (fd_flag & O_PATH) == 0) {
+		errno = EBADF;
+		return -1;
+	}
+
+	snprintf(buf, sizeof(buf), "/proc/self/fd/%d", fd);
+	errno = saved_errno;
+	return setxattr(buf, name, value, size, flags);
+}
+
 int fsetfilecon_raw(int fd, const char * context)
 {
-	int rc = fsetxattr(fd, XATTR_NAME_SELINUX, context, strlen(context) + 1,
+	int rc = fsetxattr_wrapper(fd, XATTR_NAME_SELINUX, context, strlen(context) + 1,
 			 0);
 	if (rc < 0 && errno == ENOTSUP) {
 		char * ccontext = NULL;
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread