snapdir: add 'disabled' value to make .zfs inaccessible

In some environments, just making the .zfs control dir hidden from sight
might not be enough. In particular, the following scenarios might
warrant not allowing access at all:
- old snapshots with wrong permissions/ownership
- old snapshots with exploitable setuid/setgid binaries
- old snapshots with sensitive contents

Introducing a new 'disabled' value that not only hides the control dir,
but prevents access to its contents by returning ENOENT solves all of
the above.

The new property value takes advantage of 'iuv' semantics ("ignore
unknown value") to automatically fall back to the old default value when
a pool is accessed by an older version of ZFS that doesn't yet know
about 'disabled' semantics.

I think that technically the zfs_dirlook change is enough to prevent
access, but preventing lookups and dir entries in an already opened .zfs
handle might also be a good idea to prevent races when modifying the
property at runtime.

Add zfs_snapshot_no_setuid parameter to control whether automatically
mounted snapshots have the setuid mount option set or not.

this could be considered a partial fix for one of the scenarios
mentioned in desired.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Signed-off-by: Fabian Grünbichler <[email protected]>
Co-authored-by: Fabian Grünbichler <[email protected]>
Closes #3963
Closes #16587 

7 files changed, 37 insertions, 6 deletions

diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c
index 60deab1f5..a2222a899 100644
--- a/module/os/freebsd/zfs/zfs_vnops_os.c
+++ b/module/os/freebsd/zfs/zfs_vnops_os.c
@@ -774,6 +774,8 @@ zfs_lookup(vnode_t *dvp, const char *nm, vnode_t **vpp,
 	}
 	if (zfs_has_ctldir(zdp) && strcmp(nm, ZFS_CTLDIR_NAME) == 0) {
 		zfs_exit(zfsvfs, FTAG);
+		if (zfsvfs->z_show_ctldir == ZFS_SNAPDIR_DISABLED)
+			return (SET_ERROR(ENOENT));
 		if ((cnp->cn_flags & ISLASTCN) != 0 && nameiop != LOOKUP)
 			return (SET_ERROR(ENOTSUP));
 		error = zfsctl_root(zfsvfs, cnp->cn_lkflags, vpp);
diff --git a/module/os/linux/zfs/zfs_ctldir.c b/module/os/linux/zfs/zfs_ctldir.c
index a8b25b2bd..8a42a075c 100644
--- a/module/os/linux/zfs/zfs_ctldir.c
+++ b/module/os/linux/zfs/zfs_ctldir.c
@@ -111,6 +111,7 @@ static krwlock_t zfs_snapshot_lock;
  */
 int zfs_expire_snapshot = ZFSCTL_EXPIRE_SNAPSHOT;
 static int zfs_admin_snapshot = 0;
+static int zfs_snapshot_no_setuid = 0;
 
 typedef struct {
 	char		*se_name;	/* full snapshot name */
@@ -807,7 +808,9 @@ zfsctl_root_lookup(struct inode *dip, const char *name, struct inode **ipp,
 	if ((error = zfs_enter(zfsvfs, FTAG)) != 0)
 		return (error);
 
-	if (strcmp(name, "..") == 0) {
+	if (zfsvfs->z_show_ctldir == ZFS_SNAPDIR_DISABLED) {
+		*ipp = NULL;
+	} else if (strcmp(name, "..") == 0) {
 		*ipp = dip->i_sb->s_root->d_inode;
 	} else if (strcmp(name, ZFS_SNAPDIR_NAME) == 0) {
 		*ipp = zfsctl_inode_lookup(zfsvfs, ZFSCTL_INO_SNAPDIR,
@@ -1097,9 +1100,9 @@ zfsctl_snapshot_mount(struct path *path, int flags)
 	zfsvfs_t *zfsvfs;
 	zfsvfs_t *snap_zfsvfs;
 	zfs_snapentry_t *se;
-	char *full_name, *full_path;
+	char *full_name, *full_path, *options;
 	char *argv[] = { "/usr/bin/env", "mount", "-i", "-t", "zfs", "-n",
-	    NULL, NULL, NULL };
+	    "-o", NULL, NULL, NULL, NULL };
 	char *envp[] = { NULL };
 	int error;
 	struct path spath;
@@ -1113,6 +1116,7 @@ zfsctl_snapshot_mount(struct path *path, int flags)
 
 	full_name = kmem_zalloc(ZFS_MAX_DATASET_NAME_LEN, KM_SLEEP);
 	full_path = kmem_zalloc(MAXPATHLEN, KM_SLEEP);
+	options = kmem_zalloc(7, KM_SLEEP);
 
 	error = zfsctl_snapshot_name(zfsvfs, dname(dentry),
 	    ZFS_MAX_DATASET_NAME_LEN, full_name);
@@ -1128,6 +1132,9 @@ zfsctl_snapshot_mount(struct path *path, int flags)
 	    zfsvfs->z_vfs->vfs_mntpoint ? zfsvfs->z_vfs->vfs_mntpoint : "",
 	    dname(dentry));
 
+	snprintf(options, 7, "%s",
+	    zfs_snapshot_no_setuid ? "nosuid" : "suid");
+
 	/*
 	 * Multiple concurrent automounts of a snapshot are never allowed.
 	 * The snapshot may be manually mounted as many times as desired.
@@ -1150,8 +1157,9 @@ zfsctl_snapshot_mount(struct path *path, int flags)
 	 * value from call_usermodehelper() will be (exitcode << 8 + signal).
 	 */
 	dprintf("mount; name=%s path=%s\n", full_name, full_path);
-	argv[6] = full_name;
-	argv[7] = full_path;
+	argv[7] = options;
+	argv[8] = full_name;
+	argv[9] = full_path;
 	error = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
 	if (error) {
 		if (!(error & MOUNT_BUSY << 8)) {
@@ -1312,3 +1320,7 @@ MODULE_PARM_DESC(zfs_admin_snapshot, "Enable mkdir/rmdir/mv in .zfs/snapshot");
 
 module_param(zfs_expire_snapshot, int, 0644);
 MODULE_PARM_DESC(zfs_expire_snapshot, "Seconds to expire .zfs/snapshot");
+
+module_param(zfs_snapshot_no_setuid, int, 0644);
+MODULE_PARM_DESC(zfs_snapshot_no_setuid,
+	"Disable setuid/setgid for automounts in .zfs/snapshot");
diff --git a/module/os/linux/zfs/zfs_dir.c b/module/os/linux/zfs/zfs_dir.c
index f59281f06..564e89b37 100644
--- a/module/os/linux/zfs/zfs_dir.c
+++ b/module/os/linux/zfs/zfs_dir.c
@@ -415,6 +415,9 @@ zfs_dirlook(znode_t *dzp, char *name, znode_t **zpp, int flags,
 			*zpp = zp;
 		rw_exit(&dzp->z_parent_lock);
 	} else if (zfs_has_ctldir(dzp) && strcmp(name, ZFS_CTLDIR_NAME) == 0) {
+		if (ZTOZSB(dzp)->z_show_ctldir == ZFS_SNAPDIR_DISABLED) {
+			return (SET_ERROR(ENOENT));
+		}
 		ip = zfsctl_root(dzp);
 		*zpp = ITOZ(ip);
 	} else {
diff --git a/module/os/linux/zfs/zfs_vfsops.c b/module/os/linux/zfs/zfs_vfsops.c
index a24f50412..de3e8c89c 100644
--- a/module/os/linux/zfs/zfs_vfsops.c
+++ b/module/os/linux/zfs/zfs_vfsops.c
@@ -1702,6 +1702,11 @@ zfs_vget(struct super_block *sb, struct inode **ipp, fid_t *fidp)
 	    (object == ZFSCTL_INO_ROOT || object == ZFSCTL_INO_SNAPDIR)) {
 		*ipp = zfsvfs->z_ctldir;
 		ASSERT(*ipp != NULL);
+
+		if (zfsvfs->z_show_ctldir == ZFS_SNAPDIR_DISABLED) {
+			return (SET_ERROR(ENOENT));
+		}
+
 		if (object == ZFSCTL_INO_SNAPDIR) {
 			VERIFY(zfsctl_root_lookup(*ipp, "snapshot", ipp,
 			    0, kcred, NULL, NULL) == 0);
diff --git a/module/os/linux/zfs/zpl_ctldir.c b/module/os/linux/zfs/zpl_ctldir.c
index 56a30be51..fe64bc710 100644
--- a/module/os/linux/zfs/zpl_ctldir.c
+++ b/module/os/linux/zfs/zpl_ctldir.c
@@ -57,6 +57,10 @@ zpl_root_iterate(struct file *filp, struct dir_context *ctx)
 	zfsvfs_t *zfsvfs = ITOZSB(file_inode(filp));
 	int error = 0;
 
+	if (zfsvfs->z_show_ctldir == ZFS_SNAPDIR_DISABLED) {
+		return (SET_ERROR(ENOENT));
+	}
+
 	if ((error = zpl_enter(zfsvfs, FTAG)) != 0)
 		return (error);
 
diff --git a/module/zcommon/zfs_prop.c b/module/zcommon/zfs_prop.c
index f7e6b41bf..40254c8d9 100644
--- a/module/zcommon/zfs_prop.c
+++ b/module/zcommon/zfs_prop.c
@@ -238,6 +238,7 @@ zfs_prop_init(void)
 	static const zprop_index_t snapdir_table[] = {
 		{ "hidden",	ZFS_SNAPDIR_HIDDEN },
 		{ "visible",	ZFS_SNAPDIR_VISIBLE },
+		{ "disabled",	ZFS_SNAPDIR_DISABLED },
 		{ NULL }
 	};
 
@@ -436,7 +437,7 @@ zfs_prop_init(void)
 	    "COMPRESS", compress_table, sfeatures);
 	zprop_register_index(ZFS_PROP_SNAPDIR, "snapdir", ZFS_SNAPDIR_HIDDEN,
 	    PROP_INHERIT, ZFS_TYPE_FILESYSTEM,
-	    "hidden | visible", "SNAPDIR", snapdir_table, sfeatures);
+	    "disabled | hidden | visible", "SNAPDIR", snapdir_table, sfeatures);
 	zprop_register_index(ZFS_PROP_SNAPDEV, "snapdev", ZFS_SNAPDEV_HIDDEN,
 	    PROP_INHERIT, ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME,
 	    "hidden | visible", "SNAPDEV", snapdev_table, sfeatures);
diff --git a/module/zfs/dsl_prop.c b/module/zfs/dsl_prop.c
index 3b8683593..1a0e83419 100644
--- a/module/zfs/dsl_prop.c
+++ b/module/zfs/dsl_prop.c
@@ -698,6 +698,10 @@ dsl_prop_set_iuv(objset_t *mos, uint64_t zapobj, const char *propname,
 		    *(uint64_t *)value == ZFS_REDUNDANT_METADATA_NONE)
 			iuv = B_TRUE;
 		break;
+	case ZFS_PROP_SNAPDIR:
+		if (*(uint64_t *)value == ZFS_SNAPDIR_DISABLED)
+			iuv = B_TRUE;
+		break;
 	default:
 		break;
 	}