fuse: Only allow read/writing user xattrs
Subsystems (2)
PATCH 1/1 fuse: Only allow read/writing user xattrs
In the context of unprivileged mounts supporting anything except xattrs with the "user." prefix seems foolish. Return -EOPNOSUPP for all other types of xattrs.
---
Do we really have a kernel bug as blantant as this appears?
fs/fuse/dir.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
fs/fuse/dir.c +7 -0
@@ -13,6 +13,7 @@#include <linux/sched.h> #include <linux/namei.h> #include <linux/slab.h> +#include <linux/xattr.h> #if BITS_PER_LONG >=64 static inline void fuse_dentry_settime (struct dentry *entry,u64 time)
@@ -1537,6 +1538,9 @@ static int fuse_setxattr(struct dentry *entry, const char *name,if (fc->no_setxattr)return -EOPNOTSUPP ; +if (strncmp (name,XATTR_USER_PREFIX ,XATTR_USER_PREFIX_LEN ) !=0 ) +return -EOPNOTSUPP ; + req =fuse_get_req (fc);if (IS_ERR (req))return PTR_ERR (req);
@@ -1576,6 +1580,9 @@ static ssize_t fuse_getxattr(struct dentry *entry, const char *name,if (fc->no_getxattr)return -EOPNOTSUPP ; +if (strncmp (name,XATTR_USER_PREFIX ,XATTR_USER_PREFIX_LEN ) !=0 ) +return -EOPNOTSUPP ; + req =fuse_get_req (fc);if (IS_ERR (req))return PTR_ERR (req);
Signature · 1 line
1.7.5.4
Eric Paris
NAKNAK, this makes no sense... We are working to support SELinux
attrs on fuse, why shouldn't we?
Quoted 53 lines · Eric W. Biederman
In the context of unprivileged mounts supporting anything except xattrs with the "user." prefix seems foolish. Return
-EOPNOSUPPfor all other types of xattrs.Cc Miklos Szeredi <[email protected]>Signed-off-by "Eric W. Biederman" <[email protected]>---
Do we really have a kernel bug as blantant as this appears?
fs/fuse/dir.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)fs/fuse/dir.c +7
-1@@ -13,6 +13,7 @@#include <linux/sched.h> #include <linux/namei.h> #include <linux/slab.h> +#include <linux/xattr.h> #if BITS_PER_LONG >=64 static inline void fuse_dentry_settime (struct dentry *entry,u64 time)@@ -1537,6 +1538,9 @@ static int fuse_setxattr(struct dentry *entry, const char *name,if (fc->no_setxattr)return -EOPNOTSUPP ; +if (strncmp (name,XATTR_USER_PREFIX ,XATTR_USER_PREFIX_LEN ) !=0 ) +return -EOPNOTSUPP ; + req =fuse_get_req (fc);if (IS_ERR (req))return PTR_ERR (req);@@ -1576,6 +1580,9 @@ static ssize_t fuse_getxattr(struct dentry *entry, const char *name,if (fc->no_getxattr)return -EOPNOTSUPP ; +if (strncmp (name,XATTR_USER_PREFIX ,XATTR_USER_PREFIX_LEN ) !=0 ) +return -EOPNOTSUPP ; + req =fuse_get_req (fc);if (IS_ERR (req))return PTR_ERR (req); --1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 2 lines · Eric Paris
NAK, this makes no sense... We are working to support SELinux
attrs on fuse, why shouldn't we?
Certainly there is a logical disconnect here.
What is the sense in security labels when anyone can arbitrarily choose the security label they want and change the security label whenever they want?
What is the point in a security label when you can not trust it?
Eric
Why trust uids or rwx bits. Might as well do away with those as well, right?
Quoted 14 lines · Eric W. Biederman
Quoted 2 lines · Eric Paris
NAK, this makes no sense... We are working to support SELinux
attrs on fuse, why shouldn't we?Certainly there is a logical disconnect here.
What is the sense in security labels when anyone can arbitrarily choose the security label they want and change the security label whenever they want?
What is the point in a security label when you can not trust it?
Eric
Quoted 2 lines · Eric Paris
Why trust uids or rwx bits. Might as well do away with those as well, right?
Lying to your own userspace processes (which you can do with LD_PRELOAD) is rather different than lying to the selinux or the smack modules.
What I am saying with my patch is that fuse is remarkably non-nuanced in how it interacts with extended attributes, and that it appears very clear that there are bugs in the area of unprivileged mounts that need to be addressed.
I am happy to hear about better solutions. Telling me it's not a bug and sticking your head in the sand is quite amusing.
Eric
Quoted 8 lines · Eric W. Biederman
Quoted 2 lines · Eric Paris
Why trust uids or rwx bits. Might as well do away with those as well, right?
Lying to your own userspace processes (which you can do with
LD_PRELOAD) is rather different than lying to the selinux or the smack modules.
Quoted 7 lines
What I am saying with my patch is that fuse is remarkably non-nuanced in how it interacts with extended attributes, and that it appears very clear that there are bugs in the area of unprivileged mounts that need to be addressed.
I am happy to hear about better solutions. Telling me it's not a bug and sticking your head in the sand is quite amusing.
I'm not terribly familiar with the ways fuse modules can be loaded. Would it be possible to, at load time, based on the (selinux) credentials of the user loading the module, either allow the loader to specify that security.* and trusted.* xattrs may be used, or, if user is unprivileged, ignore the xattrs and use a default based on the user's credentials?
-serge
Quoted 8 lines · Eric W. Biederman
Quoted 2 lines · Eric Paris
Why trust uids or rwx bits. Might as well do away with those as well, right?
Lying to your own userspace processes (which you can do with
LD_PRELOAD) is rather different than lying to the selinux or the smack modules.
Quoted 7 lines
What I am saying with my patch is that fuse is remarkably non-nuanced in how it interacts with extended attributes, and that it appears very clear that there are bugs in the area of unprivileged mounts that need to be addressed.
I am happy to hear about better solutions. Telling me it's not a bug and sticking your head in the sand is quite amusing.
I'm not sure how to fix it. But breaking things that do work today for untrusted user mounts isn't right or acceptable. Maybe serge is onto something. Or maybe the best solution is to require LSM policy to just disallow all unpriv (from init namespace PoV) mounts.