From 8c1dd12cb53ce141d8ade2c8abc619dcfa7f37a1 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 18 Mar 2013 08:08:37 +0100 Subject: * test/integration/framework: - continue after test failure but preserve exit status --- test/integration/framework | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'test/integration/framework') diff --git a/test/integration/framework b/test/integration/framework index 1c4872c8e..883b65bba 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1,5 +1,7 @@ #!/bin/sh -- # no runable script, just for vi +TESTFAILURES="no" + # we all like colorful messages if expr match "$(readlink -f /proc/$$/fd/1)" '/dev/pts/[0-9]\+' > /dev/null && \ expr match "$(readlink -f /proc/$$/fd/2)" '/dev/pts/[0-9]\+' > /dev/null; then @@ -36,7 +38,7 @@ msgtest() { } msgpass() { echo "${CPASS}PASS${CNORMAL}" >&2; } msgskip() { echo "${CWARNING}SKIP${CNORMAL}" >&2; } -msgfail() { echo "${CFAIL}FAIL${CNORMAL}" >&2; } +msgfail() { echo "${CFAIL}FAIL${CNORMAL}" >&2; TESTFAILURES="yes"; } # enable / disable Debugging MSGLEVEL=${MSGLEVEL:-3} @@ -113,9 +115,13 @@ gdb() { APT_CONFIG=aptconfig.conf LD_LIBRARY_PATH=${BUILDDIRECTORY} $(which gdb) ${BUILDDIRECTORY}/$1 } +exitwithstatus() { + [ "$TESTFAILURES" = "yes" ] && exit 1 || exit 0; +} + addtrap() { CURRENTTRAP="$CURRENTTRAP $1" - trap "$CURRENTTRAP exit;" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM + trap "$CURRENTTRAP exitwithstatus;" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM } setupenvironment() { -- cgit v1.2.3-70-g09d2 From f91bd741d223395cc3b1a609459e7d7226916e86 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 18 Mar 2013 11:38:19 +0100 Subject: report failures via exit and ensure we don't overflow --- test/integration/framework | 11 ++++++++--- test/integration/run-tests | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) (limited to 'test/integration/framework') diff --git a/test/integration/framework b/test/integration/framework index 883b65bba..cdaa20627 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1,6 +1,6 @@ #!/bin/sh -- # no runable script, just for vi -TESTFAILURES="no" +TESTFAILURES=0 # we all like colorful messages if expr match "$(readlink -f /proc/$$/fd/1)" '/dev/pts/[0-9]\+' > /dev/null && \ @@ -38,7 +38,7 @@ msgtest() { } msgpass() { echo "${CPASS}PASS${CNORMAL}" >&2; } msgskip() { echo "${CWARNING}SKIP${CNORMAL}" >&2; } -msgfail() { echo "${CFAIL}FAIL${CNORMAL}" >&2; TESTFAILURES="yes"; } +msgfail() { echo "${CFAIL}FAIL${CNORMAL}" >&2; TESTFAILURES=$((TESTFAILURES+1)); } # enable / disable Debugging MSGLEVEL=${MSGLEVEL:-3} @@ -116,7 +116,12 @@ gdb() { } exitwithstatus() { - [ "$TESTFAILURES" = "yes" ] && exit 1 || exit 0; + # error if we about to overflow, but ... + # "255 failures ought to be enough for everybody" + if [ $TESTFAILURES -gt 255 ]; then + msgdie "Total failure count $TESTFAILURES too big" + fi + exit $((TESTFAILURES <= 255 ? TESTFAILURES : 255)); } addtrap() { diff --git a/test/integration/run-tests b/test/integration/run-tests index 75f2ad662..18474b20f 100755 --- a/test/integration/run-tests +++ b/test/integration/run-tests @@ -37,4 +37,5 @@ for testcase in $(run-parts --list $DIR | grep '/test-'); do done echo "failures: $FAIL" -exit $FAIL +# ensure we don't overflow +exit $((FAIL <= 255 ? FAIL : 255)) -- cgit v1.2.3-70-g09d2 From 5d76cee187ea6f1443c6afd0d1cf99f3555304ef Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 18 Mar 2013 11:46:20 +0100 Subject: test/integration/framework: use EXIT_CODE to be consistent with the run-tests code --- test/integration/framework | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'test/integration/framework') diff --git a/test/integration/framework b/test/integration/framework index cdaa20627..4a70573c8 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1,6 +1,6 @@ #!/bin/sh -- # no runable script, just for vi -TESTFAILURES=0 +EXIT_CODE=0 # we all like colorful messages if expr match "$(readlink -f /proc/$$/fd/1)" '/dev/pts/[0-9]\+' > /dev/null && \ @@ -38,7 +38,7 @@ msgtest() { } msgpass() { echo "${CPASS}PASS${CNORMAL}" >&2; } msgskip() { echo "${CWARNING}SKIP${CNORMAL}" >&2; } -msgfail() { echo "${CFAIL}FAIL${CNORMAL}" >&2; TESTFAILURES=$((TESTFAILURES+1)); } +msgfail() { echo "${CFAIL}FAIL${CNORMAL}" >&2; EXIT_CODE=$((EXIT_CODE+1)); } # enable / disable Debugging MSGLEVEL=${MSGLEVEL:-3} @@ -118,10 +118,10 @@ gdb() { exitwithstatus() { # error if we about to overflow, but ... # "255 failures ought to be enough for everybody" - if [ $TESTFAILURES -gt 255 ]; then - msgdie "Total failure count $TESTFAILURES too big" + if [ $EXIT_CODE -gt 255 ]; then + msgdie "Total failure count $EXIT_CODE too big" fi - exit $((TESTFAILURES <= 255 ? TESTFAILURES : 255)); + exit $((EXIT_CODE <= 255 ? EXIT_CODE : 255)); } addtrap() { -- cgit v1.2.3-70-g09d2 From f1828b6977972b4ef6da6401602b7938f6570c32 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 18 Mar 2013 19:36:55 +0100 Subject: - 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 --- apt-pkg/contrib/gpgv.cc | 55 +++++++++++++++++++++++- apt-pkg/contrib/gpgv.h | 22 ++++++++++ debian/changelog | 6 ++- ftparchive/writer.cc | 105 ++++++++++++++++++++------------------------- test/integration/framework | 10 ++++- 5 files changed, 135 insertions(+), 63 deletions(-) (limited to 'test/integration/framework') 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 /*}}}*/ -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 #include +#include + #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 * 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 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 #include #include +#include +#include #include #include @@ -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 -- cgit v1.2.3-70-g09d2 From 233b78083f6f79730fcb5a6faeb74e2a78b6038a Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 18 Mar 2013 22:57:08 +0100 Subject: * apt-pkg/deb/debindexfile.cc, apt-pkg/deb/deblistparser.cc: - use OpenMaybeClearSignedFile to be free from detecting and skipping clearsigning metadata in dsc and Release files We can't write a "clean" file to disk as not all acquire methods copy Release files before checking them (e.g. cdrom), so this reverts recombining, but uses the method we use for dsc files also in the two places we deal with Release files --- apt-pkg/contrib/gpgv.cc | 44 ++--------------------------------------- apt-pkg/contrib/gpgv.h | 14 ------------- apt-pkg/deb/debindexfile.cc | 8 +++++++- apt-pkg/deb/deblistparser.cc | 12 +---------- apt-pkg/indexrecords.cc | 6 +++++- debian/changelog | 11 +++++------ test/integration/framework | 2 +- test/integration/test-apt-cdrom | 2 ++ 8 files changed, 23 insertions(+), 76 deletions(-) (limited to 'test/integration/framework') diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index fc16dd32c..7e244c623 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -215,7 +215,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, UNLINK_EXIT(EINTERNAL); } #undef UNLINK_EXIT - // we don't need the files any longer as we have the filedescriptors still open + // we don't need the files any longer unlink(sig); unlink(data); free(sig); @@ -235,52 +235,12 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, exit(WEXITSTATUS(Status)); } - /* looks like its fine. Our caller will check the status fd, - but we construct a good-known clear-signed file without garbage - and other non-sense. In a perfect world, we get the same file, - but empty lines, trailing whitespaces and stuff makes it inperfect … */ - if (RecombineToClearSignedFile(File, dataFd, dataHeader, sigFd) == false) - { - _error->DumpErrors(std::cerr); - exit(EINTERNAL); - } - - // everything fine, we have a clean file now! + // everything fine exit(0); } exit(EINTERNAL); // unreachable safe-guard } /*}}}*/ -// RecombineToClearSignedFile - combine data/signature to message /*{{{*/ -bool RecombineToClearSignedFile(std::string const &OutFile, int const ContentFile, - std::vector const &ContentHeader, int const SignatureFile) -{ - FILE *clean_file = fopen(OutFile.c_str(), "w"); - fputs("-----BEGIN PGP SIGNED MESSAGE-----\n", clean_file); - for (std::vector::const_iterator h = ContentHeader.begin(); h != ContentHeader.end(); ++h) - fprintf(clean_file, "%s\n", h->c_str()); - fputs("\n", clean_file); - - FILE *data_file = fdopen(ContentFile, "r"); - FILE *sig_file = fdopen(SignatureFile, "r"); - if (data_file == NULL || sig_file == NULL) - { - fclose(clean_file); - return _error->Error("Couldn't open splitfiles to recombine them into %s", OutFile.c_str()); - } - char *buf = NULL; - size_t buf_size = 0; - while (getline(&buf, &buf_size, data_file) != -1) - fputs(buf, clean_file); - fclose(data_file); - fputs("\n", clean_file); - while (getline(&buf, &buf_size, sig_file) != -1) - fputs(buf, clean_file); - fclose(sig_file); - fclose(clean_file); - return true; -} - /*}}}*/ // SplitClearSignedFile - split message into data/signature /*{{{*/ bool SplitClearSignedFile(std::string const &InFile, int const ContentFile, std::vector * const ContentHeader, int const SignatureFile) diff --git a/apt-pkg/contrib/gpgv.h b/apt-pkg/contrib/gpgv.h index ab7d35ab1..8cbe553bc 100644 --- a/apt-pkg/contrib/gpgv.h +++ b/apt-pkg/contrib/gpgv.h @@ -69,20 +69,6 @@ inline void ExecGPGV(std::string const &File, std::string const &FileSig, bool SplitClearSignedFile(std::string const &InFile, int const ContentFile, std::vector * const ContentHeader, int const SignatureFile); -/** \brief recombines message and signature to an inline signature - * - * Reverses the splitting down by #SplitClearSignedFile by writing - * a well-formed clear-signed message without unsigned messages, - * additional signed messages or just trailing whitespaces - * - * @param OutFile will be clear-signed file - * @param ContentFile is the Fd the message will be read from - * @param ContentHeader is a list of all required Amored Headers for the message - * @param SignatureFile is the Fd all signatures will be read from - */ -bool RecombineToClearSignedFile(std::string const &OutFile, int const ContentFile, - std::vector 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. diff --git a/apt-pkg/deb/debindexfile.cc b/apt-pkg/deb/debindexfile.cc index de645bb6e..909dfcf47 100644 --- a/apt-pkg/deb/debindexfile.cc +++ b/apt-pkg/deb/debindexfile.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include /*}}}*/ @@ -337,7 +338,12 @@ bool debPackagesIndex::Merge(pkgCacheGenerator &Gen,OpProgress *Prog) const if (releaseExists == true || FileExists(ReleaseFile) == true) { - FileFd Rel(ReleaseFile,FileFd::ReadOnly); + FileFd Rel; + // Beware: The 'Release' file might be clearsigned in case the + // signature for an 'InRelease' file couldn't be checked + if (OpenMaybeClearSignedFile(ReleaseFile, Rel) == false) + return false; + if (_error->PendingError() == true) return false; Parser.LoadReleaseInfo(File,Rel,Section); diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc index b84bd6fdd..2c014a734 100644 --- a/apt-pkg/deb/deblistparser.cc +++ b/apt-pkg/deb/deblistparser.cc @@ -800,13 +800,12 @@ bool debListParser::LoadReleaseInfo(pkgCache::PkgFileIterator &FileI, map_ptrloc const storage = WriteUniqString(component); FileI->Component = storage; - // FIXME: Code depends on the fact that Release files aren't compressed + // FIXME: should use FileFd and TagSection FILE* release = fdopen(dup(File.Fd()), "r"); if (release == NULL) return false; char buffer[101]; - bool gpgClose = false; while (fgets(buffer, sizeof(buffer), release) != NULL) { size_t len = 0; @@ -818,15 +817,6 @@ bool debListParser::LoadReleaseInfo(pkgCache::PkgFileIterator &FileI, if (buffer[len] == '\0') continue; - // only evalute the first GPG section - if (strncmp("-----", buffer, 5) == 0) - { - if (gpgClose == true) - break; - gpgClose = true; - continue; - } - // seperate the tag from the data const char* dataStart = strchr(buffer + len, ':'); if (dataStart == NULL) diff --git a/apt-pkg/indexrecords.cc b/apt-pkg/indexrecords.cc index af2639beb..1461a24bb 100644 --- a/apt-pkg/indexrecords.cc +++ b/apt-pkg/indexrecords.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -57,7 +58,10 @@ bool indexRecords::Exists(string const &MetaKey) const bool indexRecords::Load(const string Filename) /*{{{*/ { - FileFd Fd(Filename, FileFd::ReadOnly); + FileFd Fd; + if (OpenMaybeClearSignedFile(Filename, Fd) == false) + return false; + pkgTagFile TagFile(&Fd, Fd.Size() + 256); // XXX if (_error->PendingError() == true) { diff --git a/debian/changelog b/debian/changelog index 27fae657c..7c02b2689 100644 --- a/debian/changelog +++ b/debian/changelog @@ -8,19 +8,18 @@ apt (0.9.7.9) UNRELEASED; urgency=low and fix the inconsistency of returning in error cases - don't close stdout/stderr if it is also the statusfd - if ExecGPGV deals with a clear-signed file it will split this file - into data and signatures, pass it to gpgv for verification and - 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 + into data and signatures, pass it to gpgv for verification - 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: + * ftparchive/writer.cc, + apt-pkg/deb/debindexfile.cc, + apt-pkg/deb/deblistparser.cc: - use OpenMaybeClearSignedFile to be free from detecting and - skipping clearsigning metadata in dsc files + skipping clearsigning metadata in dsc and Release files [ Michael Vogt ] * add regression test for CVE-2013-1051 diff --git a/test/integration/framework b/test/integration/framework index 2ef61ca84..86e6ed7c3 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -114,7 +114,7 @@ gdb() { } addtrap() { - CURRENTTRAP="$CURRENTTRAP $1" + CURRENTTRAP="$1 $CURRENTTRAP" trap "$CURRENTTRAP exit;" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM } diff --git a/test/integration/test-apt-cdrom b/test/integration/test-apt-cdrom index f24c99b36..f1c4fd9d3 100755 --- a/test/integration/test-apt-cdrom +++ b/test/integration/test-apt-cdrom @@ -24,6 +24,8 @@ cat Translation-de | xz --format=lzma > Translation-de.lzma cat Translation-de | xz > Translation-de.xz rm Translation-en Translation-de cd - > /dev/null +addtrap "chmod -R +w $PWD/rootdir/media/cdrom/dists/;" +chmod -R -w rootdir/media/cdrom/dists aptcdrom add -m -o quiet=1 > apt-cdrom.log 2>&1 sed -i -e '/^Using CD-ROM/ d' -e '/gpgv/ d' -e '/^Identifying/ d' -e '/Reading / d' apt-cdrom.log -- cgit v1.2.3-70-g09d2