From 46e00c9062d09a642973e83a334483db1f310397 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 29 Apr 2016 10:16:42 +0200 Subject: support multiple fingerprints in signed-by A keyring file can include multiple keys, so its only fair for transitions and such to support multiple fingerprints as well. --- apt-pkg/deb/debmetaindex.cc | 21 ++++++++----- doc/sources.list.5.xml | 10 +++--- methods/gpgv.cc | 30 ++++++++++-------- test/integration/test-releasefile-verification | 42 +++++++++++++++++++++----- 4 files changed, 71 insertions(+), 32 deletions(-) diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc index 71b208622..5b84ea5e8 100644 --- a/apt-pkg/deb/debmetaindex.cc +++ b/apt-pkg/deb/debmetaindex.cc @@ -627,19 +627,26 @@ bool debReleaseIndex::SetSignedBy(std::string const &pSignedBy) if (SignedBy.empty() == true && pSignedBy.empty() == false) { if (pSignedBy[0] == '/') // no check for existence as we could be chrooting later or such things - ; // absolute path to a keyring file + SignedBy = pSignedBy; // absolute path to a keyring file else { // we could go all fancy and allow short/long/string matches as gpgv/apt-key does, // but fingerprints are harder to fake than the others and this option is set once, // not interactively all the time so easy to type is not really a concern. - std::string finger = pSignedBy; - finger.erase(std::remove(finger.begin(), finger.end(), ' '), finger.end()); - std::transform(finger.begin(), finger.end(), finger.begin(), ::toupper); - if (finger.length() != 40 || finger.find_first_not_of("0123456789ABCDEF") != std::string::npos) - return _error->Error(_("Invalid value set for option %s regarding source %s %s (%s)"), "Signed-By", URI.c_str(), Dist.c_str(), "not a fingerprint"); + auto fingers = VectorizeString(pSignedBy, ','); + std::transform(fingers.begin(), fingers.end(), fingers.begin(), [&](std::string finger) { + std::transform(finger.begin(), finger.end(), finger.begin(), ::toupper); + if (finger.length() != 40 || finger.find_first_not_of("0123456789ABCDEF") != std::string::npos) + { + _error->Error(_("Invalid value set for option %s regarding source %s %s (%s)"), "Signed-By", URI.c_str(), Dist.c_str(), "not a fingerprint"); + return std::string(); + } + return finger; + }); + std::stringstream os; + std::copy(fingers.begin(), fingers.end(), std::ostream_iterator(os, ",")); + SignedBy = os.str(); } - SignedBy = pSignedBy; } else if (SignedBy != pSignedBy) return _error->Error(_("Conflicting values set for option %s regarding source %s %s"), "Signed-By", URI.c_str(), Dist.c_str()); diff --git a/doc/sources.list.5.xml b/doc/sources.list.5.xml index dcf33a9ba..07455735f 100644 --- a/doc/sources.list.5.xml +++ b/doc/sources.list.5.xml @@ -284,13 +284,13 @@ 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 a - fingerprint of a key either in the - trusted.gpg keyring or in one of the + 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 key with this - fingerprint is used for the &apt-secure; verification of this + 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. 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 3e0b133a3..b9fb09a8f 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -249,25 +249,29 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, { if (Debug == true) std::clog << "GoodSigs needs to be limited to keyid " << key << std::endl; - std::vector::iterator const foundItr = std::find(ValidSigners.begin(), ValidSigners.end(), key); - bool const found = (foundItr != ValidSigners.end()); - std::copy(GoodSigners.begin(), GoodSigners.end(), std::back_insert_iterator >(NoPubKeySigners)); - if (found) + bool foundGood = false; + for (auto const &k: VectorizeString(key, ',')) { + if (std::find(ValidSigners.begin(), ValidSigners.end(), k) == ValidSigners.end()) + continue; // we look for GOODSIG here as well as an expired sig is a valid sig as well (but not a good one) - std::string const goodlongkeyid = "GOODSIG " + key.substr(24, 16); - bool const foundGood = std::find(GoodSigners.begin(), GoodSigners.end(), goodlongkeyid) != GoodSigners.end(); + std::string const goodlongkeyid = "GOODSIG " + k.substr(24, 16); + foundGood = std::find(GoodSigners.begin(), GoodSigners.end(), goodlongkeyid) != GoodSigners.end(); if (Debug == true) - std::clog << "Key " << key << " is valid sig, is " << goodlongkeyid << " also a good one? " << (foundGood ? "yes" : "no") << std::endl; + std::clog << "Key " << k << " is valid sig, is " << goodlongkeyid << " also a good one? " << (foundGood ? "yes" : "no") << std::endl; + if (foundGood == false) + continue; + std::copy(GoodSigners.begin(), GoodSigners.end(), std::back_insert_iterator >(NoPubKeySigners)); GoodSigners.clear(); - if (foundGood) - { - GoodSigners.push_back(goodlongkeyid); - NoPubKeySigners.erase(std::remove(NoPubKeySigners.begin(), NoPubKeySigners.end(), goodlongkeyid), NoPubKeySigners.end()); - } + GoodSigners.push_back(goodlongkeyid); + NoPubKeySigners.erase(std::remove(NoPubKeySigners.begin(), NoPubKeySigners.end(), goodlongkeyid), NoPubKeySigners.end()); + break; } - else + if (foundGood == false) + { + std::copy(GoodSigners.begin(), GoodSigners.end(), std::back_insert_iterator >(NoPubKeySigners)); GoodSigners.clear(); + } } int status; diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification index 5da0a8292..e2e1b5b76 100755 --- a/test/integration/test-releasefile-verification +++ b/test/integration/test-releasefile-verification @@ -33,6 +33,7 @@ prepare() { } installaptold() { + rm -rf rootdir/var/cache/apt/archives testsuccessequal "Reading package lists... Building dependency tree... Suggested packages: @@ -249,30 +250,57 @@ runtest() { signreleasefiles 'Joe Sixpack' find aptarchive/ -name "$DELETEFILE" -delete updatewithwarnings '^W: .* NO_PUBKEY' - sed -i "s#^\(deb\(-src\)\?\) \[signed-by=$MARVIN\] #\1 #" rootdir/etc/apt/sources.list.d/* + local MARVIN="$(aptkey --keyring $MARVIN finger | grep 'Key fingerprint' | cut -d'=' -f 2 | tr -d ' ')" + msgmsg 'Cold archive signed by bad keyid' 'Joe Sixpack' + rm -rf rootdir/var/lib/apt/lists + signreleasefiles 'Joe Sixpack' + find aptarchive/ -name "$DELETEFILE" -delete + sed -i "s#^\(deb\(-src\)\?\) #\1 [signed-by=$MARVIN] #" rootdir/etc/apt/sources.list.d/* + updatewithwarnings '^W: .* be verified because the public key is not available: .*' msgmsg 'Cold archive signed by good keyid' 'Marvin Paranoid' - prepare "${PKGFILE}" rm -rf rootdir/var/lib/apt/lists signreleasefiles 'Marvin Paranoid' find aptarchive/ -name "$DELETEFILE" -delete - sed -i "s#^\(deb\(-src\)\?\) #\1 [signed-by=$MARVIN] #" rootdir/etc/apt/sources.list.d/* cp keys/marvinparanoid.pub rootdir/etc/apt/trusted.gpg.d/marvinparanoid.gpg successfulaptgetupdate testsuccessequal "$(cat "${PKGFILE}") " aptcache show apt installaptold - rm -f rootdir/etc/apt/trusted.gpg.d/marvinparanoid.gpg - msgmsg 'Cold archive signed by bad keyid' 'Joe Sixpack' + msgmsg 'Cold archive signed by good keyid' 'Marvin Paranoid,Joe Sixpack' + rm -rf rootdir/var/lib/apt/lists + signreleasefiles 'Marvin Paranoid,Joe Sixpack' + find aptarchive/ -name "$DELETEFILE" -delete + successfulaptgetupdate 'NoPubKey: GOODSIG' + testsuccessequal "$(cat "${PKGFILE}") +" aptcache show apt + installaptold + + local SIXPACK="$(aptkey --keyring keys/joesixpack.pub finger | grep 'Key fingerprint' | cut -d'=' -f 2 | tr -d ' ')" + msgmsg 'Cold archive signed by good keyids' 'Joe Sixpack' rm -rf rootdir/var/lib/apt/lists signreleasefiles 'Joe Sixpack' find aptarchive/ -name "$DELETEFILE" -delete - updatewithwarnings '^W: .* be verified because the public key is not available: .*' + sed -i "s#^\(deb\(-src\)\?\) \[signed-by=$MARVIN\] #\1 [signed-by=${SIXPACK},${MARVIN}] #" rootdir/etc/apt/sources.list.d/* + successfulaptgetupdate + testsuccessequal "$(cat "${PKGFILE}") +" aptcache show apt + installaptold + + local SIXPACK="$(aptkey --keyring keys/joesixpack.pub finger | grep 'Key fingerprint' | cut -d'=' -f 2 | tr -d ' ')" + msgmsg 'Cold archive signed by good keyids' 'Joe Sixpack' + rm -rf rootdir/var/lib/apt/lists + sed -i "s#^\(deb\(-src\)\?\) \[signed-by=${SIXPACK},${MARVIN}\] #\1 [signed-by=${MARVIN},${SIXPACK}] #" rootdir/etc/apt/sources.list.d/* + successfulaptgetupdate + testsuccessequal "$(cat "${PKGFILE}") +" aptcache show apt + installaptold + rm -f rootdir/etc/apt/trusted.gpg.d/marvinparanoid.gpg + sed -i "s#^\(deb\(-src\)\?\) \[signed-by=${MARVIN},${SIXPACK}\] #\1 #" rootdir/etc/apt/sources.list.d/* - sed -i "s#^\(deb\(-src\)\?\) \[signed-by=$MARVIN\] #\1 #" rootdir/etc/apt/sources.list.d/* } runtest2() { -- cgit v1.2.3