From f8563369bc72603d9339ad905317b55d72c8ceb9 Mon Sep 17 00:00:00 2001 From: Antonio SJ Musumeci Date: Sun, 30 Sep 2018 19:47:44 -0400 Subject: [PATCH] add security_capability option --- README.md | 7 ++++++- man/mergerfs.1 | 16 +++++++++++++++- src/config.cpp | 1 + src/config.hpp | 1 + src/getxattr.cpp | 17 ++++++++++++++++- src/listxattr.cpp | 1 + src/option_parser.cpp | 15 ++++++++++----- src/setxattr.cpp | 4 ++++ 8 files changed, 54 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 3a4b93f8..29f7d747 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ % mergerfs(1) mergerfs user manual % Antonio SJ Musumeci -% 2018-08-20 +% 2018-09-30 # NAME @@ -71,6 +71,7 @@ mergerfs does **not** support the copy-on-write (CoW) behavior found in **aufs** * **symlinkify_timeout=value**: time to wait, in seconds, to activate the **symlinkify** behavior. (default: 3600) * **nullrw=true|false**: turns reads and writes into no-ops. The request will succeed but do nothing. Useful for benchmarking mergerfs. (default: false) * **ignorepponrename=true|false**: ignore path preserving on rename. Typically rename and link act differently depending on the policy of `create` (read below). Enabling this will cause rename and link to always use the non-path preserving behavior. This means files, when renamed or linked, will stay on the same drive. (default: false) +* **security_capability=true|false**: If false return ENOATTR when xattr security.capability is queried. (default: true) * **threads=num**: number of threads to use in multithreaded mode. When set to zero (the default) it will attempt to discover and use the number of logical cores. If the lookup fails it will fall back to using 4. If the thread count is set negative it will look up the number of cores then divide by the absolute value. ie. threads=-2 on an 8 core machine will result in 8 / 2 = 4 threads. There will always be at least 1 thread. NOTE: higher number of threads increases parallelism but usually decreases throughput. (default: number of cores) *NOTE2:* the option is unavailable when built with system libfuse. * **fsname=name**: sets the name of the filesystem as seen in **mount**, **df**, etc. Defaults to a list of the source paths concatenated together with the longest common prefix removed. * **func.<func>=<policy>**: sets the specific FUSE function's policy. See below for the list of value types. Example: **func.getattr=newest** @@ -736,6 +737,10 @@ Note that this does *not* affect the inode that libfuse and the kernel use internally (also called the "nodeid"). ``` +#### I notice massive slowdowns of writes over NFS + +Due to how NFS works and interacts with FUSE when not using `direct_io` its possible that a getxattr for `security.capability` will be issued prior to any write. This will usually result in a massive slowdown for writes. Using `direct_io` will keep this from happening (and generally good to enable unless you need the features it disables) but the `security_capability` option can also help by short circuiting the call and returning `ENOATTR`. + #### It's mentioned that there are some security issues with mhddfs. What are they? How does mergerfs address them? [mhddfs](https://github.com/trapexit/mhddfs) manages running as **root** by calling [getuid()](https://github.com/trapexit/mhddfs/blob/cae96e6251dd91e2bdc24800b4a18a74044f6672/src/main.c#L319) and if it returns **0** then it will [chown](http://linux.die.net/man/1/chown) the file. Not only is that a race condition but it doesn't handle many other situations. Rather than attempting to simulate POSIX ACL behavior the proper way to manage this is to use [seteuid](http://linux.die.net/man/2/seteuid) and [setegid](http://linux.die.net/man/2/setegid), in effect becoming the user making the original call, and perform the action as them. This is what mergerfs does. diff --git a/man/mergerfs.1 b/man/mergerfs.1 index f47319ad..3f6bb3ca 100644 --- a/man/mergerfs.1 +++ b/man/mergerfs.1 @@ -1,7 +1,7 @@ .\"t .\" Automatically generated by Pandoc 1.19.2.4 .\" -.TH "mergerfs" "1" "2018\-08\-20" "mergerfs user manual" "" +.TH "mergerfs" "1" "2018\-09\-30" "mergerfs user manual" "" .hy .SH NAME .PP @@ -152,6 +152,10 @@ preserving behavior. This means files, when renamed or linked, will stay on the same drive. (default: false) .IP \[bu] 2 +\f[B]security_capability=true|false\f[]: If false return ENOATTR when +xattr security.capability is queried. +(default: true) +.IP \[bu] 2 \f[B]threads=num\f[]: number of threads to use in multithreaded mode. When set to zero (the default) it will attempt to discover and use the number of logical cores. @@ -1536,6 +1540,16 @@ Note\ that\ this\ does\ *not*\ affect\ the\ inode\ that\ libfuse and\ the\ kernel\ use\ internally\ (also\ called\ the\ "nodeid"). \f[] .fi +.SS I notice massive slowdowns of writes over NFS +.PP +Due to how NFS works and interacts with FUSE when not using +\f[C]direct_io\f[] its possible that a getxattr for +\f[C]security.capability\f[] will be issued prior to any write. +This will usually result in a massive slowdown for writes. +Using \f[C]direct_io\f[] will keep this from happening (and generally +good to enable unless you need the features it disables) but the +\f[C]security_capability\f[] option can also help by short circuiting +the call and returning \f[C]ENOATTR\f[]. .SS It\[aq]s mentioned that there are some security issues with mhddfs. What are they? How does mergerfs address them? .PP diff --git a/src/config.cpp b/src/config.cpp index 5bf1d733..a6c3e069 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -45,6 +45,7 @@ namespace mergerfs symlinkify_timeout(3600), nullrw(false), ignorepponrename(false), + security_capability(true), POLICYINIT(access), POLICYINIT(chmod), POLICYINIT(chown), diff --git a/src/config.hpp b/src/config.hpp index c8ae3daf..40c6383b 100644 --- a/src/config.hpp +++ b/src/config.hpp @@ -52,6 +52,7 @@ namespace mergerfs time_t symlinkify_timeout; bool nullrw; bool ignorepponrename; + bool security_capability; public: const Policy *policies[FuseFunc::Enum::END]; diff --git a/src/getxattr.cpp b/src/getxattr.cpp index 55187624..72760e70 100644 --- a/src/getxattr.cpp +++ b/src/getxattr.cpp @@ -1,5 +1,5 @@ /* - Copyright (c) 2016, Antonio SJ Musumeci + Copyright (c) 2018, Antonio SJ Musumeci Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above @@ -34,11 +34,20 @@ #include "ugid.hpp" #include "version.hpp" +static const char SECURITY_CAPABILITY[] = "security.capability"; + using std::string; using std::vector; using std::set; using namespace mergerfs; +static +bool +_is_attrname_security_capability(const char *attrname_) +{ + return (strcmp(attrname_,SECURITY_CAPABILITY) == 0); +} + static int _lgetxattr(const string &path, @@ -199,6 +208,8 @@ _getxattr_controlfile(const Config &config, _getxattr_controlfile_bool(config.nullrw,attrvalue); else if(attr[2] == "ignorepponrename") _getxattr_controlfile_bool(config.ignorepponrename,attrvalue); + else if(attr[2] == "security_capability") + _getxattr_controlfile_bool(config.security_capability,attrvalue); else if(attr[2] == "policies") _getxattr_controlfile_policies(config,attrvalue); else if(attr[2] == "version") @@ -334,6 +345,10 @@ namespace mergerfs const fuse_context *fc = fuse_get_context(); const Config &config = Config::get(fc); + if((config.security_capability == false) && + _is_attrname_security_capability(attrname)) + return -ENOATTR; + if(fusepath == config.controlfile) return _getxattr_controlfile(config, attrname, diff --git a/src/listxattr.cpp b/src/listxattr.cpp index 61d593de..357d46c8 100644 --- a/src/listxattr.cpp +++ b/src/listxattr.cpp @@ -51,6 +51,7 @@ _listxattr_controlfile(char *list, ("user.mergerfs.symlinkify_timeout") ("user.mergerfs.nullrw") ("user.mergerfs.ignorepponrename") + ("user.mergerfs.security_capability") ("user.mergerfs.policies") ("user.mergerfs.version") ("user.mergerfs.pid"); diff --git a/src/option_parser.cpp b/src/option_parser.cpp index 04a86eb4..bb674b0d 100644 --- a/src/option_parser.cpp +++ b/src/option_parser.cpp @@ -194,6 +194,8 @@ parse_and_process_kv_arg(Config &config, rv = parse_and_process(value,config.nullrw); else if(key == "ignorepponrename") rv = parse_and_process(value,config.ignorepponrename); + else if(key == "security_capability") + rv = parse_and_process(value,config.security_capability); } if(rv == -1) @@ -282,23 +284,26 @@ usage(void) " -o use_ino Have mergerfs generate inode values rather than\n" " autogenerated by libfuse. Suggested.\n" " -o minfreespace= minimum free space needed for certain policies.\n" - " default=4G\n" + " default = 4G\n" " -o moveonenospc= Try to move file to another drive when ENOSPC\n" - " on write. default=false\n" + " on write. default = false\n" " -o dropcacheonclose=\n" " When a file is closed suggest to OS it drop\n" " the file's cache. This is useful when direct_io\n" - " is disabled. default=false\n" + " is disabled. default = false\n" " -o symlinkify= Read-only files, after a timeout, will be turned\n" " into symlinks. Read docs for limitations and\n" - " possible issues. default=false\n" + " possible issues. default = false\n" " -o symlinkify_timeout=\n" " timeout in seconds before will turn to symlinks.\n" - " default=3600\n" + " default = 3600\n" " -o nullrw= Disables reads and writes. For benchmarking.\n" " -o ignorepponrename=\n" " Ignore path preserving when performing renames\n" " and links. default = false\n" + " -o security_capability=\n" + " When disabled return ENOATTR when the xattr\n" + " security.capability is queried. default = true\n" << std::endl; } diff --git a/src/setxattr.cpp b/src/setxattr.cpp index 4c46371b..c9dc1038 100644 --- a/src/setxattr.cpp +++ b/src/setxattr.cpp @@ -297,6 +297,10 @@ _setxattr_controlfile(Config &config, return _setxattr_bool(attrval, flags, config.ignorepponrename); + else if(attr[2] == "security_capability") + return _setxattr_bool(attrval, + flags, + config.security_capability); break; case 4: