Add pot group to protect pot root (#240)

* Remove mkdir when ZFS create the directory

This was introduced in b2fb5d7 and wasn't a good
approach after all.

Left in the mkdir function in common.sh and used it
in places where mkdir was also necessary beforehand.

* Add pot group and require it to access /opt/pot

This is the correct fix to #218, as discussed in
#233 (comment)

Fixes #218

Author: grembo

.github/workflows/ci.yml

@@ -69,6 +69,7 @@ jobs:
           echo pass >>/etc/pf.conf
           service pf enable
           service pf start
+          pw groupadd pot
           bin/pot init
           #
           ### Run CI tests ################################################

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - flavours: scripts are made executable when loading
 - destroy: remove status file when destroying
 - vnet: use unique epaira interface names (#232)
+- Add pot group to protect pot root (#240)
 
 ### Fixed
 - Reverted the change of permissions of pot root mountpoint to fix a regression (#233)

etc/pot/pot.conf.sample

@@ -13,6 +13,9 @@
 # This is where pot is going to store temporary files
 # POT_TMP=/tmp
 
+# This is the group owning POT_FS_ROOT
+# POT_GROUP=pot
+
 # This is the suffix added to temporary files created using mktemp,
 # X is a placeholder for a random character, see mktemp(1)
 # POT_MKTEMP_SUFFIX=.XXXXXXXX

etc/pot/pot.default.conf

@@ -13,6 +13,9 @@ POT_CACHE=/var/cache/pot
 # This is where pot is going to store temporary files
 POT_TMP=/tmp
 
+# This is the group owning POT_FS_ROOT
+POT_GROUP=pot
+
 # This is the suffix added to temporary files created using mktemp,
 # X is a placeholder for a random character, see mktemp(1)
 POT_MKTEMP_SUFFIX=.XXXXXXXX

share/pot/clone.sh

@@ -99,7 +99,6 @@ _cj_zfs()
 		fi
 		_debug "clone $_dset@$_snap into $_jdset/m"
 		zfs clone -o mountpoint="$_pdir/m" "$_dset@$_snap" "$_jdset/m"
-		_fix_pot_mountpoint_permissions "$_pdir/m"
 		touch "$_pdir/conf/fscomp.conf"
 		while read -r line ; do
 			_dset=$( echo "$line" | awk '{print $1}' )

share/pot/common.sh

@@ -171,6 +171,10 @@ _conf_check()
 		_qerror "$1" "POT_FS_ROOT is mandatory"
 		return 1 # false
 	fi
+	if ! getent group "${POT_GROUP:-pot}" >/dev/null 2>&1; then
+		_qerror "$1" "Group '${POT_GROUP:-pot}' is missing, create it or change POT_GROUP"
+		return 1 # false
+	fi
 	return 0 # true
 }
 
@@ -238,6 +242,16 @@ _zfs_exist()
 	return 0 # true
 }
 
+# check if the dataset $1 is mounted
+# $1 the dataset NAME
+_zfs_mounted()
+{
+	if [ "$(zfs get -Ho value mounted "$1")" != "yes" ]; then
+		return 1; # false
+	fi
+	return 0 # true
+}
+
 # given a dataset, look for the corresponding mountpoint
 # $1 the dataset
 _get_zfs_mountpoint()
@@ -938,22 +952,6 @@ _get_pot_snaps()
 	done
 }
 
-# $1 mountpoint to adjust permissions for
-_fix_pot_mountpoint_permissions()
-{
-	local _mp _exp_perm
-	_mp="$1"
-	_exp_perm="755"
-
-	if [ "$(stat -f "%Lp" "${_mp}")" != "$_exp_perm" ]; then
-		_debug "Setting mountpoint permission for $_mp"
-		# chomd 755 allows everyone inside the jail to access the file system
-		# permissions like 700 don't allow access to the file system to any non-user also in the jail
-		# causing issue to applications like nginx
-		chmod "$_exp_perm" "$_mp" || ${EXIT} 1
-	fi
-}
-
 # $1 mountpoint to create (proper permissions are applied)
 _create_pot_mountpoint()
 {
@@ -964,8 +962,6 @@ _create_pot_mountpoint()
 		_debug "Creating mountpoint $_mp"
 		mkdir -p "$_mp" || exit 1
 	fi
-
-	_fix_pot_mountpoint_permissions "$_mp"
 }
 
 # $1 pot name
@@ -1097,8 +1093,20 @@ pot-cmd()
 	shift
 	if [ ! -r "${_POT_INCLUDE}/${_cmd}.sh" ]; then
 		_error "Fatal error! $_cmd implementation not found!"
-		exit 1
+		${EXIT} 1
 	fi
+
+	if [ "$_cmd" != "init" ]&& [ "$_cmd" != "de-init" ] ; then
+		if [ ! -d "$POT_FS_ROOT" ]; then
+			_error "$POT_FS_ROOT does not exist, please run 'pot init'"
+			${EXIT} 1
+		fi
+		if [ ! -r "$POT_FS_ROOT" ]; then
+			_error "Current user has no read access to $POT_FS_ROOT"
+			${EXIT} 1
+		fi
+	fi
+
 	# shellcheck disable=SC1090
 	. "${_POT_INCLUDE}/${_cmd}.sh"
 	_func=pot-${_cmd}

share/pot/create.sh

@@ -79,8 +79,6 @@ _c_zfs_single()
 		_info "$_pdset exists already"
 	fi
 
-	_create_pot_mountpoint "$_pdir/m"
-
 	if [ -z "$_potbase" ]; then
 		# create an empty dataset
 		if ! zfs create "$_pdset/m" ; then

share/pot/init.sh

@@ -18,7 +18,7 @@ init-help()
 
 pot-init()
 {
-	local pf_file
+	local pf_file dataset
 	pf_file="$(sysrc -n pf_rules)"
 	OPTIND=1
 	while getopts "hvf:" _o ; do
@@ -42,6 +42,11 @@ pot-init()
 		${EXIT} 1
 	fi
 
+	if ! _conf_check "init" ; then
+		_qerror "init" "Configuration not valid, please verify it"
+		return 1 # false
+	fi
+
 	if ! _zfs_exist "${POT_ZFS_ROOT}" "${POT_FS_ROOT}" ; then
 		if _zfs_dataset_valid "${POT_ZFS_ROOT}" ; then
 			_error "${POT_ZFS_ROOT} is an invalid POT root"
@@ -63,19 +68,21 @@ pot-init()
 		fi
 	fi
 
+	# set root directory permissions and ownership
+	chmod 750 "${POT_FS_ROOT}" || ${EXIT} 1
+	chown root:"${POT_GROUP:-pot}" "${POT_FS_ROOT}" || ${EXIT} 1
+
 	# create mandatory datasets
-	if ! _zfs_dataset_valid "${POT_ZFS_ROOT}/bases" ; then
-		_debug "creating ${POT_ZFS_ROOT}/bases"
-		zfs create "${POT_ZFS_ROOT}/bases"
-	fi
-	if ! _zfs_dataset_valid "${POT_ZFS_ROOT}/jails" ; then
-		_debug "creating ${POT_ZFS_ROOT}/jails"
-		zfs create "${POT_ZFS_ROOT}/jails"
-	fi
-	if ! _zfs_dataset_valid "${POT_ZFS_ROOT}/fscomp" ; then
-		_debug "creating ${POT_ZFS_ROOT}/fscomp"
-		zfs create "${POT_ZFS_ROOT}/fscomp"
-	fi
+	for dataset in bases jails fscomp; do
+		if ! _zfs_dataset_valid "${POT_ZFS_ROOT}/$dataset" ; then
+			_debug "creating ${POT_ZFS_ROOT}/$dataset"
+			zfs create "${POT_ZFS_ROOT}/$dataset" || ${EXIT} 1
+		fi
+		if ! _zfs_mounted "${POT_ZFS_ROOT}/$dataset"; then
+			_debug "mounting ${POT_ZFS_ROOT}/$dataset"
+			zfs mount "${POT_ZFS_ROOT}/$dataset" || ${EXIT} 1
+		fi
+	done
 	if ! _zfs_exist "${POT_ZFS_ROOT}/cache" "${POT_CACHE}" ; then
 		_debug "creating ${POT_ZFS_ROOT}/cache mounted as ${POT_CACHE}"
 		if ! _zfs_dataset_valid "${POT_ZFS_ROOT}/cache" ; then

share/pot/mount-in.sh

@@ -64,7 +64,6 @@ _mountpoint_validation()
 	fi
 	# if the mountpoint doesn't exist, make it
 	if [ ! -d "$_mpdir/$_mnt_p" ]; then
-		_create_pot_mountpoint "$_mpdir"
 		if ! mkdir -p "$_mpdir/$_mnt_p" ; then
 			if eval $_mounted ; then
 				_pot_umount "$_pname" >/dev/null

share/pot/rename.sh

@@ -67,7 +67,6 @@ _rn_zfs()
 		zfs rename "$_dset" "$_nset"
 
 	#sudo zfs mount zroot/pot/jails/dns2
-		_create_pot_mountpoint "${POT_FS_ROOT}/jails/$_newname/m"
 		_debug "Mount $_nset"
 		zfs mount "$_nset"
 	#sudo zfs mount zroot/pot/jails/dns2/custom
@@ -88,7 +87,6 @@ _rn_zfs()
 		fi
 		_debug "Renaming $_dset in $_nset"
 		zfs rename "$_dset" "$_nset"
-		_create_pot_mountpoint "${POT_FS_ROOT}/jails/$_newname/m"
 		_debug "Mount $_nset"
 		zfs mount "$_nset"
 		if _zfs_dataset_valid "$_nset/m" ; then