On Fri, 10 Jun 2022 at 23:39, Andrii Nakryiko <andriin@xxxxxx> wrote: > > > > On 6/7/22 1:47 AM, Christian Brauner wrote: > > On Wed, Jun 01, 2022 at 11:44:07AM -0700, Dave Marchevsky wrote: [...] > >> +static bool __read_mostly allow_other_parent_userns; > >> +module_param(allow_other_parent_userns, bool, 0644); > >> +MODULE_PARM_DESC(allow_other_parent_userns, > >> + "Allow users not in mounting or descendant userns " > >> + "to access FUSE with allow_other set"); > > > > The name of the parameter also suggests that access is granted to parent > > userns tasks whereas the change seems to me to allows every task access > > to that fuse filesystem independent of what userns they are in. > > > > So even a task in a sibling userns could - probably with rather > > elaborate mount propagation trickery - access that fuse filesystem. > > > > AFaict, either the module parameter is misnamed or the patch doesn't > > implement the behavior expressed in the name. > > > > The original patch restricted access to a CAP_SYS_ADMIN capable task. > > Did we agree that it was a good idea to weaken it to all tasks? > > Shouldn't we still just restrict this to CAP_SYS_ADMIN capable tasks in > > the initial userns? > > I think it's fine to allow for CAP_SYS_ADMIN only, but can we then > ignore the allow_other mount option in such case? The idea is that > CAP_SYS_ADMIN allows you to read FUSE-backed contents no matter what, so > user not mounting with allow_other preventing root from reading contents > defeats the purpose at least partially. If we want to be compatible with "user_allow_other", then it should be checking if the uid/gid of the current task is mapped in the filesystems user_ns (fsuidgid_has_mapping()). Right? Thanks, Miklos
