gpgv: show always webportal error on NODATA

gpg doesn't give use a UID on NODATA, which we were "expecting" (but not
using for anything), but just an error number. Instead of collecting
these as badsigners which will trigger a "invald signature" error with
remarks like "NODATA 1" we instead adapt a message similar to the NODATA
error of a clearsigned file (which is actually not reached anymore as we
split them up, which fails with a NOSPLIT error, which uses the same
general error message).

In other words: Not a security relevant change, just a user experience
improvement as we now point them to the most likely cause of the
problem instead of saying "invalid signature" which would point them in
the direction of the archive being broken (for everyone) instead.

Closes: 823746

diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index b9fb09a8f48b6a2a12d6ab3b74e6d92fb38ed234..dd395d6598c8fb34d3327b0307810e9ac68b7c57 100644 (file)
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -173,6 +173,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
    std::vector<std::string> ErrSigners;
    size_t buffersize = 0;
    char *buffer = NULL;
+   bool gotNODATA = false;
    while (1)
    {
       if (getline(&buffer, &buffersize, pipein) == -1)
@@ -194,8 +195,8 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
         ErrSigners.erase(std::remove_if(ErrSigners.begin(), ErrSigners.end(), [&](std::string const &errsig) {
                  return errsig.compare(strlen("ERRSIG "), 16, buffer, sizeof(GNUPGNOPUBKEY), 16) == 0;  }), ErrSigners.end());
       }
-      else if (strncmp(buffer, GNUPGNODATA, sizeof(GNUPGBADSIG)-1) == 0)
-        PushEntryWithUID(BadSigners, buffer, Debug);
+      else if (strncmp(buffer, GNUPGNODATA, sizeof(GNUPGNODATA)-1) == 0)
+        gotNODATA = true;
       else if (strncmp(buffer, GNUPGEXPKEYSIG, sizeof(GNUPGEXPKEYSIG)-1) == 0)
         PushEntryWithUID(WorthlessSigners, buffer, Debug);
       else if (strncmp(buffer, GNUPGEXPSIG, sizeof(GNUPGEXPSIG)-1) == 0)
@@ -293,10 +294,26 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
       std::for_each(SoonWorthlessSigners.begin(), SoonWorthlessSigners.end(), [](Signer const &sig) { std::cerr << sig.key << ", "; });
       std::cerr << std::endl << "  NoPubKey: ";
       std::copy(NoPubKeySigners.begin(), NoPubKeySigners.end(), std::ostream_iterator<std::string>(std::cerr, ", "));
-      std::cerr << std::endl;
+      std::cerr << std::endl << "  NODATA: " << (gotNODATA ? "yes" : "no") << std::endl;
    }
 
-   if (WEXITSTATUS(status) == 0)
+   if (WEXITSTATUS(status) == 112)
+   {
+      // acquire system checks for "NODATA" to generate GPG errors (the others are only warnings)
+      std::string errmsg;
+      //TRANSLATORS: %s is a single techy word like 'NODATA'
+      strprintf(errmsg, _("Clearsigned file isn't valid, got '%s' (does the network require authentication?)"), "NODATA");
+      return errmsg;
+   }
+   else if (gotNODATA)
+   {
+      // acquire system checks for "NODATA" to generate GPG errors (the others are only warnings)
+      std::string errmsg;
+      //TRANSLATORS: %s is a single techy word like 'NODATA'
+      strprintf(errmsg, _("Signed file isn't valid, got '%s' (does the network require authentication?)"), "NODATA");
+      return errmsg;
+   }
+   else if (WEXITSTATUS(status) == 0)
    {
       if (keyIsID)
       {
@@ -316,14 +333,6 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
       return _("At least one invalid signature was encountered.");
    else if (WEXITSTATUS(status) == 111)
       return _("Could not execute 'apt-key' to verify signature (is gnupg installed?)");
-   else if (WEXITSTATUS(status) == 112)
-   {
-      // acquire system checks for "NODATA" to generate GPG errors (the others are only warnings)
-      std::string errmsg;
-      //TRANSLATORS: %s is a single techy word like 'NODATA'
-      strprintf(errmsg, _("Clearsigned file isn't valid, got '%s' (does the network require authentication?)"), "NODATA");
-      return errmsg;
-   }
    else
       return _("Unknown error executing apt-key");
 }

diff --git a/test/integration/framework b/test/integration/framework
index 1f4da97f064d6bdb59c064fcbac03682eb6676ee..ea577c04ec32c6ce1543f98a4e47cc46797e56e3 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1697,8 +1697,9 @@ testfailure() {
                local EXITCODE=$?
                if expr match "$1" '^apt.*' >/dev/null; then
                        if [ "$1" = 'aptkey' ]; then
-                               if grep -q -E " Can't check signature: " "$OUTPUT" || \
-                                       grep -q -E " BAD signature from " "$OUTPUT"; then
+                               if grep -q " Can't check signature: 
+ BAD signature from 
+ signature could not be verified" "$OUTPUT"; then
                                        msgpass
                                else
                                        msgfailoutput "run failed with exitcode ${EXITCODE}, but no signature error" "$OUTPUT" "$@"

diff --git a/test/integration/test-apt-key b/test/integration/test-apt-key
index ddb9bf9d297e95c80b1bfd69d5c3615a9152a41c..666136098a25be06f7581f534e1d4e9ee6cbcd43 100755 (executable)
--- a/test/integration/test-apt-key
+++ b/test/integration/test-apt-key
@@ -179,10 +179,14 @@ gpg:              unchanged: 1' aptkey --fakeroot update
        cp -a keys/testcase-multikey.pub rootdir/etc/apt/trusted.gpg.d/multikey.gpg
        msgtest 'Test signing a file' 'with a key'
        echo 'Verify me. This is my signature.' > signature
+       echo 'lalalalala' > signature2
        testsuccess --nomsg aptkey --quiet --keyring keys/marvinparanoid.pub --secret-keyring keys/marvinparanoid.sec --readonly \
                adv --batch --yes --default-key 'Marvin' --armor --detach-sign --sign --output signature.gpg signature
        testsuccess test -s signature.gpg -a -s signature
 
+       msgtest 'Test verify a file' 'with no sig'
+       testfailure --nomsg aptkey --quiet --readonly --keyring keys/testcase-multikey.pub verify signature signature2
+
        for GPGV in '' 'gpgv' 'gpgv2'; do
                echo "APT::Key::GPGVCommand \"$GPGV\";" > rootdir/etc/apt/apt.conf.d/00gpgvcmd
 
@@ -210,7 +214,6 @@ gpg:              unchanged: 1' aptkey --fakeroot update
                testfailure --nomsg aptkey --quiet --readonly --keyid 'Kalnischkies' verify signature.gpg signature
 
                msgtest 'Test verify fails on' 'bad file'
-               echo 'lalalalala' > signature2
                testfailure --nomsg aptkey --quiet --readonly verify signature.gpg signature2
        done
        rm -f rootdir/etc/apt/apt.conf.d/00gpgvcmd
@@ -257,7 +260,6 @@ gpg:              unchanged: 1' aptkey --fakeroot update
                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

diff --git a/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall
index 07c13434c24313da06243d1115db2fd7893dd4e4..a756b5df2c5eb8ffac074ca1295a7a5e0a3a11b4 100755 (executable)
--- a/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall
+++ b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall
@@ -13,7 +13,7 @@ insertsource 'unstable' 'unrelated' 'all' '1.0' 'stable'
 echo 'ni ni ni' > aptarchive/knights
 
 setupaptarchive
-changetowebserver -o 'aptwebserver::overwrite::.*::filename=/knights'
+changetowebserver -o 'aptwebserver::overwrite::.*InRelease::filename=/knights' -o 'aptwebserver::overwrite::.*::filename=/knights'
 
 msgtest 'Acquire test file from the webserver to check' 'overwrite'
 if downloadfile http://localhost:${APTHTTPPORT}/holygrail ./knights-talking >/dev/null; then
@@ -34,34 +34,39 @@ ensure_n_canary_strings_in_dir() {
        test "$N" = "$EXPECTED_N" && msgpass || msgfail "Expected $EXPECTED_N canaries, got $N"
 }
 
-LISTS='rootdir/var/lib/apt/lists'
-rm -rf rootdir/var/lib/apt/lists
-testfailure aptget update
-testsuccess grep '^E:.*Clearsigned file .*NOSPLIT.*' rootdir/tmp/testfailure.output
+runtests() {
+       LISTS='rootdir/var/lib/apt/lists'
+       rm -rf "$LISTS"
+       testfailure aptget update
+       testsuccess grep "$1" rootdir/tmp/testfailure.output
 
-ensure_n_canary_strings_in_dir "$LISTS" 'ni ni ni' 0
-testequal 'lock
+       ensure_n_canary_strings_in_dir "$LISTS" 'ni ni ni' 0
+       testequal 'lock
 partial' ls "$LISTS"
 
-# and again with pre-existing files with "valid data" which should remain
-for f in Release Release.gpg main_binary-amd64_Packages main_source_Sources; do
-       echo 'peng neee-wom' > "$LISTS/localhost:${APTHTTPPORT}_dists_stable_${f}"
-       chmod 644 "$LISTS/localhost:${APTHTTPPORT}_dists_stable_${f}"
-done
+       # and again with pre-existing files with "valid data" which should remain
+       for f in Release Release.gpg main_binary-amd64_Packages main_source_Sources; do
+               echo 'peng neee-wom' > "$LISTS/localhost:${APTHTTPPORT}_dists_stable_${f}"
+               chmod 644 "$LISTS/localhost:${APTHTTPPORT}_dists_stable_${f}"
+       done
 
-testfailure aptget update
-testsuccess grep '^E:.*Clearsigned file .*NOSPLIT.*' rootdir/tmp/testfailure.output
+       testfailure aptget update
+       testsuccess grep "$1" rootdir/tmp/testfailure.output
 
-ensure_n_canary_strings_in_dir "$LISTS" 'peng neee-wom' 4
-ensure_n_canary_strings_in_dir "$LISTS" 'ni ni ni' 0
+       ensure_n_canary_strings_in_dir "$LISTS" 'peng neee-wom' 4
+       ensure_n_canary_strings_in_dir "$LISTS" 'ni ni ni' 0
 
-# and now with a pre-existing InRelease file
-echo 'peng neee-wom' > "$LISTS/localhost:${APTHTTPPORT}_dists_stable_InRelease"
-chmod 644 "$LISTS/localhost:${APTHTTPPORT}_dists_stable_InRelease"
-rm -f "$LISTS/localhost:${APTHTTPPORT}_dists_stable_Release" "$LISTS/localhost:${APTHTTPPORT}_dists_stable_Release.gpg"
-msgtest 'excpected failure of' 'apt-get update'
-testfailure aptget update
-testsuccess grep '^E:.*Clearsigned file .*NOSPLIT.*' rootdir/tmp/testfailure.output
+       # and now with a pre-existing InRelease file
+       echo 'peng neee-wom' > "$LISTS/localhost:${APTHTTPPORT}_dists_stable_InRelease"
+       chmod 644 "$LISTS/localhost:${APTHTTPPORT}_dists_stable_InRelease"
+       rm -f "$LISTS/localhost:${APTHTTPPORT}_dists_stable_Release" "$LISTS/localhost:${APTHTTPPORT}_dists_stable_Release.gpg"
+       msgtest 'excpected failure of' 'apt-get update'
+       testfailure aptget update
+       testsuccess grep "$1" rootdir/tmp/testfailure.output
 
-ensure_n_canary_strings_in_dir "$LISTS" 'peng neee-wom' 3
-ensure_n_canary_strings_in_dir "$LISTS" 'ni ni ni' 0
+       ensure_n_canary_strings_in_dir "$LISTS" 'peng neee-wom' 3
+       ensure_n_canary_strings_in_dir "$LISTS" 'ni ni ni' 0
+}
+runtests '^E:.*Clearsigned file .*NOSPLIT.*'
+webserverconfig 'aptwebserver::overwrite::.*InRelease::filename' '/404'
+runtests '^E:.*Signed file .*NODATA.*'