include/vcl/filter/PDFiumLibrary.hxx | 2 include/vcl/filter/pdfdocument.hxx | 6 + vcl/source/filter/ipdf/pdfdocument.cxx | 83 ++++++++++++++++++-- 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(+), 27 deletions(-)
New commits: commit 131a48cf746256128831a2b1203a1f629411196c Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Oct 19 16:50:07 2020 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Nov 19 09:25:37 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 (cherry picked from commit 00479937dc071246cc27f33fd6397668448a7ed9) Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106067 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 aee470cfd852..2f5a0bebfe98 100644 --- a/include/vcl/filter/PDFiumLibrary.hxx +++ b/include/vcl/filter/PDFiumLibrary.hxx @@ -173,7 +173,7 @@ public: std::unique_ptr<PDFiumTextPage> getTextPage(); /// 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 bd88d56e674d..5bb2a87482eb 100644 --- a/include/vcl/filter/pdfdocument.hxx +++ b/include/vcl/filter/pdfdocument.hxx @@ -576,6 +576,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. @@ -603,6 +604,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 701b96e38606..dd1355d924a8 100644 --- a/vcl/source/filter/ipdf/pdfdocument.cxx +++ b/vcl/source/filter/ipdf/pdfdocument.cxx @@ -38,7 +38,6 @@ namespace filter { const int MAX_SIGNATURE_CONTENT_LENGTH = 50000; - XRefEntry::XRefEntry() = default; PDFDocument::PDFDocument() = default; @@ -1943,10 +1942,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; @@ -1966,11 +1963,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"); @@ -2043,6 +2047,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 952a1e2803ca..3ccb1bf4135e 100644 --- a/vcl/source/pdf/PDFiumLibrary.cxx +++ b/vcl/source/pdf/PDFiumLibrary.cxx @@ -333,7 +333,7 @@ std::unique_ptr<PDFiumPathSegment> PDFiumPageObject::getPathSegment(int index) return pPDFiumPathSegment; } -BitmapChecksum PDFiumPage::getChecksum() +BitmapChecksum PDFiumPage::getChecksum(int nMDPPerm) { size_t nPageWidth = FPDF_GetPageWidth(mpPage); size_t nPageHeight = FPDF_GetPageHeight(mpPage); @@ -343,10 +343,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 4f62aae93281..e7d005c815cb 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -94,8 +94,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()) @@ -239,8 +240,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. @@ -408,6 +410,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 62eb7dc2fd91..a029ee9d20d1 100644 --- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx +++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx @@ -140,11 +140,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 ed7111faa7ae..7683a4cd71ee 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<BitmapChecksum>& rPageChecksums) +void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<BitmapChecksum>& rPageChecksums, + int nMDPPerm) { #if HAVE_FEATURE_PDFIUM auto pPdfium = vcl::pdf::PDFiumLibrary::get(); @@ -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); } #else @@ -166,9 +167,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)) @@ -183,7 +184,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(); @@ -192,7 +193,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 @@ 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) @@ -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