- add method to open (maybe) clearsigned files transparently

* ftparchive/writer.cc:
  - use OpenMaybeClearSignedFile to be free from detecting and
    skipping clearsigning metadata in dsc files

5 files changed, 135 insertions, 63 deletions

diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc
index 5921d7c67..fc16dd32c 100644
--- a/apt-pkg/contrib/gpgv.cc
+++ b/apt-pkg/contrib/gpgv.cc
@@ -19,7 +19,7 @@
 
 #include <apti18n.h>
 									/*}}}*/
-char * GenerateTemporaryFileTemplate(const char *basename)		/*{{{*/
+static char * GenerateTemporaryFileTemplate(const char *basename)	/*{{{*/
 {
    const char *tmpdir = getenv("TMPDIR");
 #ifdef P_tmpdir
@@ -376,5 +376,58 @@ bool SplitClearSignedFile(std::string const &InFile, int const ContentFile,
       fclose(out_signature);
    fclose(in);
 
+   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 (found_message_start == false && found_message_end == false)
+      return false;
+   // otherwise one missing indicates a syntax error
+   else if (found_message_start == false || found_message_end == false)
+      return _error->Error("Splitting of file %s failed as it doesn't contain all expected parts", InFile.c_str());
+
    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, thats enough for us
+   unlink(message);
+   free(message);
+
+   int const duppedMsg = dup(messageFd);
+   if (duppedMsg == -1)
+      return _error->Errno("dup", "Couldn't duplicate FD to work with %s", ClearSignedFileName.c_str());
+
+   _error->PushToStack();
+   bool const splitDone = SplitClearSignedFile(ClearSignedFileName.c_str(), messageFd, NULL, -1);
+   bool const errorDone = _error->PendingError();
+   _error->MergeWithStack();
+   if (splitDone == false)
+   {
+      close(duppedMsg);
+
+      if (errorDone == true)
+	 return false;
+
+      // we deal with an unsigned file
+      MessageFile.Open(ClearSignedFileName, FileFd::ReadOnly);
+   }
+   else // clear-signed
+   {
+      if (lseek(duppedMsg, 0, SEEK_SET) < 0)
+	 return _error->Errno("lseek", "Unable to seek back in message fd for file %s", ClearSignedFileName.c_str());
+      MessageFile.OpenDescriptor(duppedMsg, FileFd::ReadOnly, true);
+   }
+
+   return MessageFile.Failed() == false;
+}
+									/*}}}*/
diff --git a/apt-pkg/contrib/gpgv.h b/apt-pkg/contrib/gpgv.h
index 8e04855e4..ab7d35ab1 100644
--- a/apt-pkg/contrib/gpgv.h
+++ b/apt-pkg/contrib/gpgv.h
@@ -12,6 +12,8 @@
 #include <string>
 #include <vector>
 
+#include <apt-pkg/fileutl.h>
+
 #if __GNUC__ >= 4
 	#define APT_noreturn	__attribute__ ((noreturn))
 #else
@@ -52,10 +54,17 @@ inline void ExecGPGV(std::string const &File, std::string const &FileSig,
  *  The code doesn't support dash-encoded lines as these are not
  *  expected to be present in files we have to deal with.
  *
+ *  The content of the split files is undefined if the splitting was
+ *  unsuccessful.
+ *
+ *  Note that trying to split an unsigned file will fail, but
+ *  not generate an error message.
+ *
  *  @param InFile is the clear-signed file
  *  @param ContentFile is the Fd the message will be written to
  *  @param ContentHeader is a list of all required Amored Headers for the message
  *  @param SignatureFile is the Fd all signatures will be written to
+ *  @return true if the splitting was successful, false otherwise
  */
 bool SplitClearSignedFile(std::string const &InFile, int const ContentFile,
       std::vector<std::string> * const ContentHeader, int const SignatureFile);
@@ -74,4 +83,17 @@ bool SplitClearSignedFile(std::string const &InFile, int const ContentFile,
 bool RecombineToClearSignedFile(std::string const &OutFile, int const ContentFile,
       std::vector<std::string> const &ContentHeader, int const SignatureFile);
 
+/** \brief open a file which might be clear-signed
+ *
+ * This method tries to extract the (signed) message of a file.
+ * If the file isn't signed it will just open the given filename.
+ * Otherwise the message is extracted to a temporary file which
+ * will be opened instead.
+ *
+ * @param ClearSignedFileName is the name of the file to open
+ * @param[out] MessageFile is the FileFd in which the file will be opened
+ * @return true if opening was successful, otherwise false
+ */
+bool OpenMaybeClearSignedFile(std::string const &ClearSignedFileName, FileFd &MessageFile);
+
 #endif
diff --git a/debian/changelog b/debian/changelog
index 3ef652c56..27fae657c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -12,12 +12,16 @@ apt (0.9.7.9) UNRELEASED; urgency=low
       recombines it after that in a known-good way without unsigned blocks
       and whitespaces resulting usually in more or less the same file as
       before, but later code can be sure about the format
+    - add method to open (maybe) clearsigned files transparently
   * apt-pkg/acquire-item.cc:
     - keep the last good InRelease file around just as we do it with
       Release.gpg in case the new one we download isn't good for us
   * apt-pkg/deb/debmetaindex.cc:
     - reenable InRelease by default
-  
+  * ftparchive/writer.cc:
+    - use OpenMaybeClearSignedFile to be free from detecting and
+      skipping clearsigning metadata in dsc files
+
   [ Michael Vogt ]
   * add regression test for CVE-2013-1051
   * implement GPGSplit() based on the idea from Ansgar Burchardt
diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc
index 3065526ad..d26b160f9 100644
--- a/ftparchive/writer.cc
+++ b/ftparchive/writer.cc
@@ -20,6 +20,8 @@
 #include <apt-pkg/md5.h>
 #include <apt-pkg/hashes.h>
 #include <apt-pkg/deblistparser.h>
+#include <apt-pkg/fileutl.h>
+#include <apt-pkg/gpgv.h>
 
 #include <sys/types.h>
 #include <unistd.h>
@@ -598,77 +600,62 @@ SourcesWriter::SourcesWriter(string const &BOverrides,string const &SOverrides,
 // ---------------------------------------------------------------------
 /* */
 bool SourcesWriter::DoPackage(string FileName)
-{      
+{
    // Open the archive
-   FileFd F(FileName,FileFd::ReadOnly);
-   if (_error->PendingError() == true)
+   FileFd F;
+   if (OpenMaybeClearSignedFile(FileName, F) == false)
       return false;
-   
-   // Stat the file for later
-   struct stat St;
-   if (fstat(F.Fd(),&St) != 0)
-      return _error->Errno("fstat","Failed to stat %s",FileName.c_str());
 
-   if (St.st_size > 128*1024)
+   unsigned long long const FSize = F.FileSize();
+   //FIXME: do we really need to enforce a maximum size of the dsc file?
+   if (FSize > 128*1024)
       return _error->Error("DSC file '%s' is too large!",FileName.c_str());
-         
-   if (BufSize < (unsigned long long)St.st_size+1)
+
+   if (BufSize < FSize + 2)
    {
-      BufSize = St.st_size+1;
-      Buffer = (char *)realloc(Buffer,St.st_size+1);
+      BufSize = FSize + 2;
+      Buffer = (char *)realloc(Buffer , BufSize);
    }
-   
-   if (F.Read(Buffer,St.st_size) == false)
+
+   if (F.Read(Buffer, FSize) == false)
       return false;
 
+   // Stat the file for later (F might be clearsigned, so not F.FileSize())
+   struct stat St;
+   if (stat(FileName.c_str(), &St) != 0)
+      return _error->Errno("fstat","Failed to stat %s",FileName.c_str());
+
    // Hash the file
    char *Start = Buffer;
-   char *BlkEnd = Buffer + St.st_size;
-
-   MD5Summation MD5;
-   SHA1Summation SHA1;
-   SHA256Summation SHA256;
-   SHA256Summation SHA512;
-
-   if (DoMD5 == true)
-      MD5.Add((unsigned char *)Start,BlkEnd - Start);
-   if (DoSHA1 == true)
-      SHA1.Add((unsigned char *)Start,BlkEnd - Start);
-   if (DoSHA256 == true)
-      SHA256.Add((unsigned char *)Start,BlkEnd - Start);
-   if (DoSHA512 == true)
-      SHA512.Add((unsigned char *)Start,BlkEnd - Start);
+   char *BlkEnd = Buffer + FSize;
 
-   // Add an extra \n to the end, just in case
-   *BlkEnd++ = '\n';
-   
-   /* Remove the PGP trailer. Some .dsc's have this without a blank line 
-      before */
-   const char *Key = "-----BEGIN PGP SIGNATURE-----";
-   for (char *MsgEnd = Start; MsgEnd < BlkEnd - strlen(Key) -1; MsgEnd++)
+   Hashes DscHashes;
+   if (FSize == (unsigned long long) St.st_size)
    {
-      if (*MsgEnd == '\n' && strncmp(MsgEnd+1,Key,strlen(Key)) == 0)
-      {
-	 MsgEnd[1] = '\n';
-	 break;
-      }      
+      if (DoMD5 == true)
+	 DscHashes.MD5.Add((unsigned char *)Start,BlkEnd - Start);
+      if (DoSHA1 == true)
+	 DscHashes.SHA1.Add((unsigned char *)Start,BlkEnd - Start);
+      if (DoSHA256 == true)
+	 DscHashes.SHA256.Add((unsigned char *)Start,BlkEnd - Start);
+      if (DoSHA512 == true)
+	 DscHashes.SHA512.Add((unsigned char *)Start,BlkEnd - Start);
    }
-   
-   /* Read records until we locate the Source record. This neatly skips the
-      GPG header (which is RFC822 formed) without any trouble. */
-   pkgTagSection Tags;
-   do
+   else
    {
-      unsigned Pos;
-      if (Tags.Scan(Start,BlkEnd - Start) == false)
-	 return _error->Error("Could not find a record in the DSC '%s'",FileName.c_str());
-      if (Tags.Find("Source",Pos) == true)
-	 break;
-      Start += Tags.size();
+      FileFd DscFile(FileName, FileFd::ReadOnly);
+      DscHashes.AddFD(DscFile, St.st_size, DoMD5, DoSHA1, DoSHA256, DoSHA512);
    }
-   while (1);
+
+   // Add extra \n to the end, just in case (as in clearsigned they are missing)
+   *BlkEnd++ = '\n';
+   *BlkEnd++ = '\n';
+
+   pkgTagSection Tags;
+   if (Tags.Scan(Start,BlkEnd - Start) == false || Tags.Exists("Source") == false)
+      return _error->Error("Could not find a record in the DSC '%s'",FileName.c_str());
    Tags.Trim();
-      
+
    // Lookup the overide information, finding first the best priority.
    string BestPrio;
    string Bins = Tags.FindS("Binary");
@@ -732,25 +719,25 @@ bool SourcesWriter::DoPackage(string FileName)
    string const strippedName = flNotDir(FileName);
    std::ostringstream ostreamFiles;
    if (DoMD5 == true && Tags.Exists("Files"))
-      ostreamFiles << "\n " << string(MD5.Result()) << " " << St.st_size << " "
+      ostreamFiles << "\n " << string(DscHashes.MD5.Result()) << " " << St.st_size << " "
 		   << strippedName << "\n " << Tags.FindS("Files");
    string const Files = ostreamFiles.str();
 
    std::ostringstream ostreamSha1;
    if (DoSHA1 == true && Tags.Exists("Checksums-Sha1"))
-      ostreamSha1 << "\n " << string(SHA1.Result()) << " " << St.st_size << " "
+      ostreamSha1 << "\n " << string(DscHashes.SHA1.Result()) << " " << St.st_size << " "
 		   << strippedName << "\n " << Tags.FindS("Checksums-Sha1");
    string const ChecksumsSha1 = ostreamSha1.str();
 
    std::ostringstream ostreamSha256;
    if (DoSHA256 == true && Tags.Exists("Checksums-Sha256"))
-      ostreamSha256 << "\n " << string(SHA256.Result()) << " " << St.st_size << " "
+      ostreamSha256 << "\n " << string(DscHashes.SHA256.Result()) << " " << St.st_size << " "
 		   << strippedName << "\n " << Tags.FindS("Checksums-Sha256");
    string const ChecksumsSha256 = ostreamSha256.str();
 
    std::ostringstream ostreamSha512;
    if (Tags.Exists("Checksums-Sha512"))
-      ostreamSha512 << "\n " << string(SHA512.Result()) << " " << St.st_size << " "
+      ostreamSha512 << "\n " << string(DscHashes.SHA512.Result()) << " " << St.st_size << " "
 		   << strippedName << "\n " << Tags.FindS("Checksums-Sha512");
    string const ChecksumsSha512 = ostreamSha512.str();
 
diff --git a/test/integration/framework b/test/integration/framework
index 1c4872c8e..2ef61ca84 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -328,9 +328,15 @@ Package: $NAME" >> ${BUILDDIR}/debian/control
 	fi
 
 	echo '3.0 (native)' > ${BUILDDIR}/debian/source/format
-	local SRCS="$( (cd ${BUILDDIR}/..; dpkg-source -b ${NAME}-${VERSION} 2>&1) | grep '^dpkg-source: info: building' | grep -o '[a-z0-9._+~-]*$')"
-	for SRC in $SRCS; do
+	(cd ${BUILDDIR}/..; dpkg-source -b ${NAME}-${VERSION} 2>&1) | sed -n 's#^dpkg-source: info: building [^ ]\+ in ##p' \
+		| while read SRC; do
 		echo "pool/${SRC}" >> ${BUILDDIR}/../${RELEASE}.${DISTSECTION}.srclist
+#		if expr match "${SRC}" '.*\.dsc' >/dev/null 2>&1; then
+#			gpg --yes --no-default-keyring --secret-keyring ./keys/joesixpack.sec \
+#				--keyring ./keys/joesixpack.pub --default-key 'Joe Sixpack' \
+#				--clearsign -o "${BUILDDIR}/../${SRC}.sign" "${BUILDDIR}/../$SRC"
+#			mv "${BUILDDIR}/../${SRC}.sign" "${BUILDDIR}/../$SRC"
+#		fi
 	done
 
 	for arch in $(echo "$ARCH" | sed -e 's#,#\n#g' | sed -e "s#^native\$#$(getarchitecture 'native')#"); do