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
(Backported from commit fb7b11ebb852fa255053ecab605bc9cfe9de0603)

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 874539625452f1a0c4f166e1e8eab902bce91a70..42b940ee7fca58a668ac84a6aa36e8b8822967d9 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -1296,25 +1296,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")
@@ -1322,7 +1305,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 60a7d4719a07ee43ec75bd2dfae8cc6e2d6a54f0..473465ba6d7acdb2091a5d00a7dbabb622314241 100644 (file)
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -196,14 +196,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 213169a98375e4373654aa31b282c0d6d4602231..d020d4a7db6827decb89fa07cc6c3d6bef74f5c5 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1076,38 +1076,59 @@ setupaptarchive() {
 }
 
 signreleasefiles() {
-       local SIGNER="${1:-Joe Sixpack}"
+       local SIGNERS="${1:-Joe Sixpack}"
        local REPODIR="${2:-aptarchive}"
-       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 82b64963cd7ba5888c48e77eb1320ea90151a33f..ddb9bf9d297e95c80b1bfd69d5c3615a9152a41c 100755 (executable)
--- 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 d433baeb5aa77ceff73a537276b76cf6be4642b6..241bf383bdc3b53108ab30c897a4c4021c430975 100755 (executable)
--- 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 c349c44283b5de8d434c01b1face6b8a4ff0ee93..19d5cb9bceb3afcb1e87da802a76d05deaf3318e 100755 (executable)
--- a/test/integration/test-releasefile-verification
+++ b/test/integration/test-releasefile-verification
@@ -127,7 +127,29 @@ 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 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}"
@@ -279,11 +301,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'