From 8375d5b58038fc026098dcccc3de87cd9d740334 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 17 Aug 2018 16:33:41 +0200 Subject: Support multiple keyrings in sources.list Signed-By A user can specify multiple fingerprints for a while now, so its seems counter-intuitive to support only one keyring, especially if this isn't really checked or enforced and while unlikely mixtures of both should work properly, too, instead of a kinda random behaviour. --- apt-pkg/contrib/gpgv.cc | 11 +- cmdline/apt-key.in | 157 ++++++++++++++----------- doc/sources.list.5.xml | 29 +++-- methods/gpgv.cc | 40 +++++-- test/integration/test-apt-key | 16 +++ test/integration/test-method-gpgv | 16 ++- test/integration/test-releasefile-verification | 31 +++-- 7 files changed, 194 insertions(+), 106 deletions(-) diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index cc1fbc5aa..f8ab8d715 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -108,17 +108,20 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, Args.push_back(aptkey.c_str()); Args.push_back("--quiet"); Args.push_back("--readonly"); - if (key.empty() == false) + auto const keysFileFpr = VectorizeString(key, ','); + for (auto const &k: keysFileFpr) { - if (key[0] == '/') + if (unlikely(k.empty())) + continue; + if (k[0] == '/') { Args.push_back("--keyring"); - Args.push_back(key.c_str()); + Args.push_back(k.c_str()); } else { Args.push_back("--keyid"); - Args.push_back(key.c_str()); + Args.push_back(k.c_str()); } } Args.push_back("verify"); diff --git a/cmdline/apt-key.in b/cmdline/apt-key.in index 7ec1b034c..e9187b423 100644 --- a/cmdline/apt-key.in +++ b/cmdline/apt-key.in @@ -15,6 +15,74 @@ eval "$(apt-config shell ARCHIVE_KEYRING_URI APT::Key::ArchiveKeyringURI)" aptkey_echo() { echo "$@"; } +find_gpgv_status_fd() { + while [ -n "$1" ]; do + if [ "$1" = '--status-fd' ]; then + shift + echo "$1" + break + fi + shift + done +} +GPGSTATUSFD="$(find_gpgv_status_fd "$@")" + +apt_warn() { + if [ -z "$GPGHOMEDIR" ]; then + echo >&2 'W:' "$@" + else + echo 'W:' "$@" > "${GPGHOMEDIR}/aptwarnings.log" + fi + if [ -n "$GPGSTATUSFD" ]; then + echo >&${GPGSTATUSFD} '[APTKEY:] WARNING' "$@" + fi +} +apt_error() { + if [ -z "$GPGHOMEDIR" ]; then + echo >&2 'E:' "$@" + else + echo 'E:' "$@" > "${GPGHOMEDIR}/aptwarnings.log" + fi + if [ -n "$GPGSTATUSFD" ]; then + echo >&${GPGSTATUSFD} '[APTKEY:] ERROR' "$@" + fi +} + +cleanup_gpg_home() { + if [ -z "$GPGHOMEDIR" ]; then return; fi + if [ -s "$GPGHOMEDIR/aptwarnings.log" ]; then + cat >&2 "$GPGHOMEDIR/aptwarnings.log" + fi + if command_available 'gpgconf'; then + GNUPGHOME="${GPGHOMEDIR}" gpgconf --kill all >/dev/null 2>&1 || true + fi + rm -rf "$GPGHOMEDIR" +} + +# gpg needs (in different versions more or less) files to function correctly, +# so we give it its own homedir and generate some valid content for it later on +create_gpg_home() { + # for cases in which we want to cache a homedir due to expensive setup + if [ -n "$GPGHOMEDIR" ]; then + return + fi + if [ -n "$TMPDIR" ]; then + # tmpdir is a directory and current user has rwx access to it + # same tests as in apt-pkg/contrib/fileutl.cc GetTempDir() + if [ ! -d "$TMPDIR" ] || [ ! -r "$TMPDIR" ] || [ ! -w "$TMPDIR" ] || [ ! -x "$TMPDIR" ]; then + unset TMPDIR + fi + fi + GPGHOMEDIR="$(mktemp --directory --tmpdir 'apt-key-gpghome.XXXXXXXXXX')" + CURRENTTRAP="${CURRENTTRAP} cleanup_gpg_home;" + trap "${CURRENTTRAP}" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM + if [ -z "$GPGHOMEDIR" ]; then + apt_error "Could not create temporary gpg home directory in $TMPDIR (wrong permissions?)" + exit 28 + fi + chmod 700 "$GPGHOMEDIR" +} + requires_root() { if [ "$(id -u)" -ne 0 ]; then apt_error "This command can only be used by root." @@ -282,7 +350,7 @@ foreach_keyring_do() { shift # if a --keyring was given, just work on this one if [ -n "$FORCED_KEYRING" ]; then - $ACTION "$TRUSTEDFILE" "$@" + $ACTION "$FORCED_KEYRING" "$@" else # otherwise all known keyrings are up for inspection if accessible_file_exists "$TRUSTEDFILE" && is_supported_keyring "$TRUSTEDFILE"; then @@ -525,11 +593,26 @@ while [ -n "$1" ]; do case "$1" in --keyring) shift - TRUSTEDFILE="$1" - FORCED_KEYRING="$1" + if [ -z "$FORCED_KEYRING" -o "$FORCED_KEYRING" = '/dev/null' ]; then + TRUSTEDFILE="$1" + FORCED_KEYRING="$1" + elif [ "$TRUSTEDFILE" = "$FORCED_KEYRING" ]; then + create_gpg_home + FORCED_KEYRING="${GPGHOMEDIR}/mergedkeyrings.gpg" + echo -n '' > "$FORCED_KEYRING" + chmod 0644 -- "$FORCED_KEYRING" + catfile "$TRUSTEDFILE" "$FORCED_KEYRING" + catfile "$1" "$FORCED_KEYRING" + else + catfile "$1" "$FORCED_KEYRING" + fi ;; --keyid) shift + if [ -n "$FORCED_KEYID" ]; then + apt_error 'Specifying --keyid multiple times is not supported' + exit 1 + fi FORCED_KEYID="$1" ;; --secret-keyring) @@ -582,74 +665,6 @@ if [ -z "$command" ]; then fi shift -find_gpgv_status_fd() { - while [ -n "$1" ]; do - if [ "$1" = '--status-fd' ]; then - shift - echo "$1" - break - fi - shift - done -} -GPGSTATUSFD="$(find_gpgv_status_fd "$@")" - -apt_warn() { - if [ -z "$GPGHOMEDIR" ]; then - echo >&2 'W:' "$@" - else - echo 'W:' "$@" > "${GPGHOMEDIR}/aptwarnings.log" - fi - if [ -n "$GPGSTATUSFD" ]; then - echo >&${GPGSTATUSFD} '[APTKEY:] WARNING' "$@" - fi -} -apt_error() { - if [ -z "$GPGHOMEDIR" ]; then - echo >&2 'E:' "$@" - else - echo 'E:' "$@" > "${GPGHOMEDIR}/aptwarnings.log" - fi - if [ -n "$GPGSTATUSFD" ]; then - echo >&${GPGSTATUSFD} '[APTKEY:] ERROR' "$@" - fi -} - -cleanup_gpg_home() { - if [ -z "$GPGHOMEDIR" ]; then return; fi - if [ -s "$GPGHOMEDIR/aptwarnings.log" ]; then - cat >&2 "$GPGHOMEDIR/aptwarnings.log" - fi - if command_available 'gpgconf'; then - GNUPGHOME="${GPGHOMEDIR}" gpgconf --kill all >/dev/null 2>&1 || true - fi - rm -rf "$GPGHOMEDIR" -} - -# gpg needs (in different versions more or less) files to function correctly, -# so we give it its own homedir and generate some valid content for it later on -create_gpg_home() { - # for cases in which we want to cache a homedir due to expensive setup - if [ -n "$GPGHOMEDIR" ]; then - return - fi - if [ -n "$TMPDIR" ]; then - # tmpdir is a directory and current user has rwx access to it - # same tests as in apt-pkg/contrib/fileutl.cc GetTempDir() - if [ ! -d "$TMPDIR" ] || [ ! -r "$TMPDIR" ] || [ ! -w "$TMPDIR" ] || [ ! -x "$TMPDIR" ]; then - unset TMPDIR - fi - fi - GPGHOMEDIR="$(mktemp --directory --tmpdir 'apt-key-gpghome.XXXXXXXXXX')" - CURRENTTRAP="${CURRENTTRAP} cleanup_gpg_home;" - trap "${CURRENTTRAP}" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM - if [ -z "$GPGHOMEDIR" ]; then - apt_error "Could not create temporary gpg home directory in $TMPDIR (wrong permissions?)" - exit 28 - fi - chmod 700 "$GPGHOMEDIR" -} - prepare_gpg_home() { # crude detection if we are called from a maintainerscript where the # package depends on gnupg or not. We accept recommends here as diff --git a/doc/sources.list.5.xml b/doc/sources.list.5.xml index 84eb527e7..eaea13ae5 100644 --- a/doc/sources.list.5.xml +++ b/doc/sources.list.5.xml @@ -14,7 +14,7 @@ &apt-email; &apt-product; - 2018-02-27T00:00:00Z + 2018-08-17T00:00:00Z @@ -294,17 +294,22 @@ deb-src [ option1=value1 option2=value2 ] uri suite [component1] [component2] [. () - is either an absolute path to a keyring file (has to be - accessible and readable for the _apt user, - so ensure everyone has read-permissions on the file) or one or - more fingerprints of keys either in the - trusted.gpg keyring or in the - keyrings in the trusted.gpg.d/ directory - (see apt-key fingerprint). If the option is - set, only the key(s) in this keyring or only the keys with these - fingerprints are used for the &apt-secure; verification of this - repository. Defaults to the value of the option with the same name - if set in the previously acquired Release file. + is an option to require a repository to pass &apt-secure; verification + with a certain set of keys rather than all trusted keys apt has configured. + It is specified as a list of absolute paths to keyring files (have to be + accessible and readable for the _apt system user, + so ensure everyone has read-permissions on the file) and fingerprints + of keys to select from these keyrings. If no keyring files are specified + the default is the trusted.gpg keyring and + all keyrings in the trusted.gpg.d/ directory + (see apt-key fingerprint). If no fingerprint is + specified all keys in the keyrings are selected. A fingerprint will + accept also all signatures by a subkey of this key, if this isn't + desired an exclamation mark (!) can be appended to + the fingerprint to disable this behaviour. + The option defaults to the value of the option with the same name + if set in the previously acquired Release file + of this repository (only fingerprints can be specified there through). Otherwise all keys in the trusted keyrings are considered valid signers for this repository. diff --git a/methods/gpgv.cc b/methods/gpgv.cc index e2a20722b..9135b49c5 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -106,7 +106,8 @@ class GPGVMethod : public aptMethod { private: string VerifyGetSigners(const char *file, const char *outfile, - std::string const &key, + vector const &keyFpts, + vector const &keyFiles, vector &GoodSigners, vector &BadSigners, vector &WorthlessSigners, @@ -146,7 +147,8 @@ static void PushEntryWithUID(std::vector &Signers, char * const buf Signers.push_back(msg); } string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, - std::string const &key, + vector const &keyFpts, + vector const &keyFiles, vector &GoodSigners, vector &BadSigners, vector &WorthlessSigners, @@ -159,7 +161,6 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, std::clog << "inside VerifyGetSigners" << std::endl; int fd[2]; - bool const keyIsID = (key.empty() == false && key[0] != '/'); if (pipe(fd) < 0) return "Couldn't create pipe"; @@ -168,7 +169,15 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, if (pid < 0) return string("Couldn't spawn new process") + strerror(errno); else if (pid == 0) - ExecGPGV(outfile, file, 3, fd, (keyIsID ? "" : key)); + { + std::ostringstream keys; + if (keyFiles.empty() == false) + { + std::copy(keyFiles.begin(), keyFiles.end()-1, std::ostream_iterator(keys, ",")); + keys << *keyFiles.rbegin(); + } + ExecGPGV(outfile, file, 3, fd, keys.str()); + } close(fd[1]); FILE *pipein = fdopen(fd[0], "r"); @@ -259,18 +268,21 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, // apt-key has a --keyid parameter, but this requires gpg, so we call it without it // and instead check after the fact which keyids where used for verification - if (keyIsID == true) + if (keyFpts.empty() == false) { if (Debug == true) - std::clog << "GoodSigs needs to be limited to keyid(s) " << key << std::endl; - auto const limitedTo = VectorizeString(key, ','); + { + std::clog << "GoodSigs needs to be limited to keyid(s): "; + std::copy(keyFpts.begin(), keyFpts.end(), std::ostream_iterator(std::clog, ", ")); + std::clog << "\n"; + } std::vector filteredGood; for (auto &&good: GoodSigners) { if (Debug == true) std::clog << "Key " << good << " is good sig, is it also a valid and allowed one? "; bool found = false; - for (auto l : limitedTo) + for (auto l : keyFpts) { bool exactKey = false; if (APT::String::Endswith(l, "!")) @@ -353,7 +365,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, } else if (WEXITSTATUS(status) == 0) { - if (keyIsID) + if (keyFpts.empty() == false) { // gpgv will report success, but we want to enforce a certain keyring // so if we haven't found the key the valid we found is in fact invalid @@ -379,7 +391,6 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm) { URI const Get = Itm->Uri; string const Path = Get.Host + Get.Path; // To account for relative paths - std::string const key = LookupTag(Message, "Signed-By"); vector GoodSigners; vector BadSigners; // a worthless signature is a expired or revoked one @@ -391,8 +402,15 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm) Res.Filename = Itm->DestFile; URIStart(Res); + std::vector keyFpts, keyFiles; + for (auto &&key : VectorizeString(LookupTag(Message, "Signed-By"), ',')) + if (key.empty() == false && key[0] == '/') + keyFiles.emplace_back(std::move(key)); + else + keyFpts.emplace_back(std::move(key)); + // Run apt-key on file, extract contents and get the key ID of the signer - string const msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), key, + string const msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), keyFpts, keyFiles, GoodSigners, BadSigners, WorthlessSigners, SoonWorthlessSigners, NoPubKeySigners); if (_error->PendingError()) diff --git a/test/integration/test-apt-key b/test/integration/test-apt-key index d690a9026..a1e633ca3 100755 --- a/test/integration/test-apt-key +++ b/test/integration/test-apt-key @@ -89,6 +89,14 @@ gpg: unchanged: 1' aptkey --fakeroot update testsuccess test -s "${TMPWORKINGDIRECTORY}/aptkey.export" testsuccess test -s "${TMPWORKINGDIRECTORY}/aptkey.exportall" + msgtest 'Check that multiple keys can be' 'exported' + aptkey export 'Sixpack' 'Expired' > "${TMPWORKINGDIRECTORY}/aptkey.export" 2>/dev/null + aptkey --keyring "${KEYDIR}/rexexpired.pub.${EXT}" \ + --keyring "${ROOTDIR}/etc/apt/trusted.gpg.d/joesixpack.${EXT}" exportall > "${TMPWORKINGDIRECTORY}/aptkey.exportall" + testsuccess --nomsg cmp "${TMPWORKINGDIRECTORY}/aptkey.export" "${TMPWORKINGDIRECTORY}/aptkey.exportall" + testsuccess test -s "${TMPWORKINGDIRECTORY}/aptkey.export" + testsuccess test -s "${TMPWORKINGDIRECTORY}/aptkey.exportall" + msgtest 'Execute update again to trigger removal of' 'Rex Expired key' ${TESTSTATE} --nomsg aptkey --fakeroot update @@ -274,6 +282,14 @@ gpg: unchanged: 1' aptkey --fakeroot update msgtest 'Test verify a file' 'with good keyring' testsuccess --nomsg aptkey --quiet --readonly --keyring "${KEYDIR}/testcase-multikey.pub.${EXT}" verify "${SIGNATURE}.gpg" "${SIGNATURE}" + msgtest 'Test verify a file' 'with good keyrings 1' + testsuccess --nomsg aptkey --quiet --readonly --keyring "${KEYDIR}/joesixpack.pub.${EXT}" \ + --keyring "${KEYDIR}/marvinparanoid.pub.${EXT}" verify "${SIGNATURE}.gpg" "${SIGNATURE}" + + msgtest 'Test verify a file' 'with good keyrings 2' + testsuccess --nomsg aptkey --quiet --readonly --keyring "${KEYDIR}/marvinparanoid.pub.${EXT}" \ + --keyring "${KEYDIR}/joesixpack.pub.${EXT}" verify "${SIGNATURE}.gpg" "${SIGNATURE}" + msgtest 'Test fail verify a file' 'with bad keyring' testfailure --nomsg aptkey --quiet --readonly --keyring "${KEYDIR}/joesixpack.pub.${EXT}" verify "${SIGNATURE}.gpg" "${SIGNATURE}" diff --git a/test/integration/test-method-gpgv b/test/integration/test-method-gpgv index 2b53648f0..b7cf11bdc 100755 --- a/test/integration/test-method-gpgv +++ b/test/integration/test-method-gpgv @@ -92,7 +92,21 @@ Config-Item: APT::Hashes::SHA1::Weak=true 600 URI Acquire URI: file:///dev/null Filename: /dev/zero -Signed-By: 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE +Signed-By: /dev/null,34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE +' | runapt "${METHODSDIR}/gpgv" +} +testrun + +gpgvmethod() { + echo '601 Configuration +Config-Item: Debug::Acquire::gpgv=1 +Config-Item: Dir::Bin::apt-key=./faked-apt-key +Config-Item: APT::Hashes::SHA1::Weak=true + +600 URI Acquire +URI: file:///dev/null +Filename: /dev/zero +Signed-By: 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE,/dev/null ' | runapt "${METHODSDIR}/gpgv" } testrun diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification index f61d93f79..382d89ecd 100755 --- a/test/integration/test-releasefile-verification +++ b/test/integration/test-releasefile-verification @@ -233,22 +233,39 @@ runtest() { " aptcache show apt installaptnew - msgmsg 'Cold archive signed by good keyring' 'Marvin Paranoid' - prepare "${PKGFILE}" + msgmsg 'Cold archive signed by bad keyring' 'Joe Sixpack' rm -rf rootdir/var/lib/apt/lists - signreleasefiles 'Marvin Paranoid' local MARVIN="$(readlink -f keys/marvinparanoid.pub)" sed -i "s#^\(deb\(-src\)\?\) #\1 [signed-by=$MARVIN] #" rootdir/etc/apt/sources.list.d/* + updatewithwarnings '^W: .* NO_PUBKEY' + + msgmsg 'Cold archive signed by good keyring' 'Marvin Paranoid' + prepare "${PKGFILE}" + signreleasefiles 'Marvin Paranoid' + rm -rf rootdir/var/lib/apt/lists successfulaptgetupdate testsuccessequal "$(cat "${PKGFILE}") " aptcache show apt installaptold - msgmsg 'Cold archive signed by bad keyring' 'Joe Sixpack' + msgmsg 'Cold archive signed by good keyrings' 'Marvin Paranoid, Joe Sixpack' rm -rf rootdir/var/lib/apt/lists - signreleasefiles 'Joe Sixpack' - updatewithwarnings '^W: .* NO_PUBKEY' - sed -i "s#^\(deb\(-src\)\?\) \[signed-by=$MARVIN\] #\1 #" rootdir/etc/apt/sources.list.d/* + local SIXPACK="$(readlink -f keys/joesixpack.pub)" + sed -i "s# \[signed-by=[^]]\+\] # [signed-by=$MARVIN,$SIXPACK] #" rootdir/etc/apt/sources.list.d/* + successfulaptgetupdate + testsuccessequal "$(cat "${PKGFILE}") +" aptcache show apt + installaptold + + msgmsg 'Cold archive signed by good keyrings' 'Joe Sixpack, Marvin Paranoid' + rm -rf rootdir/var/lib/apt/lists + local SIXPACK="$(readlink -f keys/joesixpack.pub)" + sed -i "s# \[signed-by=[^]]\+\] # [signed-by=$SIXPACK,$MARVIN] #" rootdir/etc/apt/sources.list.d/* + successfulaptgetupdate + testsuccessequal "$(cat "${PKGFILE}") +" aptcache show apt + installaptold + sed -i "s# \[signed-by=[^]]\+\] # #" rootdir/etc/apt/sources.list.d/* local MARVIN="$(aptkey --keyring $MARVIN finger --with-colons | grep '^fpr' | cut -d':' -f 10)" msgmsg 'Cold archive signed by bad keyid' 'Joe Sixpack' -- cgit v1.2.3