Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian....@packages.debian.org
Usertags: pu
[ Reason ]
There is a path canonicalization bug in update-shells that may lead to
- insertion of duplicate entries in /etc/shells
- failure to remove obsolete entries from /etc/shells
on merged-/usr systems. This is triggered if the shell itself is a
symlink, e.g. in 9base it is managed via update-alternatives.
Furthermore the handling of /bin/sh by update-shells was inconsistent
with all other shells, e.g. update-shells didn't generate the
corresponding /usr/bin/sh entry (while usrmerge did).
Miscanonicalization could also lead to invalid (i.e. nonexistent) paths
added to /etc/shells, although that does not happen with the packages
currently in the archive.
[ Impact ]
The bad entries in /etc/shells are probably harmless for normal
operation (duplicate or stale entries), but nasty for QA tooling
checking for inconsistencies. I prefer to fix these issues properly
(where possible) instead of working around them (or ignoring them) in QA
tools.
[ Tests ]
piuparts install and upgrade tests of all packages making use of
update-shells using the fixed package in merged-/usr and unmerged-/usr
contexts. No inconsistencies in /etc/shells (missing or leftover
entries) were found after the fixed package was applied.
[ Risks ]
Low. The changes are fairly minimal.
[ Checklist ]
[x] *all* changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in (old)stable
[x] the issue is verified as fixed in unstable
[ Changes ]
- try to avoid creating duplicate entries in /etc/shells (using the same
mechanism that is already used multiple times in update-shells)
(duplicates should no longer happen with the canonicalization bug
fixed, unless a shell is defined multiple times in shells.d, so let's
fix these, too)
- handle /bin/sh (which is defined in /usr/share/debianutils/shells (the
template for /etc/shells) but not in /usr/share/debianutils/shells.d/)
like any other shell by treating the template file as if it were part
of shells.d/
- canonicalize only the directory of the shell (not the shell itself)
to compute the shell in an aliased location (on merged-/usr systems)
This is rebuild of the package from sid with no further changes.
[ Other info ]
With these fixes applied, my intent is to make usrmerge call
update-shells to ensure the state file of update-shells is updated after
the usrmerge conversion, too.
Andreas
diff --git a/debian/changelog b/debian/changelog
index eb37be1..b50154c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,19 @@
+debianutils (5.7-0.5~deb12u1) bookworm; urgency=medium
+
+ * Rebuild for bookworm.
+
+ -- Andreas Beckmann <a...@debian.org> Mon, 03 Jul 2023 13:56:27 +0200
+
+debianutils (5.7-0.5) unstable; urgency=medium
+
+ * Non-maintainer upload.
+ * update-shells: Do not create duplicate entries in /etc/shells.
+ * update-shells: Manage (/usr)/bin/sh in the state file.
+ * update-shells: Fix canonicalization of shells in aliased locations.
+ (Closes: #1035820)
+
+ -- Andreas Beckmann <a...@debian.org> Thu, 22 Jun 2023 21:59:33 +0200
+
debianutils (5.7-0.4) unstable; urgency=medium
* Non-maintainer upload
diff --git a/debian/patches/dpkg-root b/debian/patches/dpkg-root.patch
similarity index 87%
rename from debian/patches/dpkg-root
rename to debian/patches/dpkg-root.patch
index 2641b70..ead43a1 100644
--- a/debian/patches/dpkg-root
+++ b/debian/patches/dpkg-root.patch
@@ -1,3 +1,6 @@
+Author: Johannes Schauer Marin Rodrigues <jo...@debian.org>
+Description: set the default for ROOT to $DPKG_ROOT
+
--- a/update-shells
+++ b/update-shells
@@ -23,7 +23,7 @@ log() {
diff --git a/debian/patches/fix-aliased-location.patch
b/debian/patches/fix-aliased-location.patch
new file mode 100644
index 0000000..97e1efe
--- /dev/null
+++ b/debian/patches/fix-aliased-location.patch
@@ -0,0 +1,18 @@
+Author: Andreas Beckmann <a...@debian.org>
+Description: fix detection of shells in aliased locations
+ only canonicalize the directory where the shell resides,
+ not the shell itself (which may be a symlink, e.g. managed by
+ update-alternatives)
+Bug-Debian: https://bugs.debian.org/1035820
+
+--- a/update-shells
++++ b/update-shells
+@@ -80,7 +80,7 @@ for f in "$TEMPLATE_ETC_FILE" "$PKG_DIR/
+ while IFS='#' read -r line _; do
+ [ -n "$line" ] || continue
+ PKG_SHELLS="$PKG_SHELLS$line#"
+- realshell=$(dirname "$(dpkg-realpath --root "$ROOT"
"$line")")/$(basename "$line")
++ realshell=$(dpkg-realpath --root "$ROOT" "$(dirname
"$line")")/$(basename "$line")
+ if [ "$line" != "$realshell" ]; then
+ PKG_SHELLS="$PKG_SHELLS$realshell#"
+ fi
diff --git a/debian/patches/manage-usr-bin-sh.patch
b/debian/patches/manage-usr-bin-sh.patch
new file mode 100644
index 0000000..b9122e9
--- /dev/null
+++ b/debian/patches/manage-usr-bin-sh.patch
@@ -0,0 +1,29 @@
+Author: Andreas Beckmann <a...@debian.org>
+Description: manage (/usr)/bin/sh in the state file
+
+--- a/update-shells
++++ b/update-shells
+@@ -63,19 +63,20 @@ done
+
+ PKG_DIR="$ROOT/usr/share/debianutils/shells.d"
+ STATE_FILE="$ROOT/var/lib/shells.state"
++TEMPLATE_ETC_FILE="$ROOT/usr/share/debianutils/shells"
+ TARGET_ETC_FILE="$ROOT/etc/shells"
+ SOURCE_ETC_FILE="$TARGET_ETC_FILE"
+ NEW_ETC_FILE="$TARGET_ETC_FILE.tmp"
+ NEW_STATE_FILE="$STATE_FILE.tmp"
+
+ if ! test -e "$SOURCE_ETC_FILE"; then
+- SOURCE_ETC_FILE="$ROOT/usr/share/debianutils/shells"
++ SOURCE_ETC_FILE="$TEMPLATE_ETC_FILE"
+ fi
+
+ PKG_SHELLS='#'
+ LC_COLLATE=C.UTF-8 # glob in reproducible order
+-for f in "$PKG_DIR/"*; do
+- [ "$f" = "$PKG_DIR/*" ] && break
++for f in "$TEMPLATE_ETC_FILE" "$PKG_DIR/"*; do
++ test -f "$f" || continue
+ while IFS='#' read -r line _; do
+ [ -n "$line" ] || continue
+ PKG_SHELLS="$PKG_SHELLS$line#"
diff --git a/debian/patches/no-duplicates.patch
b/debian/patches/no-duplicates.patch
new file mode 100644
index 0000000..2c797de
--- /dev/null
+++ b/debian/patches/no-duplicates.patch
@@ -0,0 +1,18 @@
+Author: Andreas Beckmann <a...@debian.org>
+Description: do not create duplicate entries in /etc/shells
+
+--- a/update-shells
++++ b/update-shells
+@@ -106,8 +106,10 @@ while IFS= read -r line; do
+ if [ -z "$shell" ] ||
+ hashset_contains "$PKG_SHELLS" "$shell" ||
+ ! hashset_contains "$STATE_SHELLS" "$shell"; then
+- echo "$line" >> "$NEW_ETC_FILE"
+- ETC_SHELLS="$ETC_SHELLS$shell#"
++ if [ -z "$shell" ] || ! hashset_contains "$ETC_SHELLS"
"$shell"; then
++ echo "$line" >> "$NEW_ETC_FILE"
++ ETC_SHELLS="$ETC_SHELLS$shell#"
++ fi
+ else
+ log "removing shell $shell"
+ fi
diff --git a/debian/patches/series b/debian/patches/series
index fdbf332..d09aed9 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,4 @@
-dpkg-root
+dpkg-root.patch
+no-duplicates.patch
+manage-usr-bin-sh.patch
+fix-aliased-location.patch
