mbox series

[RFC,0/5] Add a chroot option to nfs.conf

Message ID 20190514204153.79603-1-trond.myklebust@hammerspace.com (mailing list archive)
Headers show
Series Add a chroot option to nfs.conf | expand

Message

Trond Myklebust May 14, 2019, 8:41 p.m. UTC
The following patchset aims to allow the configuration of a 'chroot
jail' to rpc.nfsd, and allowing us to export a filesystem /foo (and
possibly subtrees) as '/'.

Trond Myklebust (5):
  mountd: Ensure we don't share cache file descriptors among processes.
  Add a simple workqueue mechanism
  Add a helper to write to a file through the chrooted thread
  Add support for chrooted exports
  Add support for chroot in exportfs

 aclocal/libpthread.m4      |  13 +-
 configure.ac               |   6 +-
 nfs.conf                   |   1 +
 support/include/misc.h     |  11 ++
 support/misc/Makefile.am   |   2 +-
 support/misc/workqueue.c   | 267 +++++++++++++++++++++++++++++++++++++
 systemd/nfs.conf.man       |   3 +-
 utils/exportfs/Makefile.am |   2 +-
 utils/exportfs/exportfs.c  |  31 ++++-
 utils/mountd/Makefile.am   |   3 +-
 utils/mountd/cache.c       |  39 +++++-
 utils/mountd/mountd.c      |   5 +-
 utils/nfsd/nfsd.man        |   4 +
 13 files changed, 369 insertions(+), 18 deletions(-)
 create mode 100644 support/misc/workqueue.c

Comments

J. Bruce Fields May 15, 2019, 2:01 p.m. UTC | #1
On Tue, May 14, 2019 at 04:41:48PM -0400, Trond Myklebust wrote:
> The following patchset aims to allow the configuration of a 'chroot
> jail' to rpc.nfsd, and allowing us to export a filesystem /foo (and
> possibly subtrees) as '/'.

This is great, thanks!  Years ago I did an incomplete version that
worked by just string manipulations on paths.  Running part of mountd in
a chrooted thread is a neat way to do it.

If I understand right, the only part that's being run in a chroot is the
writes to /proc/net/sunrpc/*/channel files.  So that means that the path
included in writes to /proc/net/sunrpc/nfsd_fh/channel will be resolved
with respect to the chroot by the kernel code handling the write.

That's not the only place in mountd that uses export paths, though.
What about:

	- next_mnt(), which compares paths from the export file to paths
	  in /etc/mtab.
	- is_mountpoint, which stats export paths.
	- match_fsid, which stats export paths.

Etc.  Doesn't that stuff also need to be done with respect to the
chroot?  Am I missing something?

--b.

> 
> Trond Myklebust (5):
>   mountd: Ensure we don't share cache file descriptors among processes.
>   Add a simple workqueue mechanism
>   Add a helper to write to a file through the chrooted thread
>   Add support for chrooted exports
>   Add support for chroot in exportfs
> 
>  aclocal/libpthread.m4      |  13 +-
>  configure.ac               |   6 +-
>  nfs.conf                   |   1 +
>  support/include/misc.h     |  11 ++
>  support/misc/Makefile.am   |   2 +-
>  support/misc/workqueue.c   | 267 +++++++++++++++++++++++++++++++++++++
>  systemd/nfs.conf.man       |   3 +-
>  utils/exportfs/Makefile.am |   2 +-
>  utils/exportfs/exportfs.c  |  31 ++++-
>  utils/mountd/Makefile.am   |   3 +-
>  utils/mountd/cache.c       |  39 +++++-
>  utils/mountd/mountd.c      |   5 +-
>  utils/nfsd/nfsd.man        |   4 +
>  13 files changed, 369 insertions(+), 18 deletions(-)
>  create mode 100644 support/misc/workqueue.c
> 
> -- 
> 2.21.0
Trond Myklebust May 15, 2019, 2:38 p.m. UTC | #2
On Wed, 2019-05-15 at 10:01 -0400, J. Bruce Fields wrote:
> On Tue, May 14, 2019 at 04:41:48PM -0400, Trond Myklebust wrote:
> > The following patchset aims to allow the configuration of a 'chroot
> > jail' to rpc.nfsd, and allowing us to export a filesystem /foo (and
> > possibly subtrees) as '/'.
> 
> This is great, thanks!  Years ago I did an incomplete version that
> worked by just string manipulations on paths.  Running part of mountd
> in
> a chrooted thread is a neat way to do it.
> 
> If I understand right, the only part that's being run in a chroot is
> the
> writes to /proc/net/sunrpc/*/channel files.  So that means that the
> path
> included in writes to /proc/net/sunrpc/nfsd_fh/channel will be
> resolved
> with respect to the chroot by the kernel code handling the write.
> 
> That's not the only place in mountd that uses export paths, though.
> What about:
> 
> 	- next_mnt(), which compares paths from the export file to
> paths
> 	  in /etc/mtab.
> 	- is_mountpoint, which stats export paths.
> 	- match_fsid, which stats export paths.
> 
> Etc.  Doesn't that stuff also need to be done with respect to the
> chroot?  Am I missing something?
> 

Good feedback. Thanks!

Yes, I do need to fix up those comparisons too. I suspect that we want
to do the stat() in the chrooted namespace so that we resolve symlinks
etc correctly. That should be easy to add: the workqueue implementation
is generic, so adding a new operation is pretty trivial.

For the path comparisons in next_mnt(), things are a little murkier.
I'm not overly happy with a solution where user space tries to resolve
paths because the presence of symlinks, bind mounts, etc can and do
mean that user space will get that wrong. Perhaps the right solution is
to do an open() and check that it ends up in the right place (i.e.
check the fsid/inode number with a fstat())?