xmlsecurity PDF verify: support non-detached signatures

And a couple of other changes to accept the bugdoc from
<https://github.com/esig/dss/
dss-pades/target/test-classes/plugtest/esig2014/ESIG-PAdES/RO/Signature-P-RO-4.pdf>.

Change-Id: I0fca9ba0bfe927ef91ae2592a5026b05d19879fd
Reviewed-on: https://gerrit.libreoffice.org/31462
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Jenkins <ci@libreoffice.org>

3 files changed, 38 insertions, 11 deletions

diff --git a/xmlsecurity/qa/unit/pdfsigning/data/good-non-detached.pdf b/xmlsecurity/qa/unit/pdfsigning/data/good-non-detached.pdf
new file mode 100644
index 000000000000..8e5b2151159c
--- /dev/null
+++ b/xmlsecurity/qa/unit/pdfsigning/data/good-non-detached.pdf
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 8932d6fc8d7f..5b88c71d90d2 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -64,7 +64,10 @@ public:
     void testPDF14LOWin();
     /// Test a PAdES document, signed by LO on Linux.
     void testPDFPAdESGood();
+    /// Test writing a PAdES signature.
     void testSigningCertificateAttribute();
+    /// Test that we accept files which are supposed to be good.
+    void testGood();
 
     CPPUNIT_TEST_SUITE(PDFSigningTest);
     CPPUNIT_TEST(testPDFAdd);
@@ -77,6 +80,7 @@ public:
     CPPUNIT_TEST(testPDF14LOWin);
     CPPUNIT_TEST(testPDFPAdESGood);
     CPPUNIT_TEST(testSigningCertificateAttribute);
+    CPPUNIT_TEST(testGood);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -343,6 +347,25 @@ void PDFSigningTest::testSigningCertificateAttribute()
     CPPUNIT_ASSERT(rInformation.bHasSigningCertificate);
 }
 
+void PDFSigningTest::testGood()
+{
+#ifndef _WIN32
+    const std::initializer_list<OUStringLiteral> aNames =
+    {
+        // We failed to determine if this is good or bad.
+        OUStringLiteral("good-non-detached.pdf"),
+    };
+
+    for (const auto& rName : aNames)
+    {
+        std::vector<SignatureInformation> aInfos = verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + rName, 1, /*rExpectedSubFilter=*/OString());
+        CPPUNIT_ASSERT(!aInfos.empty());
+        SignatureInformation& rInformation = aInfos[0];
+        CPPUNIT_ASSERT_EQUAL(xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED, rInformation.nStatus);
+    }
+#endif
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(PDFSigningTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 2092369404ac..e3e89a0750ee 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -1552,7 +1552,7 @@ void PDFDocument::ReadXRefStream(SvStream& rStream)
         nLineLength += aW[i];
     }
 
-    if (nLineLength - 1 != nColumns)
+    if (nPredictor > 1 && nLineLength - 1 != nColumns)
     {
         SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ReadXRefStream: /DecodeParms/Columns is inconsistent with /W");
         return;
@@ -1573,7 +1573,7 @@ void PDFDocument::ReadXRefStream(SvStream& rStream)
             size_t nIndex = nFirstObject + nEntry;
 
             aStream.ReadBytes(aOrigLine.data(), aOrigLine.size());
-            if (aOrigLine[0] + 10 != nPredictor)
+            if (nPredictor > 1 && aOrigLine[0] + 10 != nPredictor)
             {
                 SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ReadXRefStream: in-stream predictor is inconsistent with /DecodeParms/Predictor for object #" << nIndex);
                 return;
@@ -2116,7 +2116,7 @@ bool PDFDocument::ValidateSignature(SvStream& rStream, PDFObjectElement* pSignat
     }
 
     auto pSubFilter = dynamic_cast<PDFNameElement*>(pValue->Lookup("SubFilter"));
-    if (!pSubFilter || (pSubFilter->GetValue() != "adbe.pkcs7.detached" && pSubFilter->GetValue() != "ETSI.CAdES.detached"))
+    if (!pSubFilter || (pSubFilter->GetValue() != "adbe.pkcs7.detached" && pSubFilter->GetValue() != "adbe.pkcs7.sha1" && pSubFilter->GetValue() != "ETSI.CAdES.detached"))
     {
         SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ValidateSignature: no or unsupported sub-filter");
         return false;
@@ -2415,15 +2415,19 @@ bool PDFDocument::ValidateSignature(SvStream& rStream, PDFObjectElement* pSignat
     SECItem* pContentInfoContentData = pCMSSignedData->contentInfo.content.data;
     if (pContentInfoContentData && pContentInfoContentData->data)
     {
-        SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ValidateSignature: expected nullptr content info");
-        return false;
+        // Not a detached signature.
+        if (!memcmp(pActualResultBuffer, pContentInfoContentData->data, nMaxResultLen) && nActualResultLen == pContentInfoContentData->len)
+            rInformation.nStatus = xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;
+    }
+    else
+    {
+        // Detached, the usual case.
+        SECItem aActualResultItem;
+        aActualResultItem.data = pActualResultBuffer;
+        aActualResultItem.len = nActualResultLen;
+        if (NSS_CMSSignerInfo_Verify(pCMSSignerInfo, &aActualResultItem, nullptr) == SECSuccess)
+            rInformation.nStatus = xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;
     }
-
-    SECItem aActualResultItem;
-    aActualResultItem.data = pActualResultBuffer;
-    aActualResultItem.len = nActualResultLen;
-    if (NSS_CMSSignerInfo_Verify(pCMSSignerInfo, &aActualResultItem, nullptr) == SECSuccess)
-        rInformation.nStatus = xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;
 
     // Everything went fine
     PORT_Free(pActualResultBuffer);