- 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

diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc
index 5921d7c67cd5f4929b96b988d1db456e00017d09..fc16dd32c98c2093a9fbde8a513d0b37b33fe9e5 100644 (file)
--- a/apt-pkg/contrib/gpgv.cc
+++ b/apt-pkg/contrib/gpgv.cc
@@ -19,7 +19,7 @@
 
 #include <apti18n.h>
                                                                        /*}}}*/
 
 #include <apti18n.h>
                                                                        /*}}}*/

-char * GenerateTemporaryFileTemplate(const char *basename)             /*{{{*/
+static char * GenerateTemporaryFileTemplate(const char *basename)      /*{{{*/

 {
    const char *tmpdir = getenv("TMPDIR");
 #ifdef P_tmpdir
 {
    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);
 
       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;
 }
    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 8e04855e4c57fbbaba0aa933fbcc8bda8856996a..ab7d35ab1c7bae1de002101fd308733d9d16448b 100644 (file)
--- a/apt-pkg/contrib/gpgv.h
+++ b/apt-pkg/contrib/gpgv.h
@@ -12,6 +12,8 @@
 #include <string>
 #include <vector>
 
 #include <string>
 #include <vector>
 

+#include <apt-pkg/fileutl.h>
+

 #if __GNUC__ >= 4
        #define APT_noreturn    __attribute__ ((noreturn))
 #else
 #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 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
  *  @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);
  */
 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);
 
 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
 #endif

diff --git a/debian/changelog b/debian/changelog
index 3ef652c56721a4c3d893ad6c12e55b56c4481a7c..27fae657c3f064ab86eb31347e9945e3f6796dbb 100644 (file)
--- 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
       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
   * 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
   [ 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 3065526ad396ab2d8aced022d8ac24bf005eca20..d26b160f954368b1b8b62ecde1f4404cd30cdefc 100644 (file)
--- 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/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>
 
 #include <sys/types.h>
 #include <unistd.h>


@@ -598,77 +600,62 @@ SourcesWriter::SourcesWriter(string const &BOverrides,string const &SOverrides,
 // ---------------------------------------------------------------------
 /* */
 bool SourcesWriter::DoPackage(string FileName)
 // ---------------------------------------------------------------------
 /* */
 bool SourcesWriter::DoPackage(string FileName)

-{      
+{

    // Open the archive
    // Open the archive

-   FileFd F(FileName,FileFd::ReadOnly);
-   if (_error->PendingError() == true)
+   FileFd F;
+   if (OpenMaybeClearSignedFile(FileName, F) == false)

       return 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());
       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;
 
       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;
    // 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();
    Tags.Trim();

-      
+

    // Lookup the overide information, finding first the best priority.
    string BestPrio;
    string Bins = Tags.FindS("Binary");
    // 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"))
    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"))
                   << 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"))
                   << 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"))
                   << 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();
 
                   << strippedName << "\n " << Tags.FindS("Checksums-Sha512");
    string const ChecksumsSha512 = ostreamSha512.str();
 

diff --git a/test/integration/framework b/test/integration/framework
index 1c4872c8e148076256af7365bb1be6f51c41ae08..2ef61ca84cae352b39abe2fccd2a7752c9138d7e 100644 (file)
--- 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
        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
                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
        done
 
        for arch in $(echo "$ARCH" | sed -e 's#,#\n#g' | sed -e "s#^native\$#$(getarchitecture 'native')#"); do