diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2020-09-04 17:17:48 +0200 |
---|---|---|
committer | Vasily Melenchuk <vasily.melenchuk@cib.de> | 2021-04-07 09:46:06 +0300 |
commit | 9a7166feef7373993c6c5540ae2a64c6e6d74cb6 (patch) | |
tree | c043210cfff39a42350a1774abe3d177009879c8 /xmlsecurity | |
parent | bdc1db1343d35d45182a1f6d983a3a823c16370a (diff) |
xmlsecurity: pdf incremental updates that are non-commenting are invalid
I.e. it's OK to add incremental updates for annotation/commenting
purposes and that doesn't invalite existing signatures. Everything else
does.
(cherry picked from commit 61834cd574568613f0b0a2ee099a60fa5a8d9804)
Conflicts:
include/vcl/filter/PDFiumLibrary.hxx
vcl/source/pdf/PDFiumLibrary.cxx
xmlsecurity/qa/unit/signing/signing.cxx
Change-Id: I4607c242b3c6f6b01517b02407e9e7a095e2e069
Diffstat (limited to 'xmlsecurity')
-rw-r--r-- | xmlsecurity/Library_xmlsecurity.mk | 5 | ||||
-rw-r--r-- | xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf | bin | 0 -> 17896 bytes | |||
-rw-r--r-- | xmlsecurity/qa/unit/signing/signing.cxx | 19 | ||||
-rw-r--r-- | xmlsecurity/source/pdfio/pdfdocument.cxx | 69 | ||||
-rw-r--r-- | xmlsecurity/workben/pdfverify.cxx | 5 |
5 files changed, 95 insertions, 3 deletions
diff --git a/xmlsecurity/Library_xmlsecurity.mk b/xmlsecurity/Library_xmlsecurity.mk index 9a65dd2152a9..0ad912d5e0cc 100644 --- a/xmlsecurity/Library_xmlsecurity.mk +++ b/xmlsecurity/Library_xmlsecurity.mk @@ -20,7 +20,10 @@ $(eval $(call gb_Library_add_defs,xmlsecurity,\ -DXMLSECURITY_DLLIMPLEMENTATION \ )) -$(eval $(call gb_Library_use_externals,xmlsecurity,boost_headers)) +$(eval $(call gb_Library_use_externals,xmlsecurity,\ + boost_headers \ + $(if $(filter PDFIUM,$(BUILD_TYPE)),pdfium) \ +)) $(eval $(call gb_Library_set_precompiled_header,xmlsecurity,$(SRCDIR)/xmlsecurity/inc/pch/precompiled_xmlsecurity)) diff --git a/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf b/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf Binary files differnew file mode 100644 index 000000000000..f2b1a71096b2 --- /dev/null +++ b/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx index bed9d0df8b0f..f9cdead4ae7f 100644 --- a/xmlsecurity/qa/unit/signing/signing.cxx +++ b/xmlsecurity/qa/unit/signing/signing.cxx @@ -96,6 +96,8 @@ public: void testPDFGood(); /// Test a typical PDF where the signature is bad. void testPDFBad(); + /// Test a maliciously manipulated signed pdf + void testPDFHideAndReplace(); /// Test a typical PDF which is not signed. void testPDFNo(); #endif @@ -141,6 +143,7 @@ public: #if HAVE_FEATURE_PDFIMPORT CPPUNIT_TEST(testPDFGood); CPPUNIT_TEST(testPDFBad); + CPPUNIT_TEST(testPDFHideAndReplace); CPPUNIT_TEST(testPDFNo); #endif CPPUNIT_TEST(test96097Calc); @@ -691,6 +694,22 @@ void SigningTest::testPDFBad() static_cast<int>(pObjectShell->GetDocumentSignatureState())); } +void SigningTest::testPDFHideAndReplace() +{ + createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + + "hide-and-replace-shadow-file-signed-2.pdf"); + SfxBaseModel* pBaseModel = dynamic_cast<SfxBaseModel*>(mxComponent.get()); + CPPUNIT_ASSERT(pBaseModel); + SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell(); + CPPUNIT_ASSERT(pObjectShell); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 2 (BROKEN) + // - Actual : 6 (NOTVALIDATED_PARTIAL_OK) + // i.e. a non-commenting update after a signature was not marked as invalid. + CPPUNIT_ASSERT_EQUAL(static_cast<int>(SignatureState::BROKEN), + static_cast<int>(pObjectShell->GetDocumentSignatureState())); +} + void SigningTest::testPDFNo() { createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + "no.pdf"); diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx index 7cf2c137c1c4..557180071a2c 100644 --- a/xmlsecurity/source/pdfio/pdfdocument.cxx +++ b/xmlsecurity/source/pdfio/pdfdocument.cxx @@ -12,6 +12,9 @@ #include <memory> #include <vector> +#include <config_features.h> + +#include <vcl/filter/PDFiumLibrary.hxx> #include <rtl/string.hxx> #include <rtl/ustrbuf.hxx> #include <sal/log.hxx> @@ -20,6 +23,7 @@ #include <svl/sigstruct.hxx> #include <svl/cryptosign.hxx> #include <vcl/filter/pdfdocument.hxx> +#include <vcl/bitmap.hxx> using namespace com::sun::star; @@ -133,6 +137,66 @@ bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument, size_t nFileEnd = rStream.Tell(); return std::find(rAllEOFs.begin(), rAllEOFs.end(), nFileEnd) != rAllEOFs.end(); } + +/// Collects the checksum of each page of one version of the PDF. +void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<BitmapChecksum>& rPageChecksums) +{ +#if HAVE_FEATURE_PDFIUM + auto pPdfium = vcl::pdf::PDFiumLibrary::get(); + vcl::pdf::PDFiumDocument aPdfDocument( + FPDF_LoadMemDocument(rStream.GetData(), rStream.GetSize(), /*password=*/nullptr)); + + int nPageCount = aPdfDocument.getPageCount(); + for (int nPage = 0; nPage < nPageCount; ++nPage) + { + std::unique_ptr<vcl::pdf::PDFiumPage> pPdfPage(aPdfDocument.openPage(nPage)); + if (!pPdfPage) + { + return; + } + + BitmapChecksum nPageChecksum = pPdfPage->getChecksum(); + rPageChecksums.push_back(nPageChecksum); + } +#else + (void)rStream; +#endif +} + +/** + * Checks if incremental updates after singing performed valid modifications only. + * Annotations/commenting is OK, other changes are not. + */ +bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature) +{ + size_t nSignatureEOF = 0; + if (!GetEOFOfSignature(pSignature, nSignatureEOF)) + { + return false; + } + + SvMemoryStream aSignatureStream; + sal_uInt64 nPos = rStream.Tell(); + rStream.Seek(0); + aSignatureStream.WriteStream(rStream, nSignatureEOF); + rStream.Seek(nPos); + aSignatureStream.Seek(0); + std::vector<BitmapChecksum> aSignedPages; + AnalyizeSignatureStream(aSignatureStream, aSignedPages); + + SvMemoryStream aFullStream; + nPos = rStream.Tell(); + rStream.Seek(0); + aFullStream.WriteStream(rStream); + rStream.Seek(nPos); + aFullStream.Seek(0); + std::vector<BitmapChecksum> aAllPages; + AnalyizeSignatureStream(aFullStream, aAllPages); + + // Fail if any page looks different after signing and at the end. Annotations/commenting doesn't + // count, though. + return aSignedPages == aAllPages; +} } namespace xmlsecurity @@ -247,6 +311,11 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat return false; } rInformation.bPartialDocumentSignature = !IsCompleteSignature(rStream, rDocument, pSignature); + if (!IsValidSignature(rStream, pSignature)) + { + SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: invalid incremental update detected"); + return false; + } // At this point there is no obviously missing info to validate the // signature. diff --git a/xmlsecurity/workben/pdfverify.cxx b/xmlsecurity/workben/pdfverify.cxx index 2754ea2f58c1..bc2978bb7c84 100644 --- a/xmlsecurity/workben/pdfverify.cxx +++ b/xmlsecurity/workben/pdfverify.cxx @@ -22,6 +22,7 @@ #include <vcl/svapp.hxx> #include <vcl/graphicfilter.hxx> #include <vcl/filter/pdfdocument.hxx> +#include <comphelper/scopeguard.hxx> #include <pdfio/pdfdocument.hxx> @@ -79,11 +80,11 @@ int pdfVerify(int nArgc, char** pArgv) uno::UNO_QUERY); comphelper::setProcessServiceFactory(xMultiServiceFactory); + InitVCL(); + comphelper::ScopeGuard g([] { DeInitVCL(); }); if (nArgc > 3 && OString(pArgv[3]) == "-p") { - InitVCL(); generatePreview(pArgv[1], pArgv[2]); - DeInitVCL(); return 0; } |