On Wed, May 11, 2022 at 3:09 PM James Carter <jwcart2@xxxxxxxxx> wrote: > > On Sun, May 8, 2022 at 3:19 PM Christian Göttsche > <cgzones@xxxxxxxxxxxxxx> wrote: > > > > 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@xxxxxxxxxxxxxx> > > Works as advertised. > > Acked-by: James Carter <jwcart2@xxxxxxxxx> > Merged. Thanks, Jim > > --- > > 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 > >
