Support cleartext signed InRelease files with CRLF line endings

Commit 89c4c588b275 ("fix from David Kalnischkies for the InRelease gpg
verification code (LP: #784473)") amended verification of cleartext
signatures by a check whether the file to be verified actually starts
with "-----BEGIN PGP SIGNATURE-----\n".

However cleartext signed InRelease files have been found in the wild
which use \r\n as line ending for this armor header line, presumably
generated by a Windows PGP client.  Such files are incorrectly deemed
unsigned and result in the following (misleading) error:

    Clearsigned file isn't valid, got 'NOSPLIT' (does the network require authentication?)

RFC 4880 specifies in 6.2 Forming ASCII Armor:

    That is to say, there is always a line ending preceding the
    starting five dashes, and following the ending five dashes.  The
    header lines, therefore, MUST start at the beginning of a line, and
    MUST NOT have text other than whitespace following them on the same
    line.

RFC 4880 does not seem to specify whether LF or CRLF is used as line
ending for armor headers, but CR is generally considered whitespace
(e.g. "man perlrecharclass"), hence using CRLF is legal even under
the assumption that LF must be used.

SplitClearSignedFile() is stripping whitespace (including CR) on lineend
already before matching the string, so StartsWithGPGClearTextSignature() is
adapted to use the same ignoring. As the earlier method is responsible
for what apt will end up actually parsing nowadays as signed/unsigned this
change has no implications for security.

Thanks: Lukas Wunner for detailed report & initial patch!
References: 89c4c588b275d098af33f36eeddea6fd75068342
Closes: 884922

1 files changed, 41 insertions, 10 deletions

diff --git a/test/libapt/openmaybeclearsignedfile_test.cc b/test/libapt/openmaybeclearsignedfile_test.cc
index 40735812d..1f63fb8fc 100644
--- a/test/libapt/openmaybeclearsignedfile_test.cc
+++ b/test/libapt/openmaybeclearsignedfile_test.cc
@@ -33,7 +33,7 @@ TEST(OpenMaybeClearSignedFileTest,SimpleSignedFile)
 "JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
 "=TB1F\n"
 "-----END PGP SIGNATURE-----\n");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
@@ -64,7 +64,7 @@ TEST(OpenMaybeClearSignedFileTest,WhitespaceSignedFile)
 "JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq			\n"
 "=TB1F	\n"
 "-----END PGP SIGNATURE-----");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
@@ -100,7 +100,7 @@ TEST(OpenMaybeClearSignedFileTest,SignedFileWithContentHeaders)
 "JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
 "=TB1F\n"
 "-----END PGP SIGNATURE-----\n");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
@@ -142,7 +142,7 @@ TEST(OpenMaybeClearSignedFileTest,SignedFileWithTwoSignatures)
 "ASc9hsAZRG0xHuRU0F94V/XrkWw8QYAobJ/yxvs4L0EuA4optbSqawDB\n"
 "=CP8j\n"
 "-----END PGP SIGNATURE-----\n");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
@@ -188,8 +188,8 @@ TEST(OpenMaybeClearSignedFileTest,TwoSimpleSignedFile)
 "JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
 "=TB1F\n"
 "-----END PGP SIGNATURE-----");
-
    EXPECT_TRUE(_error->empty());
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
@@ -211,7 +211,7 @@ TEST(OpenMaybeClearSignedFileTest,UnsignedFile)
    std::string tempfile;
    FileFd fd;
    createTemporaryFile("unsignedfile", fd, &tempfile, "Test");
-
+   EXPECT_FALSE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
       unlink(tempfile.c_str());
@@ -242,7 +242,7 @@ TEST(OpenMaybeClearSignedFileTest,GarbageTop)
 "JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
 "=TB1F\n"
 "-----END PGP SIGNATURE-----\n");
-
+   EXPECT_FALSE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(_error->empty());
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
@@ -260,6 +260,37 @@ TEST(OpenMaybeClearSignedFileTest,GarbageTop)
    EXPECT_EQ("Clearsigned file '" + tempfile + "' does not start with a signed message block.", msg);
 }
 
+TEST(OpenMaybeClearSignedFileTest,GarbageHeader)
+{
+   std::string tempfile;
+   FileFd fd;
+   createTemporaryFile("garbageheader", fd, &tempfile, "-----BEGIN PGP SIGNED MESSAGE----- Garbage\n"
+"Hash: SHA512\n"
+"\n"
+"Test\n"
+"-----BEGIN PGP SIGNATURE-----\n"
+"\n"
+"iQFEBAEBCgAuFiEENKjp0Y2zIPNn6OqgWpDRQdusja4FAlhT7+kQHGpvZUBleGFt\n"
+"cGxlLm9yZwAKCRBakNFB26yNrjvEB/9/e3jA1l0fvPafx9LEXcH8CLpUFQK7ra9l\n"
+"3M4YAH4JKQlTG1be7ixruBRlCTh3YiSs66fKMeJeUYoxA2HPhvbGFEjQFAxunEYg\n"
+"X/LBKv1mQWa+Q34P5GBjK8kQdLCN+yJAiUErmWNQG3GPninrxsC9tY5jcWvHeP1k\n"
+"V7N3MLnNqzXaCJM24mnKidC5IDadUdQ8qC8c3rjUexQ8vBz0eucH56jbqV5oOcvx\n"
+"pjlW965dCPIf3OI8q6J7bIOjyY+u/PTcVlqPq3TUz/ti6RkVbKpLH0D4ll3lUTns\n"
+"JQt/+gJCPxHUJphy8sccBKhW29CLELJIIafvU30E1nWn9szh2Xjq\n"
+"=TB1F\n"
+"-----END PGP SIGNATURE-----\n");
+   EXPECT_FALSE(StartsWithGPGClearTextSignature(tempfile));
+   // beware: the file will be successfully opened as unsigned file
+   EXPECT_TRUE(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, "-----BEGIN PGP SIGNED MESSAGE----- Garbage\n");
+   EXPECT_FALSE(fd.Eof());
+}
+
 TEST(OpenMaybeClearSignedFileTest,GarbageBottom)
 {
    std::string tempfile;
@@ -280,7 +311,7 @@ TEST(OpenMaybeClearSignedFileTest,GarbageBottom)
 "=TB1F\n"
 "-----END PGP SIGNATURE-----\n"
 "Garbage");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(_error->empty());
    EXPECT_TRUE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
@@ -306,7 +337,7 @@ TEST(OpenMaybeClearSignedFileTest,BogusNoSig)
 "Hash: SHA512\n"
 "\n"
 "Test");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(_error->empty());
    EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)
@@ -328,7 +359,7 @@ TEST(OpenMaybeClearSignedFileTest,BogusSigStart)
 "\n"
 "Test\n"
 "-----BEGIN PGP SIGNATURE-----");
-
+   EXPECT_TRUE(StartsWithGPGClearTextSignature(tempfile));
    EXPECT_TRUE(_error->empty());
    EXPECT_FALSE(OpenMaybeClearSignedFile(tempfile, fd));
    if (tempfile.empty() == false)