From bc8f83a5afd858206efe518c31bbb1ac948a39a3 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 17 Dec 2015 17:41:11 +0100 Subject: avoid evaluating shell in paths used in apt-key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apt-key creates internally a script (since ~1.1) which it will call to avoid dealing with an array of different options in the code itself, but while writing this script it wraps the values in "", which will cause the shell to evaluate its content upon execution. To make 'use' of this either set a absolute gpg command or TMPDIR to something as interesting as: "/tmp/This is fü\$\$ing cràzy, \$(man man | head -n1 | cut -d' ' -f1)\$!" If such paths can be encountered in reality is a different question… --- cmdline/apt-key.in | 44 +++++++++++++++++++++++-------------------- test/integration/framework | 16 +++++++++++++--- test/integration/test-apt-key | 8 ++++++++ 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/cmdline/apt-key.in b/cmdline/apt-key.in index f1d021e8a..4d1079a4b 100644 --- a/cmdline/apt-key.in +++ b/cmdline/apt-key.in @@ -4,17 +4,14 @@ set -e unset GREP_OPTIONS export IFS="$(printf "\n\b")" -APT_DIR="/" -eval $(apt-config shell APT_DIR Dir) - MASTER_KEYRING='&keyring-master-filename;' -eval $(apt-config shell MASTER_KEYRING APT::Key::MasterKeyring) +eval "$(apt-config shell MASTER_KEYRING APT::Key::MasterKeyring)" ARCHIVE_KEYRING='&keyring-filename;' -eval $(apt-config shell ARCHIVE_KEYRING APT::Key::ArchiveKeyring) +eval "$(apt-config shell ARCHIVE_KEYRING APT::Key::ArchiveKeyring)" REMOVED_KEYS='&keyring-removed-filename;' -eval $(apt-config shell REMOVED_KEYS APT::Key::RemovedKeys) +eval "$(apt-config shell REMOVED_KEYS APT::Key::RemovedKeys)" ARCHIVE_KEYRING_URI='&keyring-uri;' -eval $(apt-config shell ARCHIVE_KEYRING_URI APT::Key::ArchiveKeyringURI) +eval "$(apt-config shell ARCHIVE_KEYRING_URI APT::Key::ArchiveKeyringURI)" aptkey_echo() { echo "$@"; } @@ -40,6 +37,10 @@ command_available() { return 1 } +escape_shell() { + echo "$@" | sed -e "s#'#'\"'\"'#g" +} + get_fingerprints_of_keyring() { aptkey_execute "$GPG_SH" --keyring "$1" --with-colons --fingerprint | while read publine; do # search for a public key @@ -55,8 +56,8 @@ get_fingerprints_of_keyring() { } add_keys_with_verify_against_master_keyring() { - ADD_KEYRING=$1 - MASTER=$2 + ADD_KEYRING="$1" + MASTER="$2" if [ ! -f "$ADD_KEYRING" ]; then echo >&2 "ERROR: '$ADD_KEYRING' not found" @@ -72,8 +73,8 @@ add_keys_with_verify_against_master_keyring() { # all keys that are exported must have a valid signature # from a key in the $distro-master-keyring add_keys="$(get_fingerprints_of_keyring "$ADD_KEYRING")" - all_add_keys=`aptkey_execute "$GPG_SH" --keyring "$ADD_KEYRING" --with-colons --list-keys | grep ^[ps]ub | cut -d: -f5` - master_keys=`aptkey_execute "$GPG_SH" --keyring "$MASTER" --with-colons --list-keys | grep ^pub | cut -d: -f5` + all_add_keys="$(aptkey_execute "$GPG_SH" --keyring "$ADD_KEYRING" --with-colons --list-keys | grep ^[ps]ub | cut -d: -f5)" + master_keys="$(aptkey_execute "$GPG_SH" --keyring "$MASTER" --with-colons --list-keys | grep ^pub | cut -d: -f5)" # ensure there are no colisions LP: #857472 for all_add_key in $all_add_keys; do @@ -114,6 +115,9 @@ add_keys_with_verify_against_master_keyring() { # the archive-keyring keys needs to be signed with the master key # (otherwise it does not make sense from a security POV) net_update() { + local APT_DIR='/' + eval $(apt-config shell APT_DIR Dir) + # Disabled for now as code is insecure (LP: #1013639 (and 857472, 1013128)) APT_KEY_NET_UPDATE_ENABLED="" eval $(apt-config shell APT_KEY_NET_UPDATE_ENABLED APT::Key::Net-Update-Enabled) @@ -137,7 +141,7 @@ net_update() { keyring="${APT_DIR}/var/lib/apt/keyrings/$(basename "$ARCHIVE_KEYRING_URI")" old_mtime=0 if [ -e $keyring ]; then - old_mtime=$(stat -c %Y $keyring) + old_mtime=$(stat -c %Y "$keyring") fi (cd "${APT_DIR}/var/lib/apt/keyrings"; wget --timeout=90 -q -N "$ARCHIVE_KEYRING_URI") if [ ! -e "$keyring" ]; then @@ -228,7 +232,7 @@ foreach_keyring_do() { $ACTION "$TRUSTEDFILE" "$@" fi local TRUSTEDPARTS="/etc/apt/trusted.gpg.d" - eval $(apt-config shell TRUSTEDPARTS Dir::Etc::TrustedParts/d) + eval "$(apt-config shell TRUSTEDPARTS Dir::Etc::TrustedParts/d)" if [ -d "$TRUSTEDPARTS" ]; then # strip / suffix as gpg will double-slash in that case (#665411) local STRIPPED_TRUSTEDPARTS="${TRUSTEDPARTS%/}" @@ -360,7 +364,7 @@ setup_merged_keyring() { FORCED_KEYRING="${GPGHOMEDIR}/forcedkeyid.gpg" TRUSTEDFILE="${FORCED_KEYRING}" echo "#!/bin/sh -exec sh \"${GPG}\" --keyring \"${TRUSTEDFILE}\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh" +exec sh '($(escape_shell "${GPG}")' --keyring '$(escape_shell "${TRUSTEDFILE}")' \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh" GPG="${GPGHOMEDIR}/gpg.1.sh" # ignore error as this "just" means we haven't found the forced keyid and the keyring will be empty import_keyring_into_keyring '' "$TRUSTEDFILE" "$FORCED_KEYID" || true @@ -372,12 +376,12 @@ exec sh \"${GPG}\" --keyring \"${TRUSTEDFILE}\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1. touch "${GPGHOMEDIR}/pubring.gpg" "${GPGHOMEDIR}/pubring.orig.gpg" fi echo "#!/bin/sh -exec sh \"${GPG}\" --keyring \"${GPGHOMEDIR}/pubring.gpg\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh" +exec sh '$(escape_shell "${GPG}")' --keyring '$(escape_shell "${GPGHOMEDIR}/pubring.gpg")' \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh" GPG="${GPGHOMEDIR}/gpg.1.sh" else create_new_keyring "$TRUSTEDFILE" echo "#!/bin/sh -exec sh \"${GPG}\" --keyring \"${TRUSTEDFILE}\" \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh" +exec sh '$(escape_shell "${GPG}")' --keyring '$(escape_shell "${TRUSTEDFILE}")' \"\$@\"" > "${GPGHOMEDIR}/gpg.1.sh" GPG="${GPGHOMEDIR}/gpg.1.sh" fi } @@ -479,13 +483,13 @@ create_gpg_home() { fi fi GPGHOMEDIR="$(mktemp -d)" - CURRENTTRAP="${CURRENTTRAP} rm -rf '${GPGHOMEDIR}';" + CURRENTTRAP="${CURRENTTRAP} rm -rf '$(escape_shell "${GPGHOMEDIR}")';" trap "${CURRENTTRAP}" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM chmod 700 "$GPGHOMEDIR" } prepare_gpg_home() { - eval $(apt-config shell GPG_EXE Apt::Key::gpgcommand) + eval "$(apt-config shell GPG_EXE Apt::Key::gpgcommand)" if [ -n "$GPG_EXE" ] && command_available "$GPG_EXE"; then true @@ -518,8 +522,8 @@ prepare_gpg_home() { fi # tell gpg that it shouldn't try to maintain a trustdb file echo "#!/bin/sh -exec \"${GPG_EXE}\" --ignore-time-conflict --no-options --no-default-keyring \\ - --homedir \"${GPGHOMEDIR}\" --no-auto-check-trustdb --trust-model always \"\$@\"" > "${GPGHOMEDIR}/gpg.0.sh" +exec '$(escape_shell "${GPG_EXE}")' --ignore-time-conflict --no-options --no-default-keyring \\ +--homedir '$(escape_shell "${GPGHOMEDIR}")' --no-auto-check-trustdb --trust-model always \"\$@\"" > "${GPGHOMEDIR}/gpg.0.sh" GPG_SH="${GPGHOMEDIR}/gpg.0.sh" GPG="$GPG_SH" diff --git a/test/integration/framework b/test/integration/framework index 07b1f3236..8d0c9f5c3 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -54,7 +54,7 @@ msgprintf() { printf "$START " "$1" shift while [ -n "$1" ]; do - printf "$MIDDLE " "$(echo "$1" | sed -e 's#^apt\([cfghs]\)#apt-\1#')" + printf "$MIDDLE " "$(echo "$1" | sed -e 's#^apt\([cfghks]\)#apt-\1#')" shift done fi @@ -245,13 +245,23 @@ addtrap() { trap "shellsetedetector; $CURRENTTRAP exitwithstatus;" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM } +escape_shell() { + echo "$@" | sed -e "s#'#'\"'\"'#g" +} + setupenvironment() { # privilege dropping and testing doesn't work if /tmp isn't world-writeable (as e.g. with libpam-tmpdir) if [ -n "$TMPDIR" ] && [ "$(id -u)" = '0' ] && [ "$(stat --format '%a' "$TMPDIR")" != '1777' ]; then unset TMPDIR fi TMPWORKINGDIRECTORY="$(mktemp -d)" - addtrap "cd /; rm -rf \"$TMPWORKINGDIRECTORY\";" + addtrap "cd /; rm -rf '$(escape_shell "$TMPWORKINGDIRECTORY")';" + if [ -n "$TMPDIR_ADD" ]; then + TMPWORKINGDIRECTORY="${TMPWORKINGDIRECTORY}/${TMPDIR_ADD}" + mkdir -p "$TMPWORKINGDIRECTORY" + unset TMPDIR_ADD + export TMPDIR="$TMPWORKINGDIRECTORY" + fi msgninfo "Preparing environment for ${0##*/} in ${TMPWORKINGDIRECTORY}…" mkdir -m 700 "${TMPWORKINGDIRECTORY}/downloaded" @@ -1254,7 +1264,7 @@ EOF # start with an unmounted disk mv "${CD}" "${CD}-unmounted" # we don't want the disk to be modifiable - addtrap 'prefix' "chmod -f -R +w \"$PWD/rootdir/media/cdrom/dists/\" \"$PWD/rootdir/media/cdrom-unmounted/dists/\" || true;" + addtrap 'prefix' "chmod -f -R +w '$(escape_shell "$PWD/rootdir/media/cdrom/dists/")' '$(escape_shell "$PWD/rootdir/media/cdrom-unmounted/dists/")' || true;" chmod -R 555 rootdir/media/cdrom-unmounted/dists } diff --git a/test/integration/test-apt-key b/test/integration/test-apt-key index 6a4e0d867..7a2849b4e 100755 --- a/test/integration/test-apt-key +++ b/test/integration/test-apt-key @@ -1,6 +1,13 @@ #!/bin/sh set -e +# apt-key is a shell script, so relatively prune to be effected by 'crazy' things: +# confuses config parser as there exists no way of escaping " currently. +#TMPDIR="$(mktemp -d)/This is \"fü\$\$ing cràzy\", \$(man man | head -n1 | cut -d' ' -f 1)\$!" +# gpg doesn't like | in path names – documented e.g. in the man gpg2 --agent-program +#TMPDIR="$(mktemp -d)/This is fü\$\$ing cràzy, \$(man man | head -n1 | cut -d' ' -f 1)\$!" +TMPDIR_ADD="This is fü\$\$ing cràzy, \$(apt -v)\$!" + TESTDIR="$(readlink -f "$(dirname "$0")")" . "$TESTDIR/framework" @@ -187,6 +194,7 @@ gpg: unchanged: 1' aptkey --fakeroot update echo 'Verify me. This is my signature.' > signature testsuccess --nomsg aptkey --quiet --keyring keys/marvinparanoid.pub --secret-keyring keys/marvinparanoid.sec --readonly \ adv --batch --yes --default-key 'Marvin' --armor --detach-sign --sign --output signature.gpg signature + testsuccess test -s signature.gpg -a -s signature for GPGV in '' 'gpgv' 'gpgv2'; do -- cgit v1.2.3