don't show NO_PUBKEY warning if repo is signed by another key

Daniel Kahn Gillmor highlights in the bugreport that security isn't
improving by having the user import additional keys – especially as
importing keys securely is hard.

The bugreport was initially about dropping the warning to a notice, but
in given the previously mentioned observation and the fact that we
weren't printing a warning (or a notice) for expired or revoked keys
providing a signature we drop it completely as the code to display a
message if this was the only key is in another path – and is considered
critical.

Closes: 618445

6 files changed, 134 insertions, 49 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 317db65b8..fde45e1fe 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -1365,25 +1365,8 @@ void pkgAcqMetaBase::QueueIndexes(bool const verify)			/*{{{*/
    }
 }
 									/*}}}*/
-bool pkgAcqMetaBase::VerifyVendor(string const &Message)		/*{{{*/
+bool pkgAcqMetaBase::VerifyVendor(string const &)			/*{{{*/
 {
-   string::size_type pos;
-
-   // check for missing sigs (that where not fatal because otherwise we had
-   // bombed earlier)
-   string missingkeys;
-   string msg = _("There is no public key available for the "
-		  "following key IDs:\n");
-   pos = Message.find("NO_PUBKEY ");
-   if (pos != std::string::npos)
-   {
-      string::size_type start = pos+strlen("NO_PUBKEY ");
-      string Fingerprint = Message.substr(start, Message.find("\n")-start);
-      missingkeys += (Fingerprint);
-   }
-   if(!missingkeys.empty())
-      _error->Warning("%s", (msg + missingkeys).c_str());
-
    string Transformed = TransactionManager->MetaIndexParser->GetExpectedDist();
 
    if (Transformed == "../project/experimental")
@@ -1391,7 +1374,7 @@ bool pkgAcqMetaBase::VerifyVendor(string const &Message)		/*{{{*/
       Transformed = "experimental";
    }
 
-   pos = Transformed.rfind('/');
+   auto pos = Transformed.rfind('/');
    if (pos != string::npos)
    {
       Transformed = Transformed.substr(0, pos);
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 53c3ff80e..9099521bd 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -203,14 +203,14 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
       }
       else if (strncmp(buffer, GNUPGGOODSIG, sizeof(GNUPGGOODSIG)-1) == 0)
       {
-         char *sig = buffer + sizeof(GNUPGPREFIX);
-         char *p = sig + sizeof("GOODSIG");
+         char *sig = buffer + sizeof(GNUPGGOODSIG);
+         char *p = sig;
          while (*p && isxdigit(*p)) 
             p++;
          *p = 0;
          if (Debug == true)
-            std::clog << "Got GOODSIG, key ID:" << sig << std::endl;
-         GoodSigners.push_back(string(sig));
+            std::clog << "Got GOODSIG, key ID: " << sig << std::endl;
+         GoodSigners.push_back(string(buffer+sizeof(GNUPGPREFIX)));
       }
       else if (strncmp(buffer, GNUPGVALIDSIG, sizeof(GNUPGVALIDSIG)-1) == 0)
       {
diff --git a/test/integration/framework b/test/integration/framework
index a5cc842ba..7eaa36415 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1082,40 +1082,61 @@ setupaptarchive() {
 }
 
 signreleasefiles() {
-	local SIGNER="${1:-Joe Sixpack}"
+	local SIGNERS="${1:-Joe Sixpack}"
 	local REPODIR="${2:-aptarchive}"
 	if [ -n "$1" ]; then shift; fi
 	if [ -n "$1" ]; then shift; fi
-	local KEY="keys/$(echo "$SIGNER" | tr 'A-Z' 'a-z' | sed 's# ##g')"
-	local GPG="aptkey --quiet --keyring ${KEY}.pub --secret-keyring ${KEY}.sec --readonly adv --batch --yes --digest-algo ${APT_TESTS_DIGEST_ALGO:-SHA512}"
-	msgninfo "\tSign archive with $SIGNER key $KEY… "
+	local KEY="keys/$(echo "$SIGNERS" | tr 'A-Z' 'a-z' | tr -d ' ,')"
+	msgninfo "\tSign archive with $SIGNERS key $KEY… "
 	local REXKEY='keys/rexexpired'
 	local SECEXPIREBAK="${REXKEY}.sec.bak"
 	local PUBEXPIREBAK="${REXKEY}.pub.bak"
-	if [ "${SIGNER}" = 'Rex Expired' ]; then
-		# the key is expired, so gpg doesn't allow to sign with and the --faked-system-time
-		# option doesn't exist anymore (and using faketime would add a new obscure dependency)
-		# therefore we 'temporary' make the key not expired and restore a backup after signing
-		cp "${REXKEY}.sec" "$SECEXPIREBAK"
-		cp "${REXKEY}.pub" "$PUBEXPIREBAK"
-		local SECUNEXPIRED="${REXKEY}.sec.unexpired"
-		local PUBUNEXPIRED="${REXKEY}.pub.unexpired"
-		if [ -f "$SECUNEXPIRED" ] && [ -f "$PUBUNEXPIRED" ]; then
-			cp "$SECUNEXPIRED" "${REXKEY}.sec"
-			cp "$PUBUNEXPIRED" "${REXKEY}.pub"
-		else
-			if ! printf "expire\n1w\nsave\n" | $GPG --default-key "$SIGNER" --command-fd 0 --edit-key "${SIGNER}" >setexpire.gpg 2>&1; then
-				cat setexpire.gpg
-				exit 1
+	local SIGUSERS=""
+	while [ -n "${SIGNERS%%,*}" ]; do
+		local SIGNER="${SIGNERS%%,*}"
+		if [ "${SIGNERS}" = "${SIGNER}" ]; then
+			SIGNERS=""
+		fi
+		SIGNERS="${SIGNERS#*,}"
+		# FIXME: This should be the full name, but we can't encode the space properly currently
+		SIGUSERS="${SIGUSERS} -u ${SIGNER#* }"
+		if [ "${SIGNER}" = 'Rex Expired' ]; then
+			# the key is expired, so gpg doesn't allow to sign with and the --faked-system-time
+			# option doesn't exist anymore (and using faketime would add a new obscure dependency)
+			# therefore we 'temporary' make the key not expired and restore a backup after signing
+			cp "${REXKEY}.sec" "$SECEXPIREBAK"
+			cp "${REXKEY}.pub" "$PUBEXPIREBAK"
+			local SECUNEXPIRED="${REXKEY}.sec.unexpired"
+			local PUBUNEXPIRED="${REXKEY}.pub.unexpired"
+			if [ -f "$SECUNEXPIRED" ] && [ -f "$PUBUNEXPIRED" ]; then
+				cp "$SECUNEXPIRED" "${REXKEY}.sec"
+				cp "$PUBUNEXPIRED" "${REXKEY}.pub"
+			else
+				if ! printf "expire\n1w\nsave\n" | aptkey --quiet --keyring "${REXKEY}.pub" --secret-keyring "${REXKEY}.sec" \
+					--readonly adv --batch --yes --digest-algo "${APT_TESTS_DIGEST_ALGO:-SHA512}" \
+					--default-key "$SIGNER" --command-fd 0 --edit-key "${SIGNER}" >setexpire.gpg 2>&1; then
+					cat setexpire.gpg
+					exit 1
+				fi
+				cp "${REXKEY}.sec" "$SECUNEXPIRED"
+				cp "${REXKEY}.pub" "$PUBUNEXPIRED"
 			fi
-			cp "${REXKEY}.sec" "$SECUNEXPIRED"
-			cp "${REXKEY}.pub" "$PUBUNEXPIRED"
 		fi
+		if [ ! -e "${KEY}.pub" ]; then
+			local K="keys/$(echo "$SIGNER" | tr 'A-Z' 'a-z' | tr -d ' ,')"
+			cat "${K}.pub" >> "${KEY}.new.pub"
+			cat "${K}.sec" >> "${KEY}.new.sec"
+		fi
+	done
+	if [ ! -e "${KEY}.pub" ]; then
+		mv "${KEY}.new.pub" "${KEY}.pub"
+		mv "${KEY}.new.sec" "${KEY}.sec"
 	fi
+	local GPG="aptkey --quiet --keyring ${KEY}.pub --secret-keyring ${KEY}.sec --readonly adv --batch --yes --digest-algo ${APT_TESTS_DIGEST_ALGO:-SHA512}"
 	for RELEASE in $(find "${REPODIR}/" -name Release); do
-		testsuccess $GPG "$@" --default-key "$SIGNER" --armor --detach-sign --sign --output "${RELEASE}.gpg" "${RELEASE}"
+		testsuccess $GPG "$@" $SIGUSERS --armor --detach-sign --sign --output "${RELEASE}.gpg" "${RELEASE}"
 		local INRELEASE="$(echo "${RELEASE}" | sed 's#/Release$#/InRelease#')"
-		testsuccess $GPG "$@" --default-key "$SIGNER" --clearsign --output "$INRELEASE" "$RELEASE"
+		testsuccess $GPG "$@" $SIGUSERS --clearsign --output "$INRELEASE" "$RELEASE"
 		# we might have set a specific date for the Release file, so copy it
 		touch -d "$(stat --format "%y" ${RELEASE})" "${RELEASE}.gpg" "${INRELEASE}"
 	done
diff --git a/test/integration/test-apt-key b/test/integration/test-apt-key
index 82b64963c..ddb9bf9d2 100755
--- a/test/integration/test-apt-key
+++ b/test/integration/test-apt-key
@@ -19,6 +19,11 @@ cleanplate() {
 	rm -rf rootdir/etc/apt/trusted.gpg.d/ rootdir/etc/apt/trusted.gpg
 	mkdir rootdir/etc/apt/trusted.gpg.d/
 }
+testmultigpg() {
+	testfailure --nomsg aptkey --quiet --readonly "$@"
+	testsuccess grep "^gpgv: Can't check signature" rootdir/tmp/testfailure.output
+	testsuccess grep '^gpgv: Good signature from' rootdir/tmp/testfailure.output
+}
 
 echo 'APT::Key::ArchiveKeyring "./keys/joesixpack.pub";
 APT::Key::RemovedKeys "./keys/rexexpired.pub";' > rootdir/etc/apt/apt.conf.d/aptkey.conf
@@ -178,7 +183,6 @@ gpg:              unchanged: 1' aptkey --fakeroot update
 		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
 		echo "APT::Key::GPGVCommand \"$GPGV\";" > rootdir/etc/apt/apt.conf.d/00gpgvcmd
 
@@ -209,6 +213,54 @@ gpg:              unchanged: 1' aptkey --fakeroot update
 		echo 'lalalalala' > signature2
 		testfailure --nomsg aptkey --quiet --readonly verify signature.gpg signature2
 	done
+	rm -f rootdir/etc/apt/apt.conf.d/00gpgvcmd
+
+	msgtest 'Test verify a file' 'with good keyring'
+	testsuccess --nomsg aptkey --quiet --readonly --keyring keys/testcase-multikey.pub verify signature.gpg signature
+
+	cleanplate
+	cat keys/joesixpack.pub keys/marvinparanoid.pub > keys/double.pub
+	cat keys/joesixpack.sec keys/marvinparanoid.sec > keys/double.sec
+	cp -a keys/double.pub rootdir/etc/apt/trusted.gpg.d/double.gpg
+	cp -a keys/testcase-multikey.pub rootdir/etc/apt/trusted.gpg.d/multikey.gpg
+	testsuccess aptkey --quiet --keyring keys/double.pub --secret-keyring keys/double.sec --readonly \
+		adv --batch --yes -u 'Marvin' -u 'Joe' --armor --detach-sign --sign --output signature.gpg signature
+	testsuccess test -s signature.gpg -a -s signature
+
+	for GPGV in '' 'gpgv' 'gpgv2'; do
+		echo "APT::Key::GPGVCommand \"$GPGV\";" > rootdir/etc/apt/apt.conf.d/00gpgvcmd
+
+		msgtest 'Test verify a doublesigned file' 'with all keys'
+		testsuccess --nomsg aptkey --quiet --readonly verify signature.gpg signature
+
+		msgtest 'Test verify a doublesigned file' 'with good keyring joe'
+		testmultigpg --keyring keys/joesixpack.pub verify signature.gpg signature
+
+		msgtest 'Test verify a doublesigned file' 'with good keyring marvin'
+		testmultigpg --keyring keys/marvinparanoid.pub verify signature.gpg signature
+
+		msgtest 'Test fail verify a doublesigned file' 'with bad keyring'
+		testfailure --nomsg aptkey --quiet --readonly --keyring keys/rexexpired.pub verify signature.gpg signature
+
+		msgtest 'Test fail verify a doublesigned file' 'with non-existing keyring'
+		testfailure --nomsg aptkey --quiet --readonly --keyring keys/does-not-exist.pub verify signature.gpg signature
+		testfailure test -e keys/does-not-exist.pub
+
+		# note: this isn't how apts gpgv method implements keyid for verify
+		msgtest 'Test verify a doublesigned file' 'with good keyid'
+		testmultigpg --keyid 'Paranoid' verify signature.gpg signature
+
+		msgtest 'Test fail verify a doublesigned file' 'with bad keyid'
+		testfailure --nomsg aptkey --quiet --readonly --keyid 'Rex' verify signature.gpg signature
+
+		msgtest 'Test fail verify a doublesigned file' 'with non-existing keyid'
+		testfailure --nomsg aptkey --quiet --readonly --keyid 'Kalnischkies' verify signature.gpg signature
+
+		msgtest 'Test verify fails on' 'bad doublesigned file'
+		echo 'lalalalala' > signature2
+		testfailure --nomsg aptkey --quiet --readonly verify signature.gpg signature2
+	done
+	rm -f rootdir/etc/apt/apt.conf.d/00gpgvcmd
 }
 
 setupgpgcommand() {
diff --git a/test/integration/test-apt-update-ims b/test/integration/test-apt-update-ims
index d433baeb5..241bf383b 100755
--- a/test/integration/test-apt-update-ims
+++ b/test/integration/test-apt-update-ims
@@ -49,7 +49,7 @@ runtest() {
 	    $TEST aptget update -o Debug::Acquire::gpgv=1 $APTOPT
 	    cp rootdir/tmp/${TEST}.output goodsign.output
 	    testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)"
-	    testsuccess grep '^Got GOODSIG, key ID:GOODSIG' goodsign.output
+	    testsuccess grep '^Got GOODSIG, key ID:' goodsign.output
     fi
 
     # ensure no leftovers in partial
diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification
index a061832b6..5da0a8292 100755
--- a/test/integration/test-releasefile-verification
+++ b/test/integration/test-releasefile-verification
@@ -127,7 +127,7 @@ runtest() {
 	testsuccessequal "$(cat "${PKGFILE}")
 " aptcache show apt
 	failaptold
-	rm rootdir/etc/apt/trusted.gpg.d/rexexpired.gpg
+	rm -f rootdir/etc/apt/trusted.gpg.d/rexexpired.gpg
 
 	msgmsg 'Cold archive expired signed by' 'Joe Sixpack'
 	if dpkg --compare-versions "$(aptkey adv --version | head -n 2 | tail -n 1 | cut -d' ' -f 3)" '>=' '2.1' >/dev/null 2>&1; then
@@ -152,6 +152,28 @@ runtest() {
 		msgskip 'Not a new enough gpg available providing --fake-system-time'
 	fi
 
+	msgmsg 'Cold archive signed by' 'Joe Sixpack,Marvin Paranoid'
+	prepare "${PKGFILE}"
+	rm -rf rootdir/var/lib/apt/lists
+	signreleasefiles 'Joe Sixpack,Marvin Paranoid'
+	find aptarchive/ -name "$DELETEFILE" -delete
+	successfulaptgetupdate 'NO_PUBKEY'
+	testsuccessequal "$(cat "${PKGFILE}")
+" aptcache show apt
+	installaptold
+
+	msgmsg 'Cold archive signed by' 'Joe Sixpack,Rex Expired'
+	prepare "${PKGFILE}"
+	rm -rf rootdir/var/lib/apt/lists
+	signreleasefiles 'Joe Sixpack,Rex Expired'
+	find aptarchive/ -name "$DELETEFILE" -delete
+	cp keys/rexexpired.pub rootdir/etc/apt/trusted.gpg.d/rexexpired.gpg
+	successfulaptgetupdate 'EXPKEYSIG'
+	rm -f rootdir/etc/apt/trusted.gpg.d/rexexpired.gpg
+	testsuccessequal "$(cat "${PKGFILE}")
+" aptcache show apt
+	installaptold
+
 	msgmsg 'Cold archive signed by' 'Marvin Paranoid'
 	prepare "${PKGFILE}"
 	rm -rf rootdir/var/lib/apt/lists
@@ -302,11 +324,18 @@ export APT_TESTS_DIGEST_ALGO='SHA224'
 
 successfulaptgetupdate() {
 	testsuccess aptget update -o Debug::pkgAcquire::Worker=1 -o Debug::Acquire::gpgv=1
+	if [ -n "$1" ]; then
+		cp rootdir/tmp/testsuccess.output aptupdate.output
+		testsuccess grep "$1" aptupdate.output
+	fi
 }
 runtest3 'Trusted'
 
 successfulaptgetupdate() {
 	testwarning aptget update -o Debug::pkgAcquire::Worker=1 -o Debug::Acquire::gpgv=1
+	if [ -n "$1" ]; then
+		testsuccess grep "$1" rootdir/tmp/testwarning.output
+	fi
 	testsuccess grep 'uses weak digest algorithm' rootdir/tmp/testwarning.output
 }
 runtest3 'Weak'