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

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 317db65b87e6277ec024bfae40727129f595fa85..fde45e1fe9a8a37b7ceb5c9e1a4a04b36ffb0841 100644 (file)
--- 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")
    string Transformed = TransactionManager->MetaIndexParser->GetExpectedDist();
 
    if (Transformed == "../project/experimental")


@@ -1391,7 +1374,7 @@ bool pkgAcqMetaBase::VerifyVendor(string const &Message)          /*{{{*/
       Transformed = "experimental";
    }
 
       Transformed = "experimental";
    }
 

-   pos = Transformed.rfind('/');
+   auto pos = Transformed.rfind('/');

    if (pos != string::npos)
    {
       Transformed = Transformed.substr(0, pos);
    if (pos != string::npos)
    {
       Transformed = Transformed.substr(0, pos);

diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 53c3ff80e2c59287be8c082e9794bac51e6cfd36..9099521bd78fa219b80b401f8af972591c870c6a 100644 (file)
--- 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)
       {
       }
       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)
          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)
       {
       }
       else if (strncmp(buffer, GNUPGVALIDSIG, sizeof(GNUPGVALIDSIG)-1) == 0)
       {

diff --git a/test/integration/framework b/test/integration/framework
index a5cc842bab0ef9ef37984ef9fb838a47f549c7d9..7eaa36415b96ca0950355c4a8262ad156ae5b934 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1082,40 +1082,61 @@ setupaptarchive() {
 }
 
 signreleasefiles() {
 }
 
 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 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"
        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
                        fi

-                       cp "${REXKEY}.sec" "$SECUNEXPIRED"
-                       cp "${REXKEY}.pub" "$PUBUNEXPIRED"

                fi
                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
        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
        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#')"
                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
                # 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/
 }
        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
 
 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
 
                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
 
        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
                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() {
 }
 
 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)"
            $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
     fi
 
     # ensure no leftovers in partial

diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification
index a061832b632dc2e3f0ad902bf85b8bbd42380653..5da0a829254fdc6cf89c354fe3211c62e24253a9 100755 (executable)
--- a/test/integration/test-releasefile-verification
+++ b/test/integration/test-releasefile-verification
@@ -127,7 +127,7 @@ runtest() {
        testsuccessequal "$(cat "${PKGFILE}")
 " aptcache show apt
        failaptold
        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
 
        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
 
                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
        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
 
 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
 }
 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'
        testsuccess grep 'uses weak digest algorithm' rootdir/tmp/testwarning.output
 }
 runtest3 'Weak'