include/vcl/filter/PDFiumLibrary.hxx                |    2 
 include/vcl/filter/pdfdocument.hxx                  |    6 +
 vcl/source/filter/ipdf/pdfdocument.cxx              |   82 ++++++++++++++++++--
 vcl/source/pdf/PDFiumLibrary.cxx                    |   12 +-
 xmlsecurity/inc/pdfio/pdfdocument.hxx               |    2 
 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf |binary
 xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx       |   25 +++++-
 xmlsecurity/source/helper/pdfsignaturehelper.cxx    |    5 -
 xmlsecurity/source/pdfio/pdfdocument.cxx            |   17 ++--
 xmlsecurity/workben/pdfverify.cxx                   |    3 
 10 files changed, 130 insertions(+), 24 deletions(-)

New commits:
commit 255ff5f6f9f11f72fca48b337c8dc6f2d08e8d6b
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Oct 19 16:50:07 2020 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Dec 18 16:02:08 2020 +0100

    xmlsecurity: handle MDP permission during PDF verify
    
    (cherry picked from commit 586f6abee92af3cdabdce034b607b9a046ed3946)
    
    Conflicts:
            include/vcl/filter/PDFiumLibrary.hxx
            vcl/source/filter/ipdf/pdfdocument.cxx
            vcl/source/pdf/PDFiumLibrary.cxx
            xmlsecurity/inc/pdfio/pdfdocument.hxx
            xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
            xmlsecurity/source/helper/pdfsignaturehelper.cxx
    
    (cherry picked from commit 00479937dc071246cc27f33fd6397668448a7ed9)
    
    Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107966
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>

diff --git a/include/vcl/filter/PDFiumLibrary.hxx 
b/include/vcl/filter/PDFiumLibrary.hxx
index 60ece6b7cbc9..34b28787eb37 100644
--- a/include/vcl/filter/PDFiumLibrary.hxx
+++ b/include/vcl/filter/PDFiumLibrary.hxx
@@ -60,7 +60,7 @@ public:
     FPDF_PAGE getPointer() { return mpPage; }
 
     /// Get bitmap checksum of the page, without annotations/commenting.
-    BitmapChecksum getChecksum();
+    BitmapChecksum getChecksum(int nMDPPerm);
 };
 
 struct PDFiumLibrary : public rtl::StaticWithInit<std::shared_ptr<PDFium>, 
PDFiumLibrary>
diff --git a/include/vcl/filter/pdfdocument.hxx 
b/include/vcl/filter/pdfdocument.hxx
index 65f72f301c8f..a814a7da3c15 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -359,6 +359,7 @@ public:
     size_t GetObjectOffset(size_t nIndex) const;
     const std::vector< std::unique_ptr<PDFElement> >& GetElements();
     std::vector<PDFObjectElement*> GetPages();
+    PDFObjectElement* GetCatalog();
     /// Remember the end location of an EOF token.
     void PushBackEOF(size_t nOffset);
     /// Look up object based on object number, possibly by parsing object 
streams.
@@ -381,6 +382,11 @@ public:
     bool Write(SvStream& rStream);
     /// Get a list of signatures embedded into this document.
     std::vector<PDFObjectElement*> GetSignatureWidgets();
+    /**
+     * Get the value of the "modification detection and prevention" permission:
+     * Valid values are 1, 2 and 3: only 3 allows annotations after signing.
+     */
+    int GetMDPPerm();
     /// Remove the nth signature from read document in the edit buffer.
     bool RemoveSignature(size_t nPosition);
     /// Get byte offsets of the end of incremental updates.
diff --git a/vcl/source/filter/ipdf/pdfdocument.cxx 
b/vcl/source/filter/ipdf/pdfdocument.cxx
index 6851bef19b91..b17ffb25999f 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -1793,10 +1793,8 @@ static void visitPages(PDFObjectElement* pPages, 
std::vector<PDFObjectElement*>&
     pPages->setVisiting(false);
 }
 
-std::vector<PDFObjectElement*> PDFDocument::GetPages()
+PDFObjectElement* PDFDocument::GetCatalog()
 {
-    std::vector<PDFObjectElement*> aRet;
-
     PDFReferenceElement* pRoot = nullptr;
 
 
@@ -1817,11 +1815,18 @@ std::vector<PDFObjectElement*> PDFDocument::GetPages()
 
     if (!pRoot)
     {
-        SAL_WARN("vcl.filter", "PDFDocument::GetPages: trailer has no Root 
key");
-        return aRet;
+        SAL_WARN("vcl.filter", "PDFDocument::GetCatalog: trailer has no Root 
key");
+        return nullptr;
     }
 
-    PDFObjectElement* pCatalog = pRoot->LookupObject();
+    return pRoot->LookupObject();
+}
+
+std::vector<PDFObjectElement*> PDFDocument::GetPages()
+{
+    std::vector<PDFObjectElement*> aRet;
+
+    PDFObjectElement* pCatalog = GetCatalog();
     if (!pCatalog)
     {
         SAL_WARN("vcl.filter", "PDFDocument::GetPages: trailer has no 
catalog");
@@ -1896,6 +1901,71 @@ std::vector<PDFObjectElement*> 
PDFDocument::GetSignatureWidgets()
     return aRet;
 }
 
+int PDFDocument::GetMDPPerm()
+{
+    int nRet = 3;
+
+    std::vector<PDFObjectElement*> aSignatures = GetSignatureWidgets();
+    if (aSignatures.empty())
+    {
+        return nRet;
+    }
+
+    for (const auto& pSignature : aSignatures)
+    {
+        vcl::filter::PDFObjectElement* pSig = pSignature->LookupObject("V");
+        if (!pSig)
+        {
+            SAL_WARN("vcl.filter", "PDFDocument::GetMDPPerm: can't find 
signature object");
+            continue;
+        }
+
+        auto pReference = 
dynamic_cast<PDFArrayElement*>(pSig->Lookup("Reference"));
+        if (!pReference || pReference->GetElements().empty())
+        {
+            continue;
+        }
+
+        auto pFirstReference = 
dynamic_cast<PDFDictionaryElement*>(pReference->GetElements()[0]);
+        if (!pFirstReference)
+        {
+            SAL_WARN("vcl.filter",
+                     "PDFDocument::GetMDPPerm: reference array doesn't contain 
a dictionary");
+            continue;
+        }
+
+        PDFElement* pTransformParams = 
pFirstReference->LookupElement("TransformParams");
+        auto pTransformParamsDict = 
dynamic_cast<PDFDictionaryElement*>(pTransformParams);
+        if (!pTransformParamsDict)
+        {
+            auto pTransformParamsRef = 
dynamic_cast<PDFReferenceElement*>(pTransformParams);
+            if (pTransformParamsRef)
+            {
+                PDFObjectElement* pTransformParamsObj = 
pTransformParamsRef->LookupObject();
+                if (pTransformParamsObj)
+                {
+                    pTransformParamsDict = 
pTransformParamsObj->GetDictionary();
+                }
+            }
+        }
+
+        if (!pTransformParamsDict)
+        {
+            continue;
+        }
+
+        auto pP = 
dynamic_cast<PDFNumberElement*>(pTransformParamsDict->LookupElement("P"));
+        if (!pP)
+        {
+            return 2;
+        }
+
+        return pP->GetValue();
+    }
+
+    return nRet;
+}
+
 std::vector<unsigned char> PDFDocument::DecodeHexString(PDFHexStringElement 
const* pElement)
 {
     return svl::crypto::DecodeHexString(pElement->GetValue());
diff --git a/vcl/source/pdf/PDFiumLibrary.cxx b/vcl/source/pdf/PDFiumLibrary.cxx
index a8774b15bb4e..96dab5f62ff6 100644
--- a/vcl/source/pdf/PDFiumLibrary.cxx
+++ b/vcl/source/pdf/PDFiumLibrary.cxx
@@ -34,7 +34,7 @@ PDFium::PDFium()
 
 PDFium::~PDFium() { FPDF_DestroyLibrary(); }
 
-BitmapChecksum PDFiumPage::getChecksum()
+BitmapChecksum PDFiumPage::getChecksum(int nMDPPerm)
 {
     size_t nPageWidth = FPDF_GetPageWidth(mpPage);
     size_t nPageHeight = FPDF_GetPageHeight(mpPage);
@@ -44,10 +44,14 @@ BitmapChecksum PDFiumPage::getChecksum()
         return 0;
     }
 
-    // Intentionally not using FPDF_ANNOT here, annotations/commenting is OK 
to not affect the
-    // checksum, signature verification wants this.
+    int nFlags = 0;
+    if (nMDPPerm != 3)
+    {
+        // Annotations/commenting should affect the checksum, signature 
verification wants this.
+        nFlags = FPDF_ANNOT;
+    }
     FPDF_RenderPageBitmap(pPdfBitmap, mpPage, /*start_x=*/0, /*start_y=*/0, 
nPageWidth, nPageHeight,
-                          /*rotate=*/0, /*flags=*/0);
+                          /*rotate=*/0, nFlags);
     Bitmap aBitmap(Size(nPageWidth, nPageHeight), 24);
     {
         Bitmap::ScopedWriteAccess pWriteAccess(aBitmap);
diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx 
b/xmlsecurity/inc/pdfio/pdfdocument.hxx
index aaacae9db29f..5c8bf8b3557b 100644
--- a/xmlsecurity/inc/pdfio/pdfdocument.hxx
+++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx
@@ -29,7 +29,7 @@ namespace pdfio
  * @param rDocument the parsed document to see if the signature is partial.
  * @return If we can determinate a result.
  */
-XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, 
vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, 
vcl::filter::PDFDocument& rDocument);
+XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, 
vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, 
vcl::filter::PDFDocument& rDocument, int nMDPPerm);
 
 } // namespace pdfio
 } // namespace xmlsecurity
diff --git a/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf 
b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf
new file mode 100644
index 000000000000..04d9950582b0
Binary files /dev/null and 
b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf differ
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx 
b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 54fe2e7912f5..899a7567c4a3 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -72,6 +72,7 @@ public:
     /// Test a valid signature that does not cover the whole file.
     void testPartial();
     void testPartialInBetween();
+    void testBadCertP1();
     /// Test writing a PAdES signature.
     void testSigningCertificateAttribute();
     /// Test that we accept files which are supposed to be good.
@@ -94,6 +95,7 @@ public:
     CPPUNIT_TEST(testPDFPAdESGood);
     CPPUNIT_TEST(testPartial);
     CPPUNIT_TEST(testPartialInBetween);
+    CPPUNIT_TEST(testBadCertP1);
     CPPUNIT_TEST(testSigningCertificateAttribute);
     CPPUNIT_TEST(testGood);
     CPPUNIT_TEST(testTokenize);
@@ -139,7 +141,9 @@ std::vector<SignatureInformation> 
PDFSigningTest::verify(const OUString& rURL, s
     for (size_t i = 0; i < aSignatures.size(); ++i)
     {
         SignatureInformation aInfo(i);
-        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, 
aSignatures[i], aInfo, aVerifyDocument));
+        int nMDPPerm = aVerifyDocument.GetMDPPerm();
+        xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, 
aVerifyDocument,
+                                              nMDPPerm);
         aRet.push_back(aInfo);
 
         if (!rExpectedSubFilter.isEmpty())
@@ -268,7 +272,9 @@ void PDFSigningTest::testPDFRemove()
         std::vector<vcl::filter::PDFObjectElement*> aSignatures = 
aDocument.GetSignatureWidgets();
         CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aSignatures.size());
         SignatureInformation aInfo(0);
-        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, 
aSignatures[0], aInfo, aDocument));
+        int nMDPPerm = aDocument.GetMDPPerm();
+        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, 
aSignatures[0], aInfo,
+                                                             aDocument, 
nMDPPerm));
     }
 
     // Remove the signature and write out the result as remove.pdf.
@@ -398,6 +404,21 @@ void PDFSigningTest::testPartial()
     CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature);
 }
 
+void PDFSigningTest::testBadCertP1()
+{
+    std::vector<SignatureInformation> aInfos
+        = verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + 
"bad-cert-p1.pdf", 1,
+                 /*rExpectedSubFilter=*/OString());
+    CPPUNIT_ASSERT(!aInfos.empty());
+    SignatureInformation& rInformation = aInfos[0];
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 0 (SecurityOperationStatus_UNKNOWN)
+    // - Actual  : 1 (SecurityOperationStatus_OPERATION_SUCCEEDED)
+    // i.e. annotation after a P1 signature was not considered as a bad 
modification.
+    
CPPUNIT_ASSERT_EQUAL(xml::crypto::SecurityOperationStatus::SecurityOperationStatus_UNKNOWN,
+                         rInformation.nStatus);
+}
+
 void PDFSigningTest::testSigningCertificateAttribute()
 {
     // Create a new signature.
diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx 
b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
index 5e7684ca24c3..1f809fda1148 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -48,11 +48,14 @@ bool PDFSignatureHelper::ReadAndVerifySignature(const 
uno::Reference<io::XInputS
 
     m_aSignatureInfos.clear();
 
+    int nMDPPerm = aDocument.GetMDPPerm();
+
     for (size_t i = 0; i < aSignatures.size(); ++i)
     {
         SignatureInformation aInfo(i);
 
-        if (!xmlsecurity::pdfio::ValidateSignature(*pStream, aSignatures[i], 
aInfo, aDocument))
+        if (!xmlsecurity::pdfio::ValidateSignature(*pStream, aSignatures[i], 
aInfo, aDocument,
+                                                   nMDPPerm))
             SAL_WARN("xmlsecurity.helper", "failed to determine digest match");
 
         m_aSignatureInfos.push_back(aInfo);
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx 
b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 21ea3c324d1a..7b697d6d86eb 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -168,7 +168,8 @@ bool IsCompleteSignature(SvStream& rStream, 
vcl::filter::PDFDocument& rDocument,
 }
 
 /// Collects the checksum of each page of one version of the PDF.
-void AnalyizeSignatureStream(SvMemoryStream& rStream, 
std::vector<BitmapChecksum>& rPageChecksums)
+void AnalyizeSignatureStream(SvMemoryStream& rStream, 
std::vector<BitmapChecksum>& rPageChecksums,
+                             int nMDPPerm)
 {
 #if HAVE_FEATURE_PDFIUM
     auto pPdfium = vcl::pdf::PDFiumLibrary::get();
@@ -184,7 +185,7 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, 
std::vector<BitmapChecksum
             return;
         }
 
-        BitmapChecksum nPageChecksum = pPdfPage->getChecksum();
+        BitmapChecksum nPageChecksum = pPdfPage->getChecksum(nMDPPerm);
         rPageChecksums.push_back(nPageChecksum);
     }
     FPDF_CloseDocument(pPdfDocument);
@@ -195,9 +196,9 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, 
std::vector<BitmapChecksum
 
 /**
  * Checks if incremental updates after singing performed valid modifications 
only.
- * Annotations/commenting is OK, other changes are not.
+ * nMDPPerm decides if annotations/commenting is OK, other changes are always 
not.
  */
-bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* 
pSignature)
+bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* 
pSignature, int nMDPPerm)
 {
     size_t nSignatureEOF = 0;
     if (!GetEOFOfSignature(pSignature, nSignatureEOF))
@@ -212,7 +213,7 @@ bool IsValidSignature(SvStream& rStream, 
vcl::filter::PDFObjectElement* pSignatu
     rStream.Seek(nPos);
     aSignatureStream.Seek(0);
     std::vector<BitmapChecksum> aSignedPages;
-    AnalyizeSignatureStream(aSignatureStream, aSignedPages);
+    AnalyizeSignatureStream(aSignatureStream, aSignedPages, nMDPPerm);
 
     SvMemoryStream aFullStream;
     nPos = rStream.Tell();
@@ -221,7 +222,7 @@ bool IsValidSignature(SvStream& rStream, 
vcl::filter::PDFObjectElement* pSignatu
     rStream.Seek(nPos);
     aFullStream.Seek(0);
     std::vector<BitmapChecksum> aAllPages;
-    AnalyizeSignatureStream(aFullStream, aAllPages);
+    AnalyizeSignatureStream(aFullStream, aAllPages, nMDPPerm);
 
     // Fail if any page looks different after signing and at the end. 
Annotations/commenting doesn't
     // count, though.
@@ -234,7 +235,7 @@ namespace xmlsecurity
 namespace pdfio
 {
 
-bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* 
pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& 
rDocument)
+bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* 
pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& 
rDocument, int nMDPPerm)
 {
     vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V");
     if (!pValue)
@@ -337,7 +338,7 @@ bool ValidateSignature(SvStream& rStream, 
vcl::filter::PDFObjectElement* pSignat
         return false;
     }
     rInformation.bPartialDocumentSignature = !IsCompleteSignature(rStream, 
rDocument, pSignature);
-    if (!IsValidSignature(rStream, pSignature))
+    if (!IsValidSignature(rStream, pSignature, nMDPPerm))
     {
         SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: invalid incremental 
update detected");
         return false;
diff --git a/xmlsecurity/workben/pdfverify.cxx 
b/xmlsecurity/workben/pdfverify.cxx
index 92429864080b..bb66edb29981 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -147,11 +147,12 @@ int pdfVerify(int nArgc, char** pArgv)
         else
         {
             std::cerr << "found " << aSignatures.size() << " signatures" << 
std::endl;
+            int nMDPPerm = aDocument.GetMDPPerm();
             for (size_t i = 0; i < aSignatures.size(); ++i)
             {
                 SignatureInformation aInfo(i);
                 if (!xmlsecurity::pdfio::ValidateSignature(aStream, 
aSignatures[i], aInfo,
-                                                           aDocument))
+                                                           aDocument, 
nMDPPerm))
                 {
                     SAL_WARN("xmlsecurity.pdfio", "failed to determine digest 
match");
                     return 1;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to