From f0f13869316ad442de4acb4b76c367669a1f675c Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Mon, 19 Oct 2020 16:50:07 +0200 Subject: xmlsecurity: handle MDP permission during PDF verify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit 586f6abee92af3cdabdce034b607b9a046ed3946) Conflicts: include/vcl/filter/PDFiumLibrary.hxx vcl/source/pdf/PDFiumLibrary.cxx xmlsecurity/source/helper/pdfsignaturehelper.cxx Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105785 Tested-by: Jenkins Reviewed-by: Caolán McNamara (cherry picked from commit 00479937dc071246cc27f33fd6397668448a7ed9) Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107085 Tested-by: Michael Stahl Reviewed-by: Michael Stahl --- 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 +- .../qa/unit/pdfsigning/data/bad-cert-p1.pdf | Bin 0 -> 29646 bytes xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 28 ++++++- xmlsecurity/source/helper/pdfsignaturehelper.cxx | 5 +- xmlsecurity/source/pdfio/pdfdocument.cxx | 18 +++-- xmlsecurity/workben/pdfverify.cxx | 3 +- 10 files changed, 132 insertions(+), 26 deletions(-) create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx index ffc70874c19b..783b9a6da8b4 100644 --- a/include/vcl/filter/PDFiumLibrary.hxx +++ b/include/vcl/filter/PDFiumLibrary.hxx @@ -60,7 +60,7 @@ public: } /// Get bitmap checksum of the page, without annotations/commenting. - BitmapChecksum getChecksum(); + BitmapChecksum getChecksum(int nMDPPerm); }; class VCL_DLLPUBLIC PDFiumDocument final diff --git a/include/vcl/filter/pdfdocument.hxx b/include/vcl/filter/pdfdocument.hxx index 56debd53e8c7..06a2618a94ab 100644 --- a/include/vcl/filter/pdfdocument.hxx +++ b/include/vcl/filter/pdfdocument.hxx @@ -375,6 +375,7 @@ public: size_t GetObjectOffset(size_t nIndex) const; const std::vector>& GetElements(); std::vector 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. @@ -400,6 +401,11 @@ public: bool Write(SvStream& rStream); /// Get a list of signatures embedded into this document. std::vector 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 655450e28aa0..b668cb2d654a 100644 --- a/vcl/source/filter/ipdf/pdfdocument.cxx +++ b/vcl/source/filter/ipdf/pdfdocument.cxx @@ -1857,10 +1857,8 @@ static void visitPages(PDFObjectElement* pPages, std::vector& pPages->setVisiting(false); } -std::vector PDFDocument::GetPages() +PDFObjectElement* PDFDocument::GetCatalog() { - std::vector aRet; - PDFReferenceElement* pRoot = nullptr; PDFTrailerElement* pTrailer = nullptr; @@ -1880,11 +1878,18 @@ std::vector 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 PDFDocument::GetPages() +{ + std::vector aRet; + + PDFObjectElement* pCatalog = GetCatalog(); if (!pCatalog) { SAL_WARN("vcl.filter", "PDFDocument::GetPages: trailer has no catalog"); @@ -1957,6 +1962,71 @@ std::vector PDFDocument::GetSignatureWidgets() return aRet; } +int PDFDocument::GetMDPPerm() +{ + int nRet = 3; + + std::vector 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(pSig->Lookup("Reference")); + if (!pReference || pReference->GetElements().empty()) + { + continue; + } + + auto pFirstReference = dynamic_cast(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(pTransformParams); + if (!pTransformParamsDict) + { + auto pTransformParamsRef = dynamic_cast(pTransformParams); + if (pTransformParamsRef) + { + PDFObjectElement* pTransformParamsObj = pTransformParamsRef->LookupObject(); + if (pTransformParamsObj) + { + pTransformParamsDict = pTransformParamsObj->GetDictionary(); + } + } + } + + if (!pTransformParamsDict) + { + continue; + } + + auto pP = dynamic_cast(pTransformParamsDict->LookupElement("P")); + if (!pP) + { + return 2; + } + + return pP->GetValue(); + } + + return nRet; +} + std::vector 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 2bfa76e61559..f7b3caa2f791 100644 --- a/vcl/source/pdf/PDFiumLibrary.cxx +++ b/vcl/source/pdf/PDFiumLibrary.cxx @@ -60,7 +60,7 @@ std::unique_ptr PDFiumDocument::openPage(int nIndex) int PDFiumDocument::getPageCount() { return FPDF_GetPageCount(mpPdfDocument); } -BitmapChecksum PDFiumPage::getChecksum() +BitmapChecksum PDFiumPage::getChecksum(int nMDPPerm) { size_t nPageWidth = FPDF_GetPageWidth(mpPage); size_t nPageHeight = FPDF_GetPageHeight(mpPage); @@ -70,10 +70,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); { BitmapScopedWriteAccess pWriteAccess(aBitmap); diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx b/xmlsecurity/inc/pdfio/pdfdocument.hxx index f7e36492e746..87fa1d51286b 100644 --- a/xmlsecurity/inc/pdfio/pdfdocument.hxx +++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx @@ -36,7 +36,7 @@ namespace pdfio XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, - vcl::filter::PDFDocument& rDocument); + 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 8511f20eeae4..b35a1cd9a528 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -76,6 +76,7 @@ public: void testPDFPAdESGood(); /// Test a valid signature that does not cover the whole file. void testPartial(); + void testBadCertP1(); void testPartialInBetween(); /// Test writing a PAdES signature. void testSigningCertificateAttribute(); @@ -98,6 +99,7 @@ public: CPPUNIT_TEST(testPDF14LOWin); CPPUNIT_TEST(testPDFPAdESGood); CPPUNIT_TEST(testPartial); + CPPUNIT_TEST(testBadCertP1); CPPUNIT_TEST(testPartialInBetween); CPPUNIT_TEST(testSigningCertificateAttribute); CPPUNIT_TEST(testGood); @@ -145,8 +147,9 @@ std::vector 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()) @@ -287,8 +290,9 @@ void PDFSigningTest::testPDFRemove() std::vector aSignatures = aDocument.GetSignatureWidgets(); CPPUNIT_ASSERT_EQUAL(static_cast(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. @@ -437,6 +441,22 @@ void PDFSigningTest::testPartial() CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature); } +void PDFSigningTest::testBadCertP1() +{ + std::vector 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); +} + +/// Test writing a PAdES signature. void PDFSigningTest::testSigningCertificateAttribute() { // Create a new signature. diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx index 0398acac7ea0..2aea01128869 100644 --- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx +++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx @@ -53,11 +53,14 @@ bool PDFSignatureHelper::ReadAndVerifySignature( 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 557180071a2c..9d056de0a15c 100644 --- a/xmlsecurity/source/pdfio/pdfdocument.cxx +++ b/xmlsecurity/source/pdfio/pdfdocument.cxx @@ -139,7 +139,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& rPageChecksums) +void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector& rPageChecksums, + int nMDPPerm) { #if HAVE_FEATURE_PDFIUM auto pPdfium = vcl::pdf::PDFiumLibrary::get(); @@ -155,7 +156,7 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vectorgetChecksum(); + BitmapChecksum nPageChecksum = pPdfPage->getChecksum(nMDPPerm); rPageChecksums.push_back(nPageChecksum); } #else @@ -165,9 +166,9 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector aSignedPages; - AnalyizeSignatureStream(aSignatureStream, aSignedPages); + AnalyizeSignatureStream(aSignatureStream, aSignedPages, nMDPPerm); SvMemoryStream aFullStream; nPos = rStream.Tell(); @@ -191,7 +192,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu rStream.Seek(nPos); aFullStream.Seek(0); std::vector 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. @@ -204,7 +205,8 @@ namespace xmlsecurity namespace pdfio { bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, - SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument) + SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument, + int nMDPPerm) { vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V"); if (!pValue) @@ -311,7 +313,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 bc2978bb7c84..cd5c9ac5812b 100644 --- a/xmlsecurity/workben/pdfverify.cxx +++ b/xmlsecurity/workben/pdfverify.cxx @@ -156,11 +156,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; -- cgit