diff options
| -rw-r--r-- | apt-pkg/contrib/gpgv.cc | 55 | ||||
| -rw-r--r-- | apt-pkg/contrib/gpgv.h | 22 | ||||
| -rw-r--r-- | debian/changelog | 6 | ||||
| -rw-r--r-- | ftparchive/writer.cc | 105 | ||||
| -rw-r--r-- | test/integration/framework | 10 |
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 |