diff options
| -rw-r--r-- | apt-pkg/contrib/gpgv.cc | 205 | ||||
| -rwxr-xr-x | test/integration/test-cve-2013-1051-InRelease-parsing | 7 | ||||
| -rw-r--r-- | test/libapt/openmaybeclearsignedfile_test.cc | 39 |
3 files changed, 118 insertions, 133 deletions
diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index f8ab8d715..f6594e62e 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -20,6 +20,7 @@ #include <algorithm> #include <fstream> #include <iostream> +#include <memory> #include <sstream> #include <string> #include <vector> @@ -289,139 +290,131 @@ 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) +static bool GetLineErrno(std::unique_ptr<char, decltype(&free)> &buffer, size_t *n, FILE *stream, std::string const &InFile, bool acceptEoF = false) { - int result; - errno = 0; - result = getline(lineptr, n, stream); + auto lineptr = buffer.release(); + auto const result = getline(&lineptr, n, stream); + buffer.reset(lineptr); if (errno != 0) + return _error->Errno("getline", "Could not read from %s", InFile.c_str()); + if (result == -1) { - _error->Errno("getline", "Could not read from %s", InFile.c_str()); - return -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()); } - - return result; + // We remove all whitespaces including newline here as + // a) gpgv ignores them for signature + // b) we can write out a \n in code later instead of dealing with \r\n or not + _strrstrip(buffer.get()); + return true; } 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) + std::unique_ptr<FILE, decltype(&fclose)> in{fopen(InFile.c_str(), "r"), &fclose}; + if (in.get() == nullptr) return _error->Errno("fopen", "can not open %s", InFile.c_str()); - bool found_message_start = false; - bool found_message_end = false; - bool skip_until_empty_line = false; - bool found_signature = false; - bool first_line = true; - bool signed_message_not_on_first_line = false; - bool found_garbage = false; - - char *buf = NULL; + struct ScopedErrors + { + ScopedErrors() { _error->PushToStack(); } + ~ScopedErrors() { _error->MergeWithStack(); } + } scoped; + std::unique_ptr<char, decltype(&free)> buf{nullptr, &free}; size_t buf_size = 0; - _error->PushToStack(); - while (GetLineErrno(&buf, &buf_size, in, InFile) != -1) + + // start of the message + if (GetLineErrno(buf, &buf_size, in.get(), InFile) == false) + return false; // empty or read error + if (strcmp(buf.get(), "-----BEGIN PGP SIGNED MESSAGE-----") != 0) { - _strrstrip(buf); - if (found_message_start == false) - { - if (strcmp(buf, "-----BEGIN PGP SIGNED MESSAGE-----") == 0) - { - found_message_start = true; - skip_until_empty_line = true; - } - else - signed_message_not_on_first_line = found_garbage = true; - } - else if (skip_until_empty_line == true) - { - 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); - } - else if (found_signature == false) + // this might be an unsigned file we don't want to report errors for, + // but still finish unsuccessful none the less. + while (GetLineErrno(buf, &buf_size, in.get(), InFile, true)) + if (strcmp(buf.get(), "-----BEGIN PGP SIGNED MESSAGE-----") == 0) + return _error->Error("Clearsigned file '%s' does not start with a signed message block.", InFile.c_str()); + + return false; + } + + // save "Hash" Armor Headers + while (true) + { + if (GetLineErrno(buf, &buf_size, in.get(), InFile) == false) + return false; + if (*buf == '\0') + break; // empty line ends the Armor Headers + if (ContentHeader != NULL && strncmp(buf.get(), "Hash: ", strlen("Hash: ")) == 0) + ContentHeader->push_back(buf.get()); + } + + // the message itself + bool first_line = true; + bool good_write = true; + while (true) + { + if (good_write == false || GetLineErrno(buf, &buf_size, in.get(), InFile) == false) + return false; + + if (strcmp(buf.get(), "-----BEGIN PGP SIGNATURE-----") == 0) { - 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 + if (SignatureFile != nullptr) { - // 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)); + good_write &= SignatureFile->Write(buf.get(), strlen(buf.get())); + good_write &= SignatureFile->Write("\n", 1); } - else - found_garbage = true; + break; } - else if (found_signature == true) + + // we don't have any fields which need dash-escaped, + // but implementations are free to encode all lines … + char const *dashfree = buf.get(); + if (strncmp(dashfree, "- ", 2) == 0) + dashfree += 2; + if (first_line == true) // first line does not need a newline + first_line = false; + else if (ContentFile != nullptr) + good_write &= ContentFile->Write("\n", 1); + if (ContentFile != nullptr) + good_write &= ContentFile->Write(dashfree, strlen(dashfree)); + } + + // collect all signatures + bool open_signature = true; + while (true) + { + if (good_write == false) + return false; + if (GetLineErrno(buf, &buf_size, in.get(), InFile, true) == false) + break; + + if (open_signature && strcmp(buf.get(), "-----END PGP SIGNATURE-----") == 0) + open_signature = false; + else if (open_signature == false && strcmp(buf.get(), "-----BEGIN PGP SIGNATURE-----") == 0) + open_signature = true; + else if (open_signature == false) + return _error->Error("Clearsigned file '%s' contains unsigned lines.", InFile.c_str()); + + if (SignatureFile != nullptr) { - 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 + good_write &= SignatureFile->Write(buf.get(), strlen(buf.get())); + good_write &= SignatureFile->Write("\n", 1); } - // all the rest is whitespace, unsigned garbage or additional message blocks we ignore - else - found_garbage = true; } - fclose(in); - if (buf != NULL) - free(buf); + if (open_signature == true) + 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; } /*}}}*/ diff --git a/test/integration/test-cve-2013-1051-InRelease-parsing b/test/integration/test-cve-2013-1051-InRelease-parsing index 6238057c3..1f0cbda04 100755 --- a/test/integration/test-cve-2013-1051-InRelease-parsing +++ b/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 diff --git a/test/libapt/openmaybeclearsignedfile_test.cc b/test/libapt/openmaybeclearsignedfile_test.cc index 1f63fb8fc..4c6a0090f 100644 --- a/test/libapt/openmaybeclearsignedfile_test.cc +++ b/test/libapt/openmaybeclearsignedfile_test.cc @@ -190,19 +190,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 +241,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 +306,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 +336,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) |