trigger NODATA error for invalid InRelease files

With the selfgrown splitting we got the problem of not recovering
from networks which just reply with invalid data like those sending
us login pages to authenticate with the network (e.g. hotels) back.

The good thing about the InRelease file is that we know that it must
be clearsigned (a Release file might or might not have a detached sig)
so if we get a file but are unable to split it something is seriously
wrong, so there is not much point in trying further.

The Acquire system already looks out for a NODATA error from gpgv,
so this adds a new error message sent to the acquire system in case
the splitting we do now ourselves failed including this magic word.

Closes: #712486

7 files changed, 114 insertions, 13 deletions

diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc
index 31db7d5fe..f47e7ea48 100644
--- a/apt-pkg/contrib/gpgv.cc
+++ b/apt-pkg/contrib/gpgv.cc
@@ -154,7 +154,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
 	 if (sigFd != -1)
 	    unlink(data);
 	 ioprintf(std::cerr, "Splitting up %s into data and signature failed", File.c_str());
-	 exit(EINTERNAL);
+	 exit(112);
       }
       Args.push_back(sig);
       Args.push_back(data);
diff --git a/apt-pkg/contrib/gpgv.h b/apt-pkg/contrib/gpgv.h
index 08b10a97a..45f069058 100644
--- a/apt-pkg/contrib/gpgv.h
+++ b/apt-pkg/contrib/gpgv.h
@@ -23,9 +23,18 @@
 /** \brief generates and run the command to verify a file with gpgv
  *
  * If File and FileSig specify the same file it is assumed that we
- * deal with a clear-signed message. In that case the file will be
- * rewritten to be in a good-known format without uneeded whitespaces
- * and additional messages (unsigned or signed).
+ * deal with a clear-signed message. Note that the method will accept
+ * and validate files which include additional (unsigned) messages
+ * without complaining. Do NOT open files accepted by this method
+ * for reading. Use #OpenMaybeClearSignedFile to access the message
+ * instead to ensure you are only reading signed data.
+ *
+ * The method does not return, but has some noteable exit-codes:
+ * 111 signals an internal error like the inability to execute gpgv,
+ * 112 indicates a clear-signed file which doesn't include a message,
+ *  which can happen if APT is run while on a network requiring
+ *  authentication before usage (e.g. in hotels)
+ * All other exit-codes are passed-through from gpgv.
  *
  * @param File is the message (unsigned or clear-signed)
  * @param FileSig is the signature (detached or clear-signed)
diff --git a/debian/changelog b/debian/changelog
index 91d4ae536..bd25df1e2 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -20,6 +20,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low
   * try defaults if auto-detection failed in apt-cdrom (Closes: #712433)
   * support \n and \r\n line endings in ReadMessages
   * do not redownload unchanged InRelease files
+  * trigger NODATA error for invalid InRelease files (Closes: #712486)
 
  -- David Kalnischkies <kalnischkies@gmail.com>  Sun, 09 Jun 2013 15:06:24 +0200
 
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 3f814b9f0..fe8bac6c9 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -55,9 +55,6 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
 					 vector<string> &NoPubKeySigners)
 {
    bool const Debug = _config->FindB("Debug::Acquire::gpgv", false);
-   // setup a (empty) stringstream for formating the return value
-   std::stringstream ret;
-   ret.str("");
 
    if (Debug == true)
       std::clog << "inside VerifyGetSigners" << std::endl;
@@ -170,18 +167,19 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
       return "";
    }
    else if (WEXITSTATUS(status) == 1)
-   {
       return _("At least one invalid signature was encountered.");
-   }
    else if (WEXITSTATUS(status) == 111)
+      return _("Could not execute 'gpgv' to verify signature (is gpgv installed?)");
+   else if (WEXITSTATUS(status) == 112)
    {
-      ioprintf(ret, _("Could not execute 'gpgv' to verify signature (is gpgv installed?)"));
-      return ret.str();
+      // 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 gpgv");
-   }
 }
 
 bool GPGVMethod::Fetch(FetchItem *Itm)
diff --git a/test/integration/framework b/test/integration/framework
index 3f11ac23b..3a02cfb76 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -118,6 +118,9 @@ gdb() {
 	echo "gdb: run »$*«"
 	APT_CONFIG=aptconfig.conf LD_LIBRARY_PATH=${BUILDDIRECTORY} $(which gdb) ${BUILDDIRECTORY}/$1
 }
+http() {
+	LD_LIBRARY_PATH=${BUILDDIRECTORY} ${BUILDDIRECTORY}/methods/http
+}
 
 exitwithstatus() {
         # error if we about to overflow, but ...
diff --git a/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall
new file mode 100755
index 000000000..1576c396c
--- /dev/null
+++ b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall
@@ -0,0 +1,64 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+
+setupenvironment
+configarchitecture 'native'
+
+insertpackage 'unstable' 'unrelated' 'all' '1.0' 'stable'
+insertsource 'unstable' 'unrelated' 'all' '1.0' 'stable'
+
+echo 'ni ni ni' > aptarchive/knights
+
+setupaptarchive
+changetowebserver -o 'aptwebserver::overwrite::.*::filename=/knights'
+
+msgtest 'Acquire test file from the webserver to check' 'overwrite'
+echo '601 Configuration
+Config-Item: Acquire::http::DependOnSTDIN=0
+
+600 Acquire URI
+URI: http://localhost:8080/holygrail
+Filename: knights-talking
+' | http >/dev/null 2>&1 && msgpass || msgfail
+testfileequal knights-talking 'ni ni ni'
+
+ensure_n_canary_strings_in_dir() {
+	local DIR="$1"
+	local CANARY_STRING="$2"
+	local EXPECTED_N="$3"
+
+	msgtest "Testing in $DIR for $EXPECTED_N canary" "$CANARY_STRING"
+	local N=$(grep "$CANARY_STRING" $DIR/* 2>/dev/null |wc -l )
+	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
+msgtest 'Got expected NODATA failure in' 'apt-get update'
+aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail
+
+ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0
+testequal '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:8080_dists_stable_${f}
+done
+
+msgtest 'Got expected NODATA failure in' 'apt-get update'
+aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail
+
+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:8080_dists_stable_InRelease
+rm -f $LISTS/localhost:8080_dists_stable_Release $LISTS/localhost:8080_dists_stable_Release.gpg
+msgtest 'excpected failure of' 'apt-get update'
+aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail
+
+ensure_n_canary_strings_in_dir $LISTS 'peng neee-wom' 3
+ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0
diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index de235fa05..05b875673 100644
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -435,6 +435,32 @@ int main(int const argc, const char * argv[])
 	       }
 	    }
 
+	    ::Configuration::Item const *Overwrite = _config->Tree("aptwebserver::overwrite");
+	    if (Overwrite != NULL)
+	    {
+	       for (::Configuration::Item *I = Overwrite->Child; I != NULL; I = I->Next)
+	       {
+		  regex_t *pattern = new regex_t;
+		  int const res = regcomp(pattern, I->Tag.c_str(), REG_EXTENDED | REG_ICASE | REG_NOSUB);
+		  if (res != 0)
+		  {
+		     char error[300];
+		     regerror(res, pattern, error, sizeof(error));
+		     sendError(client, 500, *m, sendContent, error);
+		     continue;
+		  }
+		  if (regexec(pattern, filename.c_str(), 0, 0, 0) == 0)
+		  {
+		      filename = _config->Find("aptwebserver::overwrite::" + I->Tag + "::filename", filename);
+		      if (filename[0] == '/')
+			 filename.erase(0,1);
+		      regfree(pattern);
+		      break;
+		  }
+		  regfree(pattern);
+	       }
+	    }
+
 	    // deal with the request
 	    if (RealFileExists(filename) == true)
 	    {