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 | 18 ++-- xmlsecurity/workben/pdfverify.cxx | 3 10 files changed, 129 insertions(+), 26 deletions(-)
New commits: commit 00479937dc071246cc27f33fd6397668448a7ed9 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Oct 19 16:50:07 2020 +0200 Commit: Caolán McNamara <caol...@redhat.com> CommitDate: Sat Nov 14 16:32:15 2020 +0100 xmlsecurity: handle MDP permission during PDF verify (cherry picked from commit 586f6abee92af3cdabdce034b607b9a046ed3946) Conflicts: include/vcl/filter/PDFiumLibrary.hxx vcl/source/pdf/PDFiumLibrary.cxx xmlsecurity/source/helper/pdfsignaturehelper.cxx Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105785 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caol...@redhat.com> diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx index 639c71d61a3d..3e2851d21e0e 100644 --- a/include/vcl/filter/PDFiumLibrary.hxx +++ b/include/vcl/filter/PDFiumLibrary.hxx @@ -58,7 +58,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 638aef3b5a4d..ff52aa98d317 100644 --- a/include/vcl/filter/pdfdocument.hxx +++ b/include/vcl/filter/pdfdocument.hxx @@ -379,6 +379,7 @@ public: size_t GetObjectOffset(size_t nIndex) const; const std::vector<std::unique_ptr<PDFElement>>& GetElements() const; 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. @@ -404,6 +405,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 79efb85f9b97..b647421887e4 100644 --- a/vcl/source/filter/ipdf/pdfdocument.cxx +++ b/vcl/source/filter/ipdf/pdfdocument.cxx @@ -1862,10 +1862,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; PDFTrailerElement* pTrailer = nullptr; @@ -1885,11 +1883,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"); @@ -1962,6 +1967,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 861b7dda0acb..f481078ab726 100644 --- a/vcl/source/pdf/PDFiumLibrary.cxx +++ b/vcl/source/pdf/PDFiumLibrary.cxx @@ -57,7 +57,7 @@ std::unique_ptr<PDFiumPage> 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); @@ -67,10 +67,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 bb92e89a1161..59d86637c9b3 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -92,8 +92,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()) @@ -237,8 +238,9 @@ CPPUNIT_TEST_FIXTURE(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. @@ -406,6 +408,21 @@ CPPUNIT_TEST_FIXTURE(PDFSigningTest, testPartialInBetween) CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature); } +CPPUNIT_TEST_FIXTURE(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); +} + /// Test writing a PAdES signature. CPPUNIT_TEST_FIXTURE(PDFSigningTest, testSigningCertificateAttribute) { diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx index f10f29c61840..b0795cb8f33f 100644 --- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx +++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx @@ -52,11 +52,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 b229206391f2..3744b7b9290b 100644 --- a/xmlsecurity/source/pdfio/pdfdocument.cxx +++ b/xmlsecurity/source/pdfio/pdfdocument.cxx @@ -140,7 +140,8 @@ bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument, #if HAVE_FEATURE_PDFIUM /// 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) { auto pPdfium = vcl::pdf::PDFiumLibrary::get(); vcl::pdf::PDFiumDocument aPdfDocument( @@ -155,7 +156,7 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<BitmapChecksum return; } - BitmapChecksum nPageChecksum = pPdfPage->getChecksum(); + BitmapChecksum nPageChecksum = pPdfPage->getChecksum(nMDPPerm); rPageChecksums.push_back(nPageChecksum); } } @@ -163,9 +164,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)) @@ -181,7 +182,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(); @@ -190,7 +191,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. @@ -205,7 +206,8 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu namespace xmlsecurity::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) @@ -312,7 +314,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 b5052502573f..c448035946e6 100644 --- a/xmlsecurity/workben/pdfverify.cxx +++ b/xmlsecurity/workben/pdfverify.cxx @@ -157,11 +157,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