Re: [Virtio-fs] virtiofsd: Any reason why there's not an "openat2" sandbox mode?

[Sergio Lopez]

On Tue, Sep 27, 2022 at 04:14:20PM -0400, Stefan Hajnoczi wrote:
> On Tue, Sep 27, 2022 at 01:51:41PM -0400, Colin Walters wrote:
> > 
> > 
> > On Tue, Sep 27, 2022, at 1:27 PM, German Maglione wrote:
> > >
> > >> > Now all the development has moved to rust virtiofsd.
> > 
> > Oh, awesome!!  The code there looks great.
> > 
> > > I could work on this for the next major version and see if anything 
> > > breaks.
> > > But I prefer to add this as a compilation feature, instead of a command 
> > > line
> > > option that we will then have to maintain for a while.
> > 
> > Hmm, what would be the issue with having the code there by default?  I 
> > think rather than any new command line option, we automatically use 
> > `openat2+RESOLVE_IN_ROOT` if the process is run as a nonzero uid.
> > 
> > > Also, I don't see it as a sandbox feature, as Stefan mentioned, a 
> > > compromised
> > > process can call openat2() without RESOLVE_IN_ROOT. 
> > 
> > I'm a bit skeptical honestly about how secure the existing namespace code 
> > is against a compromised virtiofsd process.  The primary worry is guest 
> > filesystem traversals, right?  openat2+RESOLVE_IN_ROOT addresses that.  
> > Plus being in Rust makes this dramatically safer.
> > 
> > > I did some test with
> > > Landlock to lock virtiofsd inside the shared directory, but IIRC it 
> > > requires a
> > > kernel 5.13
> > 
> > But yes, landlock and other things make sense, I just don't see these 
> > things as strongly linked.  IOW we shouldn't in my opinion block 
> > unprivileged virtiofsd on more sandboxing than openat2 already gives us.
> 
> I think openat2(RESOLVE_IN_ROOT) support should be added unless there is
> another unprivileged mechanism that is stronger.
> 
> The security implications need to be covered in the user documentation
> so people can decide whether using this mode is appropriate.
> 
> We should continue to explain the difference between a voluntary
> mechanism like openat2(RESOLVE_IN_ROOT) and a mandatory mechanism like
> mount namespaces with pivot_root(2). Rust programs are not immune to
> arbitrary code execution, but it's less likely than with a C program.

I agree. Perhaps we could modify the "none" sandbox mode to use
openat2, if available, and add an "openat2" mode which does basically
the same thing, but bailing out if openat2 is not available.

And explain this clearly in the docs, of course.

Sergio.

signature.asc
Description: PGP signature