<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> Hello,<o:p></o:p> <o:p> </o:p> We were experiencing the same issue w/r/t group permissions not being respected<o:p></o:p> on directories.<o:p></o:p> <o:p> </o:p> I did a quick-and-dirty hack to add the SECBIT_NO_SETUID_FIXUP securebit, as<o:p></o:p> suggested by Chirantan and Vivek, and that seems to have done the trick. Looks<o:p></o:p> like if we set the securebits before we setup sandbox we can avoid leaving the<o:p></o:p> process with CAP_SETPCAP.<o:p></o:p> <o:p> </o:p> Seems a better approach would just to expose the ability to set securebits at<o:p></o:p> the CLI like what is done with caps. Then anybody affected by this bug can set<o:p></o:p> SECBIT_NO_SETUID_FIXUP themselves. I'd prefer this if the maintainers agree.<o:p></o:p> <o:p> </o:p> Would be happy to clean up my patch and submit it--just want to check if this<o:p></o:p> is an approach y'all are content with.<o:p></o:p> <o:p> </o:p> Current patch is included if anybody wants to try it out. Note that the<o:p></o:p> securebits header is lifted from kernel 5.4.0 /usr/include/linux/securebits.h<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Ameer Ghani<o:p></o:p> <o:p> </o:p> ---<o:p></o:p> include/standard-headers/linux/securebits.h | 61 +++++++++++++++++++++<o:p></o:p> tools/virtiofsd/passthrough_ll.c | 15 +++++<o:p></o:p> 2 files changed, 76 insertions(+)<o:p></o:p> create mode 100644 include/standard-headers/linux/securebits.h<o:p></o:p> <o:p> </o:p> diff --git a/include/standard-headers/linux/securebits.h b/include/standard-headers/linux/securebits.h<o:p></o:p> new file mode 100644<o:p></o:p> index 0000000000..69c11a49c5<o:p></o:p> --- /dev/null<o:p></o:p> +++ b/include/standard-headers/linux/securebits.h<o:p></o:p> @@ -0,0 +1,61 @@<o:p></o:p> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */<o:p></o:p> +#ifndef _LINUX_SECUREBITS_H<o:p></o:p> +#define _LINUX_SECUREBITS_H<o:p></o:p> +<o:p></o:p> +/* Each securesetting is implemented using two bits. One bit specifies<o:p></o:p> + whether the setting is on or off. The other bit specify whether the<o:p></o:p> + setting is locked or not. A setting which is locked cannot be<o:p></o:p> + changed from user-level. */<o:p></o:p> +#define issecure_mask(X) (1 << (X))<o:p></o:p> +<o:p></o:p> +#define SECUREBITS_DEFAULT 0x00000000<o:p></o:p> +<o:p></o:p> +/* When set UID 0 has no special privileges. When unset, we support<o:p></o:p> + inheritance of root-permissions and suid-root executable under<o:p></o:p> + compatibility mode. We raise the effective and inheritable bitmasks<o:p></o:p> + *of the executable file* if the effective uid of the new process is<o:p></o:p> + 0. If the real uid is 0, we raise the effective (legacy) bit of the<o:p></o:p> + executable file. */<o:p></o:p> +#define SECURE_NOROOT 0<o:p></o:p> +#define SECURE_NOROOT_LOCKED 1 /* make bit-0 immutable */<o:p></o:p> +<o:p></o:p> +#define SECBIT_NOROOT (issecure_mask(SECURE_NOROOT))<o:p></o:p> +#define SECBIT_NOROOT_LOCKED (issecure_mask(SECURE_NOROOT_LOCKED))<o:p></o:p> +<o:p></o:p> +/* When set, setuid to/from uid 0 does not trigger capability-"fixup".<o:p></o:p> + When unset, to provide compatiblility with old programs relying on<o:p></o:p> + set*uid to gain/lose privilege, transitions to/from uid 0 cause<o:p></o:p> + capabilities to be gained/lost. */<o:p></o:p> +#define SECURE_NO_SETUID_FIXUP 2<o:p></o:p> +#define SECURE_NO_SETUID_FIXUP_LOCKED 3 /* make bit-2 immutable */<o:p></o:p> +<o:p></o:p> +#define SECBIT_NO_SETUID_FIXUP (issecure_mask(SECURE_NO_SETUID_FIXUP))<o:p></o:p> +#define SECBIT_NO_SETUID_FIXUP_LOCKED \<o:p></o:p> + (issecure_mask(SECURE_NO_SETUID_FIXUP_LOCKED))<o:p></o:p> +<o:p></o:p> +/* When set, a process can retain its capabilities even after<o:p></o:p> + transitioning to a non-root user (the set-uid fixup suppressed by<o:p></o:p> + bit 2). Bit-4 is cleared when a process calls exec(); setting both<o:p></o:p> + bit 4 and 5 will create a barrier through exec that no exec()'d<o:p></o:p> + child can use this feature again. */<o:p></o:p> +#define SECURE_KEEP_CAPS 4<o:p></o:p> +#define SECURE_KEEP_CAPS_LOCKED 5 /* make bit-4 immutable */<o:p></o:p> +<o:p></o:p> +#define SECBIT_KEEP_CAPS (issecure_mask(SECURE_KEEP_CAPS))<o:p></o:p> +#define SECBIT_KEEP_CAPS_LOCKED (issecure_mask(SECURE_KEEP_CAPS_LOCKED))<o:p></o:p> +<o:p></o:p> +/* When set, a process cannot add new capabilities to its ambient set. */<o:p></o:p> +#define SECURE_NO_CAP_AMBIENT_RAISE 6<o:p></o:p> +#define SECURE_NO_CAP_AMBIENT_RAISE_LOCKED 7 /* make bit-6 immutable */<o:p></o:p> +<o:p></o:p> +#define SECBIT_NO_CAP_AMBIENT_RAISE (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))<o:p></o:p> +#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \<o:p></o:p> + (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))<o:p></o:p> +<o:p></o:p> +#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \<o:p></o:p> + issecure_mask(SECURE_NO_SETUID_FIXUP) | \<o:p></o:p> + issecure_mask(SECURE_KEEP_CAPS) | \<o:p></o:p> + issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))<o:p></o:p> +#define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)<o:p></o:p> +<o:p></o:p> +#endif /* _LINUX_SECUREBITS_H */<o:p></o:p> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c<o:p></o:p> index 49c21fd855..afec9a6567 100644<o:p></o:p> --- a/tools/virtiofsd/passthrough_ll.c<o:p></o:p> +++ b/tools/virtiofsd/passthrough_ll.c<o:p></o:p> @@ -43,6 +43,7 @@<o:p></o:p> #include "fuse_log.h"<o:p></o:p> #include "fuse_lowlevel.h"<o:p></o:p> #include "standard-headers/linux/fuse.h"<o:p></o:p> +#include "standard-headers/linux/securebits.h"<o:p></o:p> #include <cap-ng.h><o:p></o:p> #include <dirent.h><o:p></o:p> #include <pthread.h><o:p></o:p> @@ -3515,6 +3516,17 @@ static void setup_chroot(struct lo_data *lo)<o:p></o:p> }<o:p></o:p> }<o:p></o:p> <o:p></o:p> +static void setup_securebits(void)<o:p></o:p> +{<o:p></o:p> + int status;<o:p></o:p> +<o:p></o:p> + status = prctl(PR_SET_SECUREBITS, SECBIT_NO_SETUID_FIXUP);<o:p></o:p> + if (status == -1) {<o:p></o:p> + fuse_log(FUSE_LOG_ERR, "prctl(PR_SET_SECUREBITS): %m\n");<o:p></o:p> + exit(1);<o:p></o:p> + }<o:p></o:p> +}<o:p></o:p> +<o:p></o:p> /*<o:p></o:p> * Lock down this process to prevent access to other processes or files outside<o:p></o:p> * source directory. This reduces the impact of arbitrary code execution bugs.<o:p></o:p> @@ -3869,6 +3881,9 @@ int main(int argc, char *argv[])<o:p></o:p> <o:p></o:p> setup_nofile_rlimit(opts.rlimit_nofile);<o:p></o:p> <o:p></o:p> + /* Set securebits before we drop CAP_SETPCAP */<o:p></o:p> + setup_securebits();<o:p></o:p> +<o:p></o:p> /* Must be before sandbox since it wants /proc */<o:p></o:p> setup_capng();<o:p></o:p> <o:p></o:p> -- <o:p></o:p> 2.25.1<o:p></o:p> <o:p> </o:p> </div> </body> </html>