Merge branch 'pu/refuseunsignedlines' into 'master'

Fail if InRelease or Release.gpg contain unsigned lines

See merge request apt-team/apt!45

apt-pkg/contrib/fileutl.cc

@@ -3114,7 +3114,7 @@ FileFd* GetTempFile(std::string const &Prefix, bool ImmediateUnlink, FileFd * co
    snprintf(fn, sizeof(fn), "%s/%s.XXXXXX",
             tempdir.c_str(), Prefix.c_str());
    int const fd = mkstemp(fn);
-   if(ImmediateUnlink)
+   if (ImmediateUnlink)
       unlink(fn);
    if (fd < 0)
    {
@@ -3130,6 +3130,8 @@ FileFd* GetTempFile(std::string const &Prefix, bool ImmediateUnlink, FileFd * co
 	 delete Fd;
       return nullptr;
    }
+   if (ImmediateUnlink == false)
+      Fd->SetFileName(fn);
    return Fd;
 }
 									/*}}}*/

apt-pkg/contrib/gpgv.cc

@@ -20,18 +20,94 @@
 #include <algorithm>
 #include <fstream>
 #include <iostream>
+#include <memory>
 #include <sstream>
 #include <string>
 #include <vector>
 
 #include <apti18n.h>
 									/*}}}*/
-static char * GenerateTemporaryFileTemplate(const char *basename)	/*{{{*/
+
+// syntactic sugar to wrap a raw pointer with a custom deleter in a std::unique_ptr
+static std::unique_ptr<char, decltype(&free)> make_unique_char(void *const str = nullptr)
+{
+   return {static_cast<char *>(str), &free};
+}
+static std::unique_ptr<FILE, decltype(&fclose)> make_unique_FILE(std::string const &filename, char const *const mode)
 {
-   std::string out;
-   std::string tmpdir = GetTempDir();
-   strprintf(out,  "%s/%s.XXXXXX", tmpdir.c_str(), basename);
-   return strdup(out.c_str());
+   return {fopen(filename.c_str(), mode), &fclose};
+}
+
+class LineBuffer							/*{{{*/
+{
+   char *buffer = nullptr;
+   size_t buffer_size = 0;
+   int line_length = 0;
+   // a "normal" find_last_not_of returns npos if not found
+   int find_last_not_of_length(APT::StringView const bad) const
+   {
+      for (int result = line_length - 1; result >= 0; --result)
+	 if (bad.find(buffer[result]) == APT::StringView::npos)
+	    return result + 1;
+      return 0;
+   }
+
+   public:
+   bool empty() const noexcept { return view().empty(); }
+   APT::StringView view() const noexcept { return {buffer, static_cast<size_t>(line_length)}; }
+   bool starts_with(APT::StringView const start) const { return view().substr(0, start.size()) == start; }
+
+   bool writeTo(FileFd *const to, size_t offset = 0) const
+   {
+      if (to == nullptr)
+	 return true;
+      return to->Write(buffer + offset, line_length - offset);
+   }
+   bool writeLineTo(FileFd *const to) const
+    {
+       if (to == nullptr)
+         return true;
+       buffer[line_length] = '\n';
+       bool const result = to->Write(buffer, line_length + 1);
+       buffer[line_length] = '\0';
+       return result;
+    }
+   bool writeNewLineIf(FileFd *const to, bool const condition) const
+   {
+      if (not condition || to == nullptr)
+	 return true;
+      return to->Write("\n", 1);
+   }
+
+   bool readFrom(FILE *stream, std::string const &InFile, bool acceptEoF = false)
+   {
+      errno = 0;
+      line_length = getline(&buffer, &buffer_size, stream);
+      if (errno != 0)
+	 return _error->Errno("getline", "Could not read from %s", InFile.c_str());
+      if (line_length == -1)
+      {
+	 if (acceptEoF)
+	    return false;
+	 return _error->Error("Splitting of clearsigned file %s failed as it doesn't contain all expected parts", InFile.c_str());
+      }
+      // a) remove newline characters, so we can work consistently with lines
+      line_length = find_last_not_of_length("\n\r");
+      // b) remove trailing whitespaces as defined by rfc4880 §7.1
+      line_length = find_last_not_of_length(" \t");
+      buffer[line_length] = '\0';
+      return true;
+   }
+
+   ~LineBuffer() { free(buffer); }
+};
+static bool operator==(LineBuffer const &buf, APT::StringView const exp) noexcept
+{
+   return buf.view() == exp;
+}
+static bool operator!=(LineBuffer const &buf, APT::StringView const exp) noexcept
+{
+   return buf.view() != exp;
 }
 									/*}}}*/
 // ExecGPGV - returns the command needed for verify			/*{{{*/
@@ -47,11 +123,10 @@ static char * GenerateTemporaryFileTemplate(const char *basename)	/*{{{*/
 */
 static bool iovprintf(std::ostream &out, const char *format,
 		      va_list &args, ssize_t &size) {
-   char *S = (char*)malloc(size);
-   ssize_t const n = vsnprintf(S, size, format, args);
+   auto S = make_unique_char(malloc(size));
+   ssize_t const n = vsnprintf(S.get(), size, format, args);
    if (n > -1 && n < size) {
-      out << S;
-      free(S);
+      out << S.get();
       return true;
    } else {
       if (n > -1)
@@ -59,7 +134,6 @@ static bool iovprintf(std::ostream &out, const char *format,
       else
 	 size *= 2;
    }
-   free(S);
    return false;
 }
 static void APT_PRINTF(4) apt_error(std::ostream &outterm, int const statusfd, int fd[2], const char *format, ...)
@@ -73,7 +147,7 @@ static void APT_PRINTF(4) apt_error(std::ostream &outterm, int const statusfd, i
       va_start(args,format);
       ret = iovprintf(out, format, args, size);
       va_end(args);
-      if (ret == true)
+      if (ret)
 	 break;
    }
    if (statusfd != -1)
@@ -81,8 +155,8 @@ static void APT_PRINTF(4) apt_error(std::ostream &outterm, int const statusfd, i
       auto const errtag = "[APTKEY:] ERROR ";
       outstr << '\n';
       auto const errtext = outstr.str();
-      if (FileFd::Write(fd[1], errtag, strlen(errtag)) == false ||
-	    FileFd::Write(fd[1], errtext.data(), errtext.size()) == false)
+      if (not FileFd::Write(fd[1], errtag, strlen(errtag)) ||
+	    not FileFd::Write(fd[1], errtext.data(), errtext.size()))
 	 outterm << errtext << std::flush;
    }
 }
@@ -141,83 +215,134 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
       Opts = Opts->Child;
       for (; Opts != 0; Opts = Opts->Next)
       {
-	 if (Opts->Value.empty() == true)
+	 if (Opts->Value.empty())
 	    continue;
 	 Args.push_back(Opts->Value.c_str());
       }
    }
 
    enum  { DETACHED, CLEARSIGNED } releaseSignature = (FileGPG != File) ? DETACHED : CLEARSIGNED;
-   char * sig = NULL;
-   char * data = NULL;
-   char * conf = nullptr;
+   auto sig = make_unique_char();
+   auto data = make_unique_char();
+   auto conf = make_unique_char();
 
    // Dump the configuration so apt-key picks up the correct Dir values
    {
-      conf = GenerateTemporaryFileTemplate("apt.conf");
+      {
+	 std::string tmpfile;
+	 strprintf(tmpfile, "%s/apt.conf.XXXXXX", GetTempDir().c_str());
+	 conf.reset(strdup(tmpfile.c_str()));
+      }
       if (conf == nullptr) {
 	 apt_error(std::cerr, statusfd, fd, "Couldn't create tempfile names for passing config to apt-key");
 	 local_exit(EINTERNAL);
       }
-      int confFd = mkstemp(conf);
+      int confFd = mkstemp(conf.get());
       if (confFd == -1) {
-	 apt_error(std::cerr, statusfd, fd, "Couldn't create temporary file %s for passing config to apt-key", conf);
+	 apt_error(std::cerr, statusfd, fd, "Couldn't create temporary file %s for passing config to apt-key", conf.get());
 	 local_exit(EINTERNAL);
       }
-      local_exit.files.push_back(conf);
+      local_exit.files.push_back(conf.get());
 
-      std::ofstream confStream(conf);
+      std::ofstream confStream(conf.get());
       close(confFd);
       _config->Dump(confStream);
       confStream.close();
-      setenv("APT_CONFIG", conf, 1);
+      setenv("APT_CONFIG", conf.get(), 1);
    }
 
    if (releaseSignature == DETACHED)
    {
-      Args.push_back(FileGPG.c_str());
-      Args.push_back(File.c_str());
-   }
-   else // clear-signed file
-   {
-      sig = GenerateTemporaryFileTemplate("apt.sig");
-      data = GenerateTemporaryFileTemplate("apt.data");
-      if (sig == NULL || data == NULL)
+      auto detached = make_unique_FILE(FileGPG, "r");
+      if (detached.get() == nullptr)
       {
-	 apt_error(std::cerr, statusfd, fd, "Couldn't create tempfile names for splitting up %s", File.c_str());
+	 apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' could not be opened", FileGPG.c_str());
 	 local_exit(EINTERNAL);
       }
-
-      int const sigFd = mkstemp(sig);
-      int const dataFd = mkstemp(data);
-      if (dataFd != -1)
-	 local_exit.files.push_back(data);
-      if (sigFd != -1)
-	 local_exit.files.push_back(sig);
-      if (sigFd == -1 || dataFd == -1)
+      LineBuffer buf;
+      bool open_signature = false;
+      bool found_badcontent = false;
+      size_t found_signatures = 0;
+      while (buf.readFrom(detached.get(), FileGPG, true))
       {
-	 apt_error(std::cerr, statusfd, fd, "Couldn't create tempfiles for splitting up %s", File.c_str());
-	 local_exit(EINTERNAL);
+	 if (open_signature)
+	 {
+	    if (buf == "-----END PGP SIGNATURE-----")
+	       open_signature = false;
+	    else if (buf.starts_with("-"))
+	    {
+	       // the used Radix-64 is not using dash for any value, so a valid line can't
+	       // start with one. Header keys could, but no existent one does and seems unlikely.
+	       // Instead it smells a lot like a header the parser didn't recognize.
+	       apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' contains unexpected line starting with a dash", FileGPG.c_str());
+	       local_exit(112);
+	    }
+	 }
+	 else //if (not open_signature)
+	 {
+	    if (buf == "-----BEGIN PGP SIGNATURE-----")
+	    {
+	       open_signature = true;
+	       ++found_signatures;
+	       if (found_badcontent)
+		  break;
+	    }
+	    else
+	    {
+	       found_badcontent = true;
+	       if (found_signatures != 0)
+		  break;
+	    }
+	 }
+      }
+      if (found_signatures == 0 && statusfd != -1)
+      {
+	 // This is not an attack attempt but a file even gpgv would complain about
+	 // likely the result of a paywall which is covered by the gpgv method
+	 auto const errtag = "[GNUPG:] NODATA\n";
+	 FileFd::Write(fd[1], errtag, strlen(errtag));
+	 local_exit(113);
+      }
+      else if (found_badcontent)
+      {
+	 apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' contains lines not belonging to a signature", FileGPG.c_str());
+	 local_exit(112);
+      }
+      if (open_signature)
+      {
+	 apt_error(std::cerr, statusfd, fd, "Detached signature file '%s' contains unclosed signatures", FileGPG.c_str());
+	 local_exit(112);
       }
 
+      Args.push_back(FileGPG.c_str());
+      Args.push_back(File.c_str());
+   }
+   else // clear-signed file
+   {
       FileFd signature;
-      signature.OpenDescriptor(sigFd, FileFd::WriteOnly, true);
+      if (GetTempFile("apt.sig", false, &signature) == nullptr)
+	 local_exit(EINTERNAL);
+      sig.reset(strdup(signature.Name().c_str()));
+      local_exit.files.push_back(sig.get());
       FileFd message;
-      message.OpenDescriptor(dataFd, FileFd::WriteOnly, true);
+      if (GetTempFile("apt.data", false, &message) == nullptr)
+	 local_exit(EINTERNAL);
+      data.reset(strdup(message.Name().c_str()));
+      local_exit.files.push_back(data.get());
 
-      if (signature.Failed() == true || message.Failed() == true ||
-	    SplitClearSignedFile(File, &message, nullptr, &signature) == false)
+      if (signature.Failed() || message.Failed() ||
+	  not SplitClearSignedFile(File, &message, nullptr, &signature))
       {
 	 apt_error(std::cerr, statusfd, fd, "Splitting up %s into data and signature failed", File.c_str());
 	 local_exit(112);
       }
-      Args.push_back(sig);
-      Args.push_back(data);
+      Args.push_back(sig.get());
+      Args.push_back(data.get());
    }
 
    Args.push_back(NULL);
 
-   if (Debug == true)
+   if (Debug)
    {
       std::clog << "Preparing to exec: ";
       for (std::vector<const char *>::const_iterator a = Args.begin(); *a != NULL; ++a)
@@ -270,7 +395,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
    }
 
    // check if it exit'ed normally …
-   if (WIFEXITED(Status) == false)
+   if (not WIFEXITED(Status))
    {
       apt_error(std::cerr, statusfd, fd, _("Sub-process %s exited unexpectedly"), "apt-key");
       local_exit(EINTERNAL);
@@ -289,168 +414,141 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
 }
 									/*}}}*/
 // SplitClearSignedFile - split message into data/signature		/*{{{*/
-static int GetLineErrno(char **lineptr, size_t *n, FILE *stream, std::string const &InFile)
+bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
+      std::vector<std::string> * const ContentHeader, FileFd * const SignatureFile)
 {
-   int result;
+   auto in = make_unique_FILE(InFile, "r");
+   if (in.get() == nullptr)
+      return _error->Errno("fopen", "can not open %s", InFile.c_str());
 
-   errno = 0;
-   result = getline(lineptr, n, stream);
-   if (errno != 0)
+   struct ScopedErrors
+   {
+      ScopedErrors() { _error->PushToStack(); }
+      ~ScopedErrors() { _error->MergeWithStack(); }
+   } scoped;
+   LineBuffer buf;
+
+   // start of the message
+   if (not buf.readFrom(in.get(), InFile))
+      return false; // empty or read error
+   if (buf != "-----BEGIN PGP SIGNED MESSAGE-----")
    {
-      _error->Errno("getline", "Could not read from %s", InFile.c_str());
-      return -1;
+      // this might be an unsigned file we don't want to report errors for,
+      // but still finish unsuccessful none the less.
+      while (buf.readFrom(in.get(), InFile, true))
+	 if (buf == "-----BEGIN PGP SIGNED MESSAGE-----")
+	    return _error->Error("Clearsigned file '%s' does not start with a signed message block.", InFile.c_str());
+
+      return false;
    }
 
-   return result;
-}
-bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
-      std::vector<std::string> * const ContentHeader, FileFd * const SignatureFile)
-{
-   FILE *in = fopen(InFile.c_str(), "r");
-   if (in == NULL)
-      return _error->Errno("fopen", "can not open %s", InFile.c_str());
+   // save "Hash" Armor Headers
+   while (true)
+   {
+      if (not buf.readFrom(in.get(), InFile))
+	 return false;
+      if (buf.empty())
+	 break; // empty line ends the Armor Headers
+      if (buf.starts_with("-"))
+	 // § 6.2 says unknown keys should be reported to the user. We don't go that far,
+	 // but we assume that there will never be a header key starting with a dash
+	 return _error->Error("Clearsigned file '%s' contains unexpected line starting with a dash (%s)", InFile.c_str(), "armor");
+      if (ContentHeader != nullptr && buf.starts_with("Hash: "))
+	 ContentHeader->push_back(buf.view().to_string());
+   }
 
-   bool found_message_start = false;
-   bool found_message_end = false;
-   bool skip_until_empty_line = false;
-   bool found_signature = false;
+   // the message itself
    bool first_line = true;
-   bool signed_message_not_on_first_line = false;
-   bool found_garbage = false;
-
-   char *buf = NULL;
-   size_t buf_size = 0;
-   _error->PushToStack();
-   while (GetLineErrno(&buf, &buf_size, in, InFile) != -1)
+   while (true)
    {
-      _strrstrip(buf);
-      if (found_message_start == false)
+      if (not buf.readFrom(in.get(), InFile))
+	 return false;
+
+      if (buf.starts_with("-"))
       {
-	 if (strcmp(buf, "-----BEGIN PGP SIGNED MESSAGE-----") == 0)
+	 if (buf == "-----BEGIN PGP SIGNATURE-----")
+	 {
+	    if (not buf.writeLineTo(SignatureFile))
+	       return false;
+	    break;
+	 }
+	 else if (buf.starts_with("- "))
 	 {
-	    found_message_start = true;
-	    skip_until_empty_line = true;
+	    // we don't have any fields which need to be dash-escaped,
+	    // but implementations are free to escape all lines …
+	    if (not buf.writeNewLineIf(ContentFile, not first_line) || not buf.writeTo(ContentFile, 2))
+	       return false;
 	 }
 	 else
-	    signed_message_not_on_first_line = found_garbage = true;
+	    // § 7.1 says a client should warn, but we don't really work with files which
+	    // should contain lines starting with a dash, so it is a lot more likely that
+	    // this is an attempt to trick our parser vs. gpgv parser into ignoring a header
+	    return _error->Error("Clearsigned file '%s' contains unexpected line starting with a dash (%s)", InFile.c_str(), "msg");
       }
-      else if (skip_until_empty_line == true)
+      else if (not buf.writeNewLineIf(ContentFile, not first_line) || not buf.writeTo(ContentFile))
+	 return false;
+      first_line = false;
+   }
+
+   // collect all signatures
+   bool open_signature = true;
+   while (true)
+   {
+      if (not buf.readFrom(in.get(), InFile, true))
+	 break;
+
+      if (open_signature)
       {
-	 if (strlen(buf) == 0)
-	    skip_until_empty_line = false;
-	 // save "Hash" Armor Headers, others aren't allowed
-	 else if (ContentHeader != NULL && strncmp(buf, "Hash: ", strlen("Hash: ")) == 0)
-	    ContentHeader->push_back(buf);
+	 if (buf == "-----END PGP SIGNATURE-----")
+	    open_signature = false;
+	 else if (buf.starts_with("-"))
+	    // the used Radix-64 is not using dash for any value, so a valid line can't
+	    // start with one. Header keys could, but no existent one does and seems unlikely.
+	    // Instead it smells a lot like a header the parser didn't recognize.
+	    return _error->Error("Clearsigned file '%s' contains unexpected line starting with a dash (%s)", InFile.c_str(), "sig");
       }
-      else if (found_signature == false)
+      else //if (not open_signature)
       {
-	 if (strcmp(buf, "-----BEGIN PGP SIGNATURE-----") == 0)
-	 {
-	    found_signature = true;
-	    found_message_end = true;
-	    if (SignatureFile != NULL)
-	    {
-	       SignatureFile->Write(buf, strlen(buf));
-	       SignatureFile->Write("\n", 1);
-	    }
-	 }
-	 else if (found_message_end == false) // we are in the message block
-	 {
-	    // we don't have any fields which need dash-escaped,
-	    // but implementations are free to encode all lines …
-	    char const * dashfree = buf;
-	    if (strncmp(dashfree, "- ", 2) == 0)
-	       dashfree += 2;
-	    if(first_line == true) // first line does not need a newline
-	       first_line = false;
-	    else if (ContentFile != NULL)
-	       ContentFile->Write("\n", 1);
-	    else
-	       continue;
-	    if (ContentFile != NULL)
-	       ContentFile->Write(dashfree, strlen(dashfree));
-	 }
+	 if (buf == "-----BEGIN PGP SIGNATURE-----")
+	    open_signature = true;
 	 else
-	    found_garbage = true;
+	    return _error->Error("Clearsigned file '%s' contains unsigned lines.", InFile.c_str());
       }
-      else if (found_signature == true)
-      {
-	 if (SignatureFile != NULL)
-	 {
-	    SignatureFile->Write(buf, strlen(buf));
-	    SignatureFile->Write("\n", 1);
-	 }
-	 if (strcmp(buf, "-----END PGP SIGNATURE-----") == 0)
-	    found_signature = false; // look for other signatures
-      }
-      // all the rest is whitespace, unsigned garbage or additional message blocks we ignore
-      else
-	 found_garbage = true;
+
+      if (not buf.writeLineTo(SignatureFile))
+	 return false;
    }
-   fclose(in);
-   if (buf != NULL)
-      free(buf);
+   if (open_signature)
+      return _error->Error("Signature in file %s wasn't closed", InFile.c_str());
 
-   // Flush the files. Errors will be checked below.
+   // Flush the files
    if (SignatureFile != nullptr)
       SignatureFile->Flush();
    if (ContentFile != nullptr)
       ContentFile->Flush();
 
-   if (found_message_start)
-   {
-      if (signed_message_not_on_first_line)
-	 _error->Warning("Clearsigned file '%s' does not start with a signed message block.", InFile.c_str());
-      else if (found_garbage)
-	 _error->Warning("Clearsigned file '%s' contains unsigned lines.", InFile.c_str());
-   }
-
-   // An error occurred during reading - propagate it up
-   bool const hasErrored = _error->PendingError();
-   _error->MergeWithStack();
-   if (hasErrored)
-      return false;
-
-   if (found_signature == true)
-      return _error->Error("Signature in file %s wasn't closed", InFile.c_str());
-
-   // if we haven't found any of them, this an unsigned file,
-   // so don't generate an error, but splitting was unsuccessful none-the-less
-   if (first_line == true && found_message_start == false && found_message_end == false)
+   // Catch-all for "unhandled" read/sync errors
+   if (_error->PendingError())
       return false;
-   // otherwise one missing indicates a syntax error
-   else if (first_line == true || found_message_start == false || found_message_end == false)
-     return _error->Error("Splitting of file %s failed as it doesn't contain all expected parts %i %i %i", InFile.c_str(), first_line, found_message_start, found_message_end);
-
    return true;
 }
 									/*}}}*/
 bool OpenMaybeClearSignedFile(std::string const &ClearSignedFileName, FileFd &MessageFile) /*{{{*/
 {
-   char * const message = GenerateTemporaryFileTemplate("fileutl.message");
-   int const messageFd = mkstemp(message);
-   if (messageFd == -1)
-   {
-      free(message);
-      return _error->Errno("mkstemp", "Couldn't create temporary file to work with %s", ClearSignedFileName.c_str());
-   }
-   // we have the fd, that's enough for us
-   unlink(message);
-   free(message);
-
-   MessageFile.OpenDescriptor(messageFd, FileFd::ReadWrite | FileFd::BufferedWrite, true);
-   if (MessageFile.Failed() == true)
+   if (GetTempFile("clearsigned.message", true, &MessageFile) == nullptr)
+      return false;
+   if (MessageFile.Failed())
       return _error->Error("Couldn't open temporary file to work with %s", ClearSignedFileName.c_str());
 
    _error->PushToStack();
    bool const splitDone = SplitClearSignedFile(ClearSignedFileName, &MessageFile, NULL, NULL);
    bool const errorDone = _error->PendingError();
    _error->MergeWithStack();
-   if (splitDone == false)
+   if (not splitDone)
    {
       MessageFile.Close();
 
-      if (errorDone == true)
+      if (errorDone)
 	 return false;
 
       // we deal with an unsigned file
@@ -458,10 +556,10 @@ bool OpenMaybeClearSignedFile(std::string const &ClearSignedFileName, FileFd &Me
    }
    else // clear-signed
    {
-      if (MessageFile.Seek(0) == false)
+      if (not MessageFile.Seek(0))
 	 return _error->Errno("lseek", "Unable to seek back in message for file %s", ClearSignedFileName.c_str());
    }
 
-   return MessageFile.Failed() == false;
+   return not MessageFile.Failed();
 }
 									/*}}}*/

apt-pkg/deb/debindexfile.cc

@@ -315,13 +315,10 @@ const pkgIndexFile::Type *debStringPackageIndex::GetType() const
 debStringPackageIndex::debStringPackageIndex(std::string const &content) :
    pkgDebianIndexRealFile("", false), d(NULL)
 {
-   char fn[1024];
-   std::string const tempdir = GetTempDir();
-   snprintf(fn, sizeof(fn), "%s/%s.XXXXXX", tempdir.c_str(), "apt-tmp-index");
-   int const fd = mkstemp(fn);
-   File = fn;
-   FileFd::Write(fd, content.data(), content.length());
-   close(fd);
+   FileFd fd;
+   GetTempFile("apt-tmp-index", false, &fd);
+   fd.Write(content.data(), content.length());
+   File = fd.Name();
 }
 debStringPackageIndex::~debStringPackageIndex()
 {

cmdline/apt-extracttemplates.cc

@@ -229,29 +229,13 @@ static bool ShowHelp(CommandLine &)					/*{{{*/
 /* */
 static string WriteFile(const char *package, const char *prefix, const char *data)
 {
-	char fn[512];
-
-        std::string tempdir = GetTempDir();
-	snprintf(fn, sizeof(fn), "%s/%s.%s.XXXXXX",
-                 _config->Find("APT::ExtractTemplates::TempDir", 
-                               tempdir.c_str()).c_str(),
-                 package, prefix);
-	FileFd f;
-	if (data == NULL)
-		data = "";
-        int fd = mkstemp(fn);
-        if (fd < 0) {
-		_error->Errno("ofstream::ofstream",_("Unable to mkstemp %s"),fn);
-                return string();
-        }
-	if (!f.OpenDescriptor(fd, FileFd::WriteOnly, FileFd::None, true))
-	{
-		_error->Errno("ofstream::ofstream",_("Unable to write to %s"),fn);
-		return string();
-	}
-	f.Write(data, strlen(data));
-	f.Close();
-	return fn;
+   FileFd f;
+   std::string tplname;
+   strprintf(tplname, "%s.%s", package, prefix);
+   GetTempFile(tplname, false, &f);
+   if (data != nullptr)
+      f.Write(data, strlen(data));
+   return f.Name();
 }
 									/*}}}*/
 // WriteConfig - write out the config data from a debian package file	/*{{{*/
@@ -289,6 +273,10 @@ static bool Go(CommandLine &CmdL)
 	if (debconfver.empty() == true)
 		return _error->Error( _("Cannot get debconf version. Is debconf installed?"));
 
+	auto const tmpdir = _config->Find("APT::ExtractTemplates::TempDir");
+	if (tmpdir.empty() == false)
+	   setenv("TMPDIR", tmpdir.c_str(), 1);
+
 	// Process each package passsed in
 	for (unsigned int I = 0; I != CmdL.FileSize(); I++)
 	{

test/integration/test-apt-extracttemplates

@@ -44,6 +44,13 @@ Description: Some bar var
 	testfileequal "$TEMPLATE" "$TEMPLATE_STR"
 	CONFIG=$(cut -f4 -d' ' $OUT)
 	testfileequal "$CONFIG" "$CONFIG_STR"
+	msgtest 'No extra files or directories in extraction directory'
+	if [ "$(find ./extracttemplates-out | wc -l)" = '3' ]; then
+		msgpass
+	else
+		msgfail
+		ls -l ./extracttemplates-out
+	fi
 
 	# ensure that the format of the output string has the right number of dots
 	for s in "$CONFIG" "$TEMPLATE"; do

test/integration/test-cve-2013-1051-InRelease-parsing

@@ -46,9 +46,12 @@ touch -d '+1hour' aptarchive/dists/stable/InRelease
 listcurrentlistsdirectory | sed '/_InRelease/ d' > listsdir.lst
 msgtest 'apt-get update should ignore unsigned data in the' 'InRelease'
 testwarningequal "Get:1 http://localhost:${APTHTTPPORT} stable InRelease [$(stat -c%s aptarchive/dists/stable/InRelease) B]
+Err:1 http://localhost:${APTHTTPPORT} stable InRelease
+  Splitting up ${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease into data and signature failed
 Reading package lists...
-W: Clearsigned file '${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease' contains unsigned lines.
-W: Clearsigned file '${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/localhost:${APTHTTPPORT}_dists_stable_InRelease' contains unsigned lines." --nomsg aptget update
+W: An error occurred during the signature verification. The repository is not updated and the previous index files will be used. GPG error: http://localhost:${APTHTTPPORT} stable InRelease: Splitting up ${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease into data and signature failed
+W: Failed to fetch http://localhost:${APTHTTPPORT}/dists/stable/InRelease  Splitting up ${TMPWORKINGDIRECTORY}/rootdir/var/lib/apt/lists/partial/localhost:${APTHTTPPORT}_dists_stable_InRelease into data and signature failed
+W: Some index files failed to download. They have been ignored, or old ones used instead." --nomsg aptget update
 testfileequal './listsdir.lst' "$(listcurrentlistsdirectory | sed '/_InRelease/ d')"
 
 # ensure there is no package

test/integration/test-cve-2019-3462-Release.gpg-payload

@@ -0,0 +1,43 @@
+#!/bin/sh
+set -e
+
+# This is not covered by the CVE and harmless by itself, but used in
+# the exploit and while harmless it is also pointless to allow it
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+configarchitecture 'amd64'
+
+export APT_DONT_SIGN='InRelease'
+
+insertpackage 'unstable' 'foo' 'all' '1'
+setupaptarchive
+rm -rf rootdir/var/lib/apt/lists
+
+verify() {
+	testfailure apt update
+	testsuccess grep '^  Detached signature file' rootdir/tmp/testfailure.output
+	testfailure apt show foo
+}
+
+msgmsg 'Payload after detached signature'
+find aptarchive -name 'Release.gpg' | while read FILE; do
+	cp -a "$FILE" "${FILE}.bak"
+	echo "evil payload" >> "$FILE"
+done
+verify
+
+msgmsg 'Payload in-between detached signatures'
+find aptarchive -name 'Release.gpg' | while read FILE; do
+	cat "${FILE}.bak" >> "$FILE"
+done
+verify
+
+msgmsg 'Payload before detached signature'
+find aptarchive -name 'Release.gpg' | while read FILE; do
+	echo "evil payload" > "$FILE"
+	cat "${FILE}.bak" >> "$FILE"
+done
+verify

test/integration/test-method-gpgv

@@ -71,44 +71,60 @@ testrun() {
 [GNUPG:] VALIDSIG 891CC50E605796A0C6E733F74BC0A39C27CE74F9 2016-09-01 1472742629 0 4 0 1 11 00 891CC50E605796A0C6E733F74BC0A39C27CE74F9'
 }
 
+echo 'Test' > message.data
+cat >message.sig <<EOF
+-----BEGIN PGP SIGNATURE-----
+
+iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt
+cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l
+3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg
+X/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k
+V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx
+pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns
+JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq
+=TB1F
+-----END PGP SIGNATURE-----
+EOF
+
+
 gpgvmethod() {
-	echo '601 Configuration
+	echo "601 Configuration
 Config-Item: Debug::Acquire::gpgv=1
 Config-Item: Dir::Bin::apt-key=./faked-apt-key
 Config-Item: APT::Hashes::SHA1::Weak=true
 
 600 URI Acquire
-URI: file:///dev/null
-Filename: /dev/zero
-' | runapt "${METHODSDIR}/gpgv"
+URI: file://${TMPWORKINGDIRECTORY}/message.sig
+Filename: ${TMPWORKINGDIRECTORY}/message.data
+" | runapt "${METHODSDIR}/gpgv"
 }
 testrun
 
 gpgvmethod() {
-	echo '601 Configuration
+	echo "601 Configuration
 Config-Item: Debug::Acquire::gpgv=1
 Config-Item: Dir::Bin::apt-key=./faked-apt-key
 Config-Item: APT::Hashes::SHA1::Weak=true
 
 600 URI Acquire
-URI: file:///dev/null
-Filename: /dev/zero
+URI: file://${TMPWORKINGDIRECTORY}/message.sig
+Filename: ${TMPWORKINGDIRECTORY}/message.data
 Signed-By: /dev/null,34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE
-' | runapt "${METHODSDIR}/gpgv"
+" | runapt "${METHODSDIR}/gpgv"
 }
 testrun
 
 gpgvmethod() {
-	echo '601 Configuration
+	echo "601 Configuration
 Config-Item: Debug::Acquire::gpgv=1
 Config-Item: Dir::Bin::apt-key=./faked-apt-key
 Config-Item: APT::Hashes::SHA1::Weak=true
 
 600 URI Acquire
-URI: file:///dev/null
-Filename: /dev/zero
+URI: file://${TMPWORKINGDIRECTORY}/message.sig
+Filename: ${TMPWORKINGDIRECTORY}/message.data
 Signed-By: 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE,/dev/null
-' | runapt "${METHODSDIR}/gpgv"
+" | runapt "${METHODSDIR}/gpgv"
 }
 testrun
 
@@ -122,16 +138,16 @@ testsuccess grep '^\s\+Good:\s\+$' method.output
 testsuccess grep 'verified because the public key is not available: GOODSIG' method.output
 
 gpgvmethod() {
-	echo '601 Configuration
+	echo "601 Configuration
 Config-Item: Debug::Acquire::gpgv=1
 Config-Item: Dir::Bin::apt-key=./faked-apt-key
 Config-Item: APT::Hashes::SHA1::Weak=true
 
 600 URI Acquire
-URI: file:///dev/null
-Filename: /dev/zero
+URI: file://${TMPWORKINGDIRECTORY}/message.sig
+Filename: ${TMPWORKINGDIRECTORY}/message.data
 Signed-By: 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE!
-' | runapt "${METHODSDIR}/gpgv"
+" | runapt "${METHODSDIR}/gpgv"
 }
 testgpgv 'Exact matched subkey signed with long keyid' 'Good: GOODSIG 5A90D141DBAC8DAE' '34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE!' '[GNUPG:] GOODSIG 5A90D141DBAC8DAE Sebastian Subkey <subkey@example.org>
 [GNUPG:] VALIDSIG 34A8E9D18DB320F367E8EAA05A90D141DBAC8DAE 2018-08-16 1534459673 0 4 0 1 11 00 4281DEDBD466EAE8C1F4157E5B6896415D44C43E'

test/libapt/openmaybeclearsignedfile_test.cc

@@ -111,7 +111,6 @@ TEST(OpenMaybeClearSignedFileTest,SignedFileWithContentHeaders)
    EXPECT_TRUE(fd.Eof());
 }
 
-// That isn't how multiple signatures are done
 TEST(OpenMaybeClearSignedFileTest,SignedFileWithTwoSignatures)
 {
    std::string tempfile;
@@ -190,19 +189,16 @@ TEST(OpenMaybeClearSignedFileTest,TwoSimpleSignedFile)
 "-----END PGP SIGNATURE-----");
    EXPECT_TRUE(_error->empty());
    EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
-   EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
+   EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
    EXPECT_FALSE(_error->empty());
-   EXPECT_TRUE(fd.IsOpen());
-   char buffer[100];
-   EXPECT_TRUE(fd.ReadLine(buffer, sizeof(buffer)));
-   EXPECT_STREQ(buffer, "Test");
-   EXPECT_TRUE(fd.Eof());
-   ASSERT_FALSE(_error->empty());
+   EXPECT_FALSE(fd.IsOpen());
 
+   // technically they are signed, but we just want one message
+   EXPECT_TRUE(_error->PendingError());
    std::string msg;
-   _error->PopMessage(msg);
+   EXPECT_TRUE(_error->PopMessage(msg));
    EXPECT_EQ("Clearsigned file '" + tempfile + "' contains unsigned lines.", msg);
 }
 
@@ -244,19 +240,15 @@ TEST(OpenMaybeClearSignedFileTest,GarbageTop)
 "-----END PGP SIGNATURE-----\n");
    EXPECT_FALSE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(_error->empty());
-   EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
+   EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
-   EXPECT_TRUE(fd.IsOpen());
-   char buffer[100];
-   EXPECT_TRUE(fd.ReadLine(buffer, sizeof(buffer)));
-   EXPECT_STREQ(buffer, "Test");
-   EXPECT_TRUE(fd.Eof());
+   EXPECT_FALSE(fd.IsOpen());
    ASSERT_FALSE(_error->empty());
-   ASSERT_FALSE(_error->PendingError());
+   ASSERT_TRUE(_error->PendingError());
 
    std::string msg;
-   _error->PopMessage(msg);
+   EXPECT_TRUE(_error->PopMessage(msg));
    EXPECT_EQ("Clearsigned file '" + tempfile + "' does not start with a signed message block.", msg);
 }
 
@@ -313,19 +305,15 @@ TEST(OpenMaybeClearSignedFileTest,GarbageBottom)
 "Garbage");
    EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(_error->empty());
-   EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
+   EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
-   EXPECT_TRUE(fd.IsOpen());
-   char buffer[100];
-   EXPECT_TRUE(fd.ReadLine(buffer, sizeof(buffer)));
-   EXPECT_STREQ(buffer, "Test");
-   EXPECT_TRUE(fd.Eof());
+   EXPECT_FALSE(fd.IsOpen());
    ASSERT_FALSE(_error->empty());
-   ASSERT_FALSE(_error->PendingError());
+   ASSERT_TRUE(_error->PendingError());
 
    std::string msg;
-   _error->PopMessage(msg);
+   EXPECT_TRUE(_error->PopMessage(msg));
    EXPECT_EQ("Clearsigned file '" + tempfile + "' contains unsigned lines.", msg);
 }
 
@@ -347,7 +335,7 @@ TEST(OpenMaybeClearSignedFileTest,BogusNoSig)
 
    std::string msg;
    _error->PopMessage(msg);
-   EXPECT_EQ("Splitting of file " + tempfile + " failed as it doesn't contain all expected parts 0 1 0", msg);
+   EXPECT_EQ("Splitting of clearsigned file " + tempfile + " failed as it doesn't contain all expected parts", msg);
 }
 
 TEST(OpenMaybeClearSignedFileTest,BogusSigStart)
@@ -371,3 +359,127 @@ TEST(OpenMaybeClearSignedFileTest,BogusSigStart)
    _error->PopMessage(msg);
    EXPECT_EQ("Signature in file " + tempfile + " wasn't closed", msg);
 }
+
+TEST(OpenMaybeClearSignedFileTest,DashedSignedFile)
+{
+   std::string tempfile;
+   FileFd fd;
+   createTemporaryFile("dashedsignedfile", fd, &tempfile, "-----BEGIN PGP SIGNED MESSAGE-----\n"
+"Hash: SHA512\n"
+"\n"
+"- Test\n"
+"-----BEGIN PGP SIGNATURE-----\n"
+"\n"
+"iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt\n"
+"cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l\n"
+"3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg\n"
+"X/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k\n"
+"V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx\n"
+"pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns\n"
+"JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
+"=TB1F\n"
+"-----END PGP SIGNATURE-----\n");
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
+   EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
+   if (tempfile.empty() == false)
+      unlink(tempfile.c_str());
+   EXPECT_TRUE(fd.IsOpen());
+   char buffer[100];
+   EXPECT_TRUE(fd.ReadLine(buffer, sizeof(buffer)));
+   EXPECT_STREQ(buffer, "Test");
+   EXPECT_TRUE(fd.Eof());
+}
+TEST(OpenMaybeClearSignedFileTest,StrangeDashArmorFile)
+{
+   std::string tempfile;
+   FileFd fd;
+   createTemporaryFile("strangedashfile", fd, &tempfile, "-----BEGIN PGP SIGNED MESSAGE-----\n"
+"Hash: SHA512\n"
+"-Hash: SHA512\n"
+"\n"
+"Test\n"
+"-----BEGIN PGP SIGNATURE-----\n"
+"\n"
+"iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt\n"
+"cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l\n"
+"3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg\n"
+"X/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k\n"
+"V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx\n"
+"pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns\n"
+"JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
+"=TB1F\n"
+"-----END PGP SIGNATURE-----\n");
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
+   EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
+   if (tempfile.empty() == false)
+      unlink(tempfile.c_str());
+   EXPECT_FALSE(_error->empty());
+   EXPECT_FALSE(fd.IsOpen());
+
+   std::string msg;
+   EXPECT_TRUE(_error->PendingError());
+   EXPECT_TRUE(_error->PopMessage(msg));
+   EXPECT_EQ("Clearsigned file '" + tempfile + "' contains unexpected line starting with a dash (armor)", msg);
+}
+TEST(OpenMaybeClearSignedFileTest,StrangeDashMsgFile)
+{
+   std::string tempfile;
+   FileFd fd;
+   createTemporaryFile("strangedashfile", fd, &tempfile, "-----BEGIN PGP SIGNED MESSAGE-----\n"
+"Hash: SHA512\n"
+"\n"
+"-Test\n"
+"-----BEGIN PGP SIGNATURE-----\n"
+"\n"
+"iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt\n"
+"cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l\n"
+"3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg\n"
+"X/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k\n"
+"V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx\n"
+"pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns\n"
+"JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
+"=TB1F\n"
+"-----END PGP SIGNATURE-----\n");
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
+   EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
+   if (tempfile.empty() == false)
+      unlink(tempfile.c_str());
+   EXPECT_FALSE(_error->empty());
+   EXPECT_FALSE(fd.IsOpen());
+
+   std::string msg;
+   EXPECT_TRUE(_error->PendingError());
+   EXPECT_TRUE(_error->PopMessage(msg));
+   EXPECT_EQ("Clearsigned file '" + tempfile + "' contains unexpected line starting with a dash (msg)", msg);
+}
+TEST(OpenMaybeClearSignedFileTest,StrangeDashSigFile)
+{
+   std::string tempfile;
+   FileFd fd;
+   createTemporaryFile("strangedashfile", fd, &tempfile, "-----BEGIN PGP SIGNED MESSAGE-----\n"
+"Hash: SHA512\n"
+"\n"
+"Test\n"
+"-----BEGIN PGP SIGNATURE-----\n"
+"\n"
+"iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt\n"
+"cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l\n"
+"3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg\n"
+"-/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k\n"
+"V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx\n"
+"pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns\n"
+"JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
+"=TB1F\n"
+"-----END PGP SIGNATURE-----\n");
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
+   EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
+   if (tempfile.empty() == false)
+      unlink(tempfile.c_str());
+   EXPECT_FALSE(_error->empty());
+   EXPECT_FALSE(fd.IsOpen());
+
+   std::string msg;
+   EXPECT_TRUE(_error->PendingError());
+   EXPECT_TRUE(_error->PopMessage(msg));
+   EXPECT_EQ("Clearsigned file '" + tempfile + "' contains unexpected line starting with a dash (sig)", msg);
+}