From 592af451936b4be9c2d586c25467460ac0b70d4f Mon Sep 17 00:00:00 2001 From: Robert Fairley Date: Wed, 15 Apr 2020 03:42:16 -0400 Subject: [PATCH 1/2] motdgen: do not share a staged file, use mktemp and mv With the `staged` file shared, there would be potential for two or more processes executing `motdgen` to write to it resulting in corrupted output, or the error in the `cat` command due to missing file reported in #35 (comment). Currently, this is not a problem with motdgen, but could be if `motdgen` were invoked by something like the udev rules that invoke `issuegen`. A bug reported in `issuegen` for this reason is: https://github.com/coreos/console-login-helper-messages/issues/35 Instead, write the intermediate output to a unique tempfile and mv the tempfile to the final output location. If the final output is on the same filesystem as the tempfile, this operation should be atomic. This ensures only valid output is written to the issue file shown to the terminal. Additionally, perform code tidyups similar to those done for `issuegen` in https://github.com/coreos/console-login-helper-messages/pull/40. --- .../console-login-helper-messages/libutil.sh | 46 +++++++++++++++++++ ...sole-login-helper-messages-motdgen.service | 2 +- .../console-login-helper-messages/motdgen | 46 +++++++++---------- 3 files changed, 68 insertions(+), 26 deletions(-) create mode 100644 usr/lib/console-login-helper-messages/libutil.sh diff --git a/usr/lib/console-login-helper-messages/libutil.sh b/usr/lib/console-login-helper-messages/libutil.sh new file mode 100644 index 00000000..24d24d3b --- /dev/null +++ b/usr/lib/console-login-helper-messages/libutil.sh @@ -0,0 +1,46 @@ +#!/usr/bin/bash +# +# Collection of util functions and common definitions for +# console-login-helper-messages scripts. + +PKG_NAME="console-login-helper-messages" + +tempfile_template="${PKG_NAME}.XXXXXXXXXX.tmp" +# Use same filesystem, under /run, as where snippets are generated, so +# that rename operations through `mv` are atomic. +tempfile_dir="/run/${PKG_NAME}" +# Default SELinux context at destination is applied, e.g. for sshd which +# requires that written files in `/run/motd.d` maintain the type +# `pam_var_run_t`. +mv_Z="mv -Z" + +# Write stdin to a tempfile, and rename the tempfile to the path given +# as an argument. When called from multiple processes on the same +# generated file path, this avoids interleaving writes to the generated +# file by using `mv` to overwrite the file. +write_via_tempfile() { + local generated_file="$1" + local staged_file="$(mktemp --tmpdir="${tempfile_dir}" "${tempfile_template}")" + cat > "${staged_file}" + ${mv_Z} "${staged_file}" "${generated_file}" +} + +# Write concatenation of all files with a given suffix from a list of +# source directories to a target file. The target file is the first +# argument; suffix the second; and source directories the remaining, +# searched in the given order in the list. Atomicity of the write to +# the target file is given by appending file contents to a tempfile +# before moving to the target file. +cat_via_tempfile() { + local generated_file="$1" + local filter_suffix="$2" + shift 2 + local source_dirs="$@" + local staged_file="$(mktemp --tmpdir="${tempfile_dir}" "${tempfile_template}")" + for source_dir in ${source_dirs[@]}; do + # Ignore stderr, and let the command succeed if no files are + # found in the source directory. + cat "${source_dir}"/*"$filter_suffix" 2>/dev/null >> "${staged_file}" || : + done + ${mv_Z} "${staged_file}" "${generated_file}" +} diff --git a/usr/lib/systemd/system/console-login-helper-messages-motdgen.service b/usr/lib/systemd/system/console-login-helper-messages-motdgen.service index 4a028813..60d89083 100644 --- a/usr/lib/systemd/system/console-login-helper-messages-motdgen.service +++ b/usr/lib/systemd/system/console-login-helper-messages-motdgen.service @@ -1,5 +1,5 @@ [Unit] -Description=Generate /run/motd.d/console-login-helper-messages.motd +Description=Generate console-login-helper-messages motd snippet Documentation=https://github.com/coreos/console-login-helper-messages Before=systemd-user-sessions.service diff --git a/usr/libexec/console-login-helper-messages/motdgen b/usr/libexec/console-login-helper-messages/motdgen index f5ca2b8e..1e699332 100755 --- a/usr/libexec/console-login-helper-messages/motdgen +++ b/usr/libexec/console-login-helper-messages/motdgen @@ -9,35 +9,31 @@ # Get updated system information and generate a motd # to display this at login. +. /usr/lib/console-login-helper-messages/libutil.sh + set -e PKG_NAME=console-login-helper-messages -MOTD_DIR_PUBLIC=motd.d -# Should only be read by this script. -MOTD_DIR_PRIVATE="${PKG_NAME}/motd.d" +MOTD_SNIPPETS_PATH="${PKG_NAME}/motd.d" +ETC_SNIPPETS="/etc/${MOTD_SNIPPETS_PATH}" +RUN_SNIPPETS="/run/${MOTD_SNIPPETS_PATH}" +USR_LIB_SNIPPETS="/usr/lib/${MOTD_SNIPPETS_PATH}" -staged="/run/${PKG_NAME}/40_${PKG_NAME}.motd.staged" -# Pick 40 as an index as other files can order around it easily. -generated="/run/motd.d/40_${PKG_NAME}.motd" +# Parts of this script write to the `${RUN_SNIPPETS}` directory, +# make sure it is created upfront. +mkdir -p "${RUN_SNIPPETS}" -mkdir -p "/run/${MOTD_DIR_PRIVATE}" -mkdir -p "/run/${MOTD_DIR_PUBLIC}" -rm -f "${generated}" +# Write distro release information to MOTD, in distro-specific color. source /usr/lib/os-release -echo -e "\e[${ANSI_COLOR}m${PRETTY_NAME}\e[39m" > "/run/${MOTD_DIR_PRIVATE}/21_os_release.motd" - -# Generate a motd from files found in the private (package-specific) directories, -# and place the motd in a public directory. -if [[ -d "/etc/${MOTD_DIR_PRIVATE}" ]]; then - cat /etc/${MOTD_DIR_PRIVATE}/* 2>/dev/null >> "${staged}" || true -fi -if [[ -d /run/"${MOTD_DIR_PRIVATE}" ]]; then - cat /run/${MOTD_DIR_PRIVATE}/* 2>/dev/null >> "${staged}" || true -fi -if [[ -d /usr/lib/"${MOTD_DIR_PRIVATE}" ]]; then - cat /usr/lib/${MOTD_DIR_PRIVATE}/* 2>/dev/null >> "${staged}" || true -fi - -cat "${staged}" > "${generated}" -rm -rf "${staged}" +OS_RELEASE_OUTDIR="${RUN_SNIPPETS}" +echo -e "\e[${ANSI_COLOR}m${PRETTY_NAME}\e[39m" \ + | write_via_tempfile "${OS_RELEASE_OUTDIR}/21_os_release.motd" + + +# Generate a final motd from compiling the snippets. +# Pick 40 as a prefix as other files can order around it easily. +generated_file="/run/motd.d/40_${PKG_NAME}.motd" +source_dirs=("${ETC_SNIPPETS}" "${RUN_SNIPPETS}" "${USR_LIB_SNIPPETS}") + +cat_via_tempfile "${generated_file}" ".motd" "${source_dirs[@]}" From 8fedbd386d8e456e0ec2e28d4b55f3600ca0a2d5 Mon Sep 17 00:00:00 2001 From: Robert Fairley Date: Wed, 1 Jul 2020 13:45:06 -0400 Subject: [PATCH 2/2] issuegen: use mktemp and mv when writing files Similar to `motdgen` (#44), use `mktemp` to create a uniquely-named tmpfile file to stage changes and then `mv` the file to the target location. This ensures no interleaving of writes when two processes are running `issuegen`. See: #35 --- .../gensnippet_udev_if | 4 +++- usr/libexec/console-login-helper-messages/issuegen | 13 ++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/usr/libexec/console-login-helper-messages/gensnippet_udev_if b/usr/libexec/console-login-helper-messages/gensnippet_udev_if index db0ae5bc..8c84cc97 100755 --- a/usr/libexec/console-login-helper-messages/gensnippet_udev_if +++ b/usr/libexec/console-login-helper-messages/gensnippet_udev_if @@ -11,13 +11,15 @@ set -e +. /usr/lib/console-login-helper-messages/libutil.sh . /usr/libexec/console-login-helper-messages/issuegen.defs # Add/remove data from udev rules. outfile="${RUN_SNIPPETS}/22_${INTERFACE}.issue" case "${ACTION}" in add) - echo "${INTERFACE}: \\4{${INTERFACE}} \\6{${INTERFACE}}" > ${outfile} + echo "${INTERFACE}: \\4{${INTERFACE}} \\6{${INTERFACE}}" \ + | write_via_tempfile ${outfile} ;; remove) rm -f ${outfile} diff --git a/usr/libexec/console-login-helper-messages/issuegen b/usr/libexec/console-login-helper-messages/issuegen index ff2940e7..9de6a1f8 100755 --- a/usr/libexec/console-login-helper-messages/issuegen +++ b/usr/libexec/console-login-helper-messages/issuegen @@ -13,6 +13,7 @@ set -e +. /usr/lib/console-login-helper-messages/libutil.sh . /usr/libexec/console-login-helper-messages/issuegen.defs # Provide key fingerprints via issue. @@ -22,16 +23,14 @@ mkdir -p "${SSH_DIR}" SSH_KEY_OUTDIR="${RUN_SNIPPETS}" for KEY_FILE in $(find "${SSH_DIR}" -name 'ssh_host_*_key') ; do ssh-keygen -l -f "${KEY_FILE}" -done | awk '{print "SSH host key: " $2 " " $4}' > "${SSH_KEY_OUTDIR}/21_ssh_host_keys.issue" +done \ + | awk '{print "SSH host key: " $2 " " $4}' \ + | write_via_tempfile "${SSH_KEY_OUTDIR}/21_ssh_host_keys.issue" # Generate a final issue message from compiling the snippets. # Pick 40 as a prefix as other files can order around it easily. generated_file="/run/${PKG_NAME}/40_${PKG_NAME}.issue" -generated_string='' -# Hack around files potentially not existing in the below paths with `|| true`. -generated_string+=$(cat ${ETC_SNIPPETS}/* 2>/dev/null || true) -generated_string+=$(cat ${RUN_SNIPPETS}/* 2>/dev/null || true) -generated_string+=$(cat ${USR_LIB_SNIPPETS}/* 2>/dev/null || true) +source_dirs=("${ETC_SNIPPETS}" "${RUN_SNIPPETS}" "${USR_LIB_SNIPPETS}") -echo "${generated_string}" > "${generated_file}" +cat_via_tempfile "${generated_file}" ".issue" "${source_dirs[@]}"