| Message ID | 20220505174420.24537-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a782abf226a3 |
| Headers | show |
| Series | [RFC] libselinux: emulate O_PATH support in fgetfilecon/fsetfilecon | expand |
On Sun, May 8, 2022 at 3:19 PM Christian Göttsche <cgzones@googlemail.com> 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@googlemail.com> Works as advertised. Acked-by: James Carter <jwcart2@gmail.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 >
On Wed, May 11, 2022 at 3:09 PM James Carter <jwcart2@gmail.com> wrote: > > On Sun, May 8, 2022 at 3:19 PM Christian Göttsche > <cgzones@googlemail.com> 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@googlemail.com> > > Works as advertised. > > Acked-by: James Carter <jwcart2@gmail.com> > 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 > >
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;
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(-)