Skip to content

Commit

Permalink
snapdir: add 'disabled' value to make .zfs inaccessible
Browse files Browse the repository at this point in the history
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 wheter 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]>
Signed-off-by: Fabian Grünbichler <[email protected]>
Fixes: openzfs#3963
  • Loading branch information
Fabian-Gruenbichler authored and behlendorf committed Oct 1, 2024
1 parent 141368a commit ce0a978
Show file tree
Hide file tree
Showing 16 changed files with 56 additions and 15 deletions.
2 changes: 1 addition & 1 deletion include/os/freebsd/zfs/sys/zfs_ctldir.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extern "C" {
((zdp)->z_zfsvfs->z_ctldir != NULL))
#define zfs_show_ctldir(zdp) \
(zfs_has_ctldir(zdp) && \
((zdp)->z_zfsvfs->z_show_ctldir))
((zdp)->z_zfsvfs->z_show_ctldir == ZFS_SNAPDIR_VISIBLE))

void zfsctl_create(zfsvfs_t *);
void zfsctl_destroy(zfsvfs_t *);
Expand Down
2 changes: 1 addition & 1 deletion include/os/freebsd/zfs/sys/zfs_vfsops_os.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ struct zfsvfs {
list_t z_all_znodes; /* all vnodes in the fs */
kmutex_t z_znodes_lock; /* lock for z_all_znodes */
struct zfsctl_root *z_ctldir; /* .zfs directory pointer */
boolean_t z_show_ctldir; /* expose .zfs in the root dir */
uint_t z_show_ctldir; /* how to expose .zfs in the root dir */
boolean_t z_issnap; /* true if this is a snapshot */
boolean_t z_use_fuids; /* version allows fuids */
boolean_t z_replay; /* set during ZIL replay */
Expand Down
2 changes: 1 addition & 1 deletion include/os/linux/zfs/sys/zfs_ctldir.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
(ZTOZSB(zdp)->z_ctldir != NULL))
#define zfs_show_ctldir(zdp) \
(zfs_has_ctldir(zdp) && \
(ZTOZSB(zdp)->z_show_ctldir))
(ZTOZSB(zdp)->z_show_ctldir == ZFS_SNAPDIR_VISIBLE))

extern int zfs_expire_snapshot;

Expand Down
2 changes: 1 addition & 1 deletion include/os/linux/zfs/sys/zfs_vfsops_os.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ struct zfsvfs {
kmutex_t z_znodes_lock; /* lock for z_all_znodes */
arc_prune_t *z_arc_prune; /* called by ARC to prune caches */
struct inode *z_ctldir; /* .zfs directory inode */
boolean_t z_show_ctldir; /* expose .zfs in the root dir */
uint_t z_show_ctldir; /* how to expose .zfs in the root dir */
boolean_t z_issnap; /* true if this is a snapshot */
boolean_t z_use_fuids; /* version allows fuids */
boolean_t z_replay; /* set during ZIL replay */
Expand Down
1 change: 1 addition & 0 deletions include/sys/zfs_ioctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ extern "C" {
*/
#define ZFS_SNAPDIR_HIDDEN 0
#define ZFS_SNAPDIR_VISIBLE 1
#define ZFS_SNAPDIR_DISABLED 2

/*
* Property values for snapdev
Expand Down
9 changes: 9 additions & 0 deletions man/man4/zfs.4
Original file line number Diff line number Diff line change
Expand Up @@ -1570,6 +1570,15 @@ which have the
.Em no_root_squash
option set.
.
.It Sy zfs_snapshot_no_setuid Ns = Ns Sy 0 Ns | Ns 1 Pq int
Whether to disable
.Em setuid/setgid
support for snapshot mounts triggered by access to the
.Sy .zfs/snapshot
directory by setting the
.Em nosuid
mount option.
.
.It Sy zfs_flags Ns = Ns Sy 0 Pq int
Set additional debugging flags.
The following flags may be bitwise-ored together:
Expand Down
2 changes: 1 addition & 1 deletion man/man7/zfsconcepts.7
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ File system snapshots can be accessed under the
directory in the root of the file system.
Snapshots are automatically mounted on demand and may be unmounted at regular
intervals.
The visibility of the
The availability and visibility of the
.Pa .zfs
directory can be controlled by the
.Sy snapdir
Expand Down
6 changes: 3 additions & 3 deletions man/man7/zfsprops.7
Original file line number Diff line number Diff line change
Expand Up @@ -1848,11 +1848,11 @@ Controls whether the volume snapshot devices under
are hidden or visible.
The default value is
.Sy hidden .
.It Sy snapdir Ns = Ns Sy hidden Ns | Ns Sy visible
.It Sy snapdir Ns = Ns Sy disabled Ns | Ns Sy hidden Ns | Ns Sy visible
Controls whether the
.Pa .zfs
directory is hidden or visible in the root of the file system as discussed in
the
directory is disabled, hidden or visible in the root of the file system as
discussed in the
.Sx Snapshots
section of
.Xr zfsconcepts 7 .
Expand Down
2 changes: 2 additions & 0 deletions module/os/freebsd/zfs/zfs_vnops_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 17 additions & 5 deletions module/os/linux/zfs/zfs_ctldir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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.
Expand All @@ -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)) {
Expand Down Expand Up @@ -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");
3 changes: 3 additions & 0 deletions module/os/linux/zfs/zfs_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions module/os/linux/zfs/zfs_vfsops.c
Original file line number Diff line number Diff line change
Expand Up @@ -1692,6 +1692,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);
Expand Down
4 changes: 4 additions & 0 deletions module/os/linux/zfs/zpl_ctldir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion module/zcommon/zfs_prop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
};

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions module/zfs/dsl_prop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/zfs-tests/include/properties.shlib
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ typeset -a logbias_prop_vals=('latency' 'throughput')
typeset -a primarycache_prop_vals=('all' 'none' 'metadata')
typeset -a redundant_metadata_prop_vals=('all' 'most' 'some' 'none')
typeset -a secondarycache_prop_vals=('all' 'none' 'metadata')
typeset -a snapdir_prop_vals=('hidden' 'visible')
typeset -a snapdir_prop_vals=('disabled' 'hidden' 'visible')
typeset -a sync_prop_vals=('standard' 'always' 'disabled')

typeset -a fs_props=('compress' 'checksum' 'recsize'
Expand Down

0 comments on commit ce0a978

Please sign in to comment.