Re: [PATCH v2 0/3] fuse: Add support for mounts from pid/user namespaces

[Michael j Theall <mtheall-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org> wrote on 10/06/2014 11:00:06 AM:

> From: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
> To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>, Miklos Szeredi 
> <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>, Alexander Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>, fuse-
> devel <fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>, Kernel Mailing List 
> <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Linux-Fsdevel <linux-
> fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "Serge E. Hallyn" <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>
> Date: 10/06/2014 11:04 AM
> Subject: Re: [fuse-devel] [PATCH v2 0/3] fuse: Add support for 
> mounts from pid/user namespaces
> 
> Quoting Seth Forshee (seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org):
> ...
> > After digging into this some more I think I agree with you. At minimum
> > letting users insert arbitrary xattrs via fuse bypasses the usual
> > restrictions on setting xattrs. This is probably mitigated by the
> > limited visibility of the fuse mount in the usual case for 
unprivileged
> > users, but it does seem like a bad idea fundamentally.
> > 
> > So I was thinking of something like the following (untested) to let 
root
> > in the host support privileged xattrs while limiting unprivileged 
users
> > to user.*. Miklos, does this look acceptable or would you prefer
> > something different?
> 
> So it won't be possible to set capabilities in a fuse fs?  This may
> be necessary, but it will prevent i.e. live-iso builders from writing
> for instance a CAP_NET_RAW=pe (instead of setuid-root) /bin/ping in the
> iso.

Our filesystem passes through security.* (even though neither our backing 
filesystem nor FUSE enforce the SELinux labels; we simply store the data). 
This was more for future-proofing. We also intercept 
system.posix_acl_access and system.posix_acl_default and translate them to 
the backing filesystem's ACL system. The trusted.* namespace is also 
pass-through.

Apart from these, we have many additional file attributes which we expose 
via system.* xattrs. Some are immutable, while the mutable ones are 
subject to input validation for setxattr(2) (e.g. some can only be an 
integer value). None of them can be deleted with removexattr(2). These 
attributes always exist for every file.

We also reserve the user.* namespace for truly user-define attributes with 
arbitrary values.

We fully expect these namespaces to work on privileged and unprivileged 
mounts alike. If that's not going to be possible anymore, we'll probably 
need some guidance on how to work around these limitations.

> 
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index e3123bfbc711..1a3ee5663dea 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -1882,6 +1882,10 @@ static int fuse_setxattr(struct dentry 
> *entry, const char *name,
> >     if (fc->no_setxattr)
> >        return -EOPNOTSUPP;
> > 
> > +   if (!(fc->flags & FUSE_PRIV_XATTRS) &&
> > +       strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN) != 0)
> > +      return -EOPNOTSUPP;
> > +
> >     req = fuse_get_req_nopages(fc);
> >     if (IS_ERR(req))
> >        return PTR_ERR(req);
> > @@ -1925,6 +1929,10 @@ static ssize_t fuse_getxattr(struct dentry 
> *entry, const char *name,
> >     if (fc->no_getxattr)
> >        return -EOPNOTSUPP;
> > 
> > +   if (!(fc->flags & FUSE_PRIV_XATTRS) &&
> > +       strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN) != 0)
> > +      return -EOPNOTSUPP;
> > +
> >     req = fuse_get_req_nopages(fc);
> >     if (IS_ERR(req))
> >        return PTR_ERR(req);
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 81187ba04e4a..bc0fd14b962a 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -46,6 +46,11 @@
> >      doing the mount will be allowed to access the filesystem */
> >  #define FUSE_ALLOW_OTHER         (1 << 1)
> > 
> > +/** If the FUSE_PRIV_XATTRS flag is given, then xattrs outside the
> > +    user.* namespace are allowed. This option is only allowed for
> > +    system root. */
> > +#define FUSE_PRIV_XATTRS   (1 << 2)
> > +
> >  /** Number of page pointers embedded in fuse_req */
> >  #define FUSE_REQ_INLINE_PAGES 1
> > 
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index b88b5a780228..6716b56d43a1 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -493,6 +493,7 @@ enum {
> >     OPT_ALLOW_OTHER,
> >     OPT_MAX_READ,
> >     OPT_BLKSIZE,
> > +   OPT_PRIV_XATTRS,
> >     OPT_ERR
> >  };
> > 
> > @@ -505,6 +506,7 @@ static const match_table_t tokens = {
> >     {OPT_ALLOW_OTHER,      "allow_other"},
> >     {OPT_MAX_READ,         "max_read=%u"},
> >     {OPT_BLKSIZE,         "blksize=%u"},
> > +   {OPT_PRIV_XATTRS,      "priv_xattr"},
> >     {OPT_ERR,         NULL}
> >  };
> > 
> > @@ -592,6 +594,12 @@ static int parse_fuse_opt(char *opt, struct 
> fuse_mount_data *d, int is_bdev)
> >           d->blksize = value;
> >           break;
> > 
> > +      case OPT_PRIV_XATTRS:
> > +         if (!capable(CAP_SYS_ADMIN))
> > +            return 0;
> > +         d->flags |= FUSE_PRIV_XATTRS;
> > +         break;
> > +
> >        default:
> >           return 0;
> >        }
> > 
> 
> 
------------------------------------------------------------------------------
> Slashdot TV.  Videos for Nerds.  Stuff that Matters.
> 
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk

> _______________________________________________
> fuse-devel mailing list
> fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/fuse-devel
> 
------------------------------------------------------------------------------
Slashdot TV.  Videos for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk