From ca334fac3be6eb4fd97513ad9b7e31c520249d09 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 Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105785 Tested-by: Jenkins Reviewed-by: Caolán McNamara (cherry picked from commit 00479937dc071246cc27f33fd6397668448a7ed9) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107062 Tested-by: Michael Stahl Reviewed-by: Michael Stahl --- xmlsecurity/inc/pdfio/pdfdocument.hxx | 2 +- .../qa/unit/pdfsigning/data/bad-cert-p1.pdf | Bin 0 -> 29646 bytes xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 25 +++++++++++++++++---- xmlsecurity/source/helper/pdfsignaturehelper.cxx | 5 ++++- xmlsecurity/source/pdfio/pdfdocument.cxx | 18 ++++++++------- xmlsecurity/workben/pdfverify.cxx | 3 ++- 6 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf (limited to 'xmlsecurity') 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 8a7cbbdc3730..aaca2c5acc63 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -96,8 +96,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()) @@ -241,8 +242,9 @@ CPPUNIT_TEST_FIXTURE(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. @@ -410,6 +412,21 @@ CPPUNIT_TEST_FIXTURE(PDFSigningTest, testPartialInBetween) CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature); } +CPPUNIT_TEST_FIXTURE(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. 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 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 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; -- cgit