[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: What are desired semantics for /etc/shells?



Hi,

I would like to thank you for the many constructive answers. Let me try
to summarize the discussion and try to draw conclusions.

On Thu, Jun 10, 2021 at 08:00:02PM +0200, Helmut Grohne wrote:
> Desired behaviour
> =================
> 
> This raises the question of what the desired semantics for `/etc/shells` are.
> Do we want the strict interpretation of the policy to be followed and update
> all those packages to conditionalize their `add-shell` invocations? Or is
> `/etc/shells` a simple collection of installed shells and administrators are
> not supposed to mess with it? The latter interpretation somewhat conflicts with
> our policy, so we might have to update it. If `/etc/shells` is not to be messed
> with, maybe it should not live inside `/etc`?

Consensus seems to be to use the strict policy interpretation: When you
change /etc/shells, upgrades should preserve those changes.

Some related issues were raised:

Richard Laager asked whether it was harmful to list shells that don't
actually exist on the system (such that the file could be shipped
statically). If not, shipping a static /etc/shells would be possible. He
highlighted that this would be bad for third party packages containing
shells. I've not further pursued this option.

Felix C. Stegerman cautioned that the contents of /etc/shells depends on
whether the underlying system is /usr-merged.

Johannes Schauer Marin Rodrigues mentioned (off-list) that /etc/shells
is not reproducible (in terms of reproducible installations) as its
order may differ.

> Declarative packaging
> =====================
> 
> I think using triggers has an obvious benefit here, but depending in the
> intended semantics of `/etc/shells`, implementing the part about preserving
> user changes may be difficult. A possible solution could be moving
> `/etc/shells` to `/var` and replacing it with a symbolic link when its contents
> match with the generated one one.

There are two proposals for a trigger-based solution:

Mattia Rizzolo extended my proposal of turning /etc/shells into a
symlink with additional files /etc/shells.add and /etc/shells.remove
that would influence the generation of /etc/shells.

Guillem Jover proposed maintaining /etc/shells as a regular file and
updating it using an external state file to recognize additions and
removals by packages.

Both proposals honour the request of allowing administrator changes to
be preserved. My proposal is more heavy handed in the transition code
and bears changes for administrators to adapt to. At present, I favour
Guillem's approach, because it is so unintrusive. It can coexist with
existing maintainer scripts and does not require any flag days.

I've taken Guillem's proposal and turned it into code. I propose adding
a utility update-shells. debianutils would declare a trigger interest in
/usr/share/debianutils/shells.d where other packages can declare their
shells and run `update-shells --root "$DPKG_ROOT"`. I've attached my
implementation of update-shells to this mail. It is written in posix
shell and beyond builtins it only uses dpkg (>= 1.20.1) for
dpkg-realpath and coreutils for chmod, chown, mv, rm, and sync.

How well does this implementation cope with the aspects raised earlier?
 * As per Guillem's proposal it retains administrator addition and
   removal of shells to /etc/shells on update.
 * It does not add shells that do not exist (unless a buggy shell
   includes such a filename).
 * When running update-shells after a /usr-merge, the file will be
   updated with all the relevant realpaths. After dpkg-fsys-usrunmess,
   running update-shells will go back to those that do exist.
 * While the order of /etc/shells will not be sorted, it will be
   deterministic if update-shells is run after all packages have been
   unpacked. Installing two packages one after another will still cause
   their order in /etc/shells to differ, but changing the order of
   /etc/shells could break comments left by administrators. So this is a
   compromise that partially improves reproducibility without regressing
   maintainability of /etc/shells. I hope that it is sufficient in
   practice.
 * The update-shells mechanism will support DPKG_ROOT.

I solicit feedback on this summary and approach. Barring unforseen
issues, I plan to open a bug against debianutils to incorporate the
change and once implemented opening bugs against all shell providers at
normal severity to convert their add-shell/remove-shell calls to
declarative ones and at rc-severity for not retaining local changes.
Deferring those bugs post bullseye seems sensible to me as fixing those
add-shell calls now and later turning them into declarative instructions
seems like double effort. While the behaviour is not policy-compliant
now, the number of people running into it must be fairly small.

Helmut
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0-or-later
# Copyright 2021 Helmut Grohne <helmut@subdivi.de>

# A "hashset" is a shell variable containing a sequence of elements separated
# and surrounded by hash (#) characters. None of the elements may contain a
# hash character. The character is thus chosen, because it initiates a comment
# in /etc/shells. All variables ending in _SHELLS in this file are hashsets.

set -e

# Check whether hashset $1 contains element $2.
hashset_contains() {
	case "$1" in
		*"#$2#"*) return 0 ;;
		*) return 1 ;;
	esac
}

log() {
	if [ "$VERBOSE" = 1 ]; then
		echo "$*"
	fi
}

ROOT=
VERBOSE=0
NOACT=0

while [ $# -gt 0 ]; do
	case "$1" in
		--help)
			cat <<EOF
usage: $0 [options]

 --no-act    Do not move the actual update into place
 --verbose   Be more verbose
 --root DIR  Operate on the given chroot, defaults to /
EOF
			exit 0
		;;
		--no-act)
			NOACT=1
		;;
		--root)
			shift
			if [ "$#" -lt 1 ]; then
				echo "missing argument to --root" 1>&2
				exit 1
			fi
			ROOT=$1
		;;
		--verbose)
			VERBOSE=1
		;;
		*)
			echo "unrecognized option $1" 1>&2
			exit 1
		;;
	esac
	shift
done

PKG_DIR="$ROOT/usr/share/debianutils/shells.d"
STATE_FILE="$ROOT/var/lib/shells.state"
ETC_FILE="$ROOT/etc/shells"
NEW_ETC_FILE="$ETC_FILE.tmp"
NEW_STATE_FILE="$STATE_FILE.tmp"

PKG_SHELLS='#'
for f in "$PKG_DIR/"*; do
	[ "$f" = "$PKG_DIR/*" ] && break
	while IFS='#' read -r line _; do
		[ -n "$line" ] && continue
		PKG_SHELLS="$PKG_SHELLS$line#"
		realshell=$(dpkg-realpath --root "$ROOT" "$line")
		if [ "$line" != "$realshell" ]; then
			PKG_SHELLS="$PKG_SHELLS$realshell#"
		fi
	done < "$f"
done

STATE_SHELLS='#'
if [ -e "$STATE_FILE" ] ; then
	while IFS='#' read -r line _; do
		[ -n "$line" ] && STATE_SHELLS="$STATE_SHELLS$line#"
	done < "$STATE_FILE"
fi

cleanup() {
	rm -f "$NEW_ETC_FILE" "$NEW_STATE_FILE"
}
trap cleanup EXIT

: > "$NEW_ETC_FILE"
ETC_SHELLS='#'
while IFS= read -r line; do
	shell=${line%%#*}
	# copy all comment lines, packaged shells and local additions
	if [ -z "$shell" ] ||
			hashset_contains "$PKG_SHELLS" "$shell" ||
			! hashset_contains "$STATE_SHELLS" "$shell"; then
		echo "$line" >> "$NEW_ETC_FILE"
		ETC_SHELLS="$ETC_SHELLS$shell#"
	else
		log "removing shell $shell"
	fi
done < "$ETC_FILE"

: > "$NEW_STATE_FILE"
saved_IFS=$IFS
IFS='#'
set -f
# shellcheck disable=SC2086 # word splitting intended, globbing disabled
set -- ${PKG_SHELLS###}
set +f
IFS=$saved_IFS
for shell; do
	echo "$shell" >> "$NEW_STATE_FILE"
	# add shells that are neither already present nor locally removed
	if ! hashset_contains "$ETC_SHELLS" "$shell" &&
			! hashset_contains "$STATE_SHELLS" "$shell"; then
		echo "$shell" >> "$NEW_ETC_FILE"
		log "adding shell $shell"
	fi
done

if [ "$NOACT" = 0 ]; then
	if [ -e "$NEW_STATE_FILE" ]; then
		chmod --reference="$STATE_FILE" "$NEW_STATE_FILE"
		chown --reference="$STATE_FILE" "$NEW_STATE_FILE"
	else
		chmod 0644 "$NEW_STATE_FILE"
	fi
	chmod --reference="$ETC_FILE" "$NEW_ETC_FILE"
	chown --reference="$ETC_FILE" "$NEW_ETC_FILE"
	sync --data "$NEW_ETC_FILE" "$NEW_STATE_FILE"
	mv "$NEW_ETC_FILE" "$ETC_FILE"
	sync "$ETC_FILE"
	mv "$NEW_STATE_FILE" "$STATE_FILE"
	sync "$STATE_FILE"
	trap "" EXIT
fi

Reply to: