[Libreoffice-commits] core.git: Branch 'feature/cib_contract138c' - 3 commits - include/vcl vcl/CppunitTest_vcl_filter_ipdf.mk vcl/Module_vcl.mk vcl/qa vcl/source xmlsecurity/inc xmlsecurity/Library_xmlsecurity.mk xmlsecurity/qa xmlsecurity/source xmlsecurity/workben
Miklos Vajna (via logerrit)
logerrit at kemper.freedesktop.org
Tue Apr 6 19:00:03 UTC 2021
Rebased ref, commits from common ancestor:
commit 34fb117dfe1e53ab3a0dd1f999ebfcd554d1c1f4
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon Oct 19 16:50:07 2020 +0200
Commit: Vasily Melenchuk <vasily.melenchuk at cib.de>
CommitDate: Tue Apr 6 21:57:53 2021 +0300
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
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105785
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
(cherry picked from commit 00479937dc071246cc27f33fd6397668448a7ed9)
Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107085
Tested-by: Michael Stahl <michael.stahl at cib.de>
Reviewed-by: Michael Stahl <michael.stahl at cib.de>
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<std::unique_ptr<PDFElement>>& GetElements();
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.
@@ -400,6 +401,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 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<PDFObjectElement*>&
pPages->setVisiting(false);
}
-std::vector<PDFObjectElement*> PDFDocument::GetPages()
+PDFObjectElement* PDFDocument::GetCatalog()
{
- std::vector<PDFObjectElement*> aRet;
-
PDFReferenceElement* pRoot = nullptr;
PDFTrailerElement* pTrailer = nullptr;
@@ -1880,11 +1878,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");
@@ -1957,6 +1962,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 3ad6b5ee0780..e9ccab2d1861 100644
--- a/vcl/source/pdf/PDFiumLibrary.cxx
+++ b/vcl/source/pdf/PDFiumLibrary.cxx
@@ -59,7 +59,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);
@@ -69,10 +69,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<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())
@@ -287,8 +290,9 @@ void 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.
@@ -437,6 +441,22 @@ void PDFSigningTest::testPartial()
CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature);
}
+void 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.
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<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
@@ -165,9 +166,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))
@@ -182,7 +183,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();
@@ -191,7 +192,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.
@@ -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;
commit 83570d53b1a9eb8d61d0d18275ef89e199899a97
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Oct 16 18:15:21 2020 +0200
Commit: Vasily Melenchuk <vasily.melenchuk at cib.de>
CommitDate: Tue Apr 6 21:57:52 2021 +0300
vcl pdf tokenizer: fix handling of dict -> array -> dict tokens
Needed to be able to parse the /Reference key of signatures.
(cherry picked from commit 056c1284d6a68525002c54bef10834cc135385db)
Conflicts:
vcl/qa/cppunit/filter/ipdf/ipdf.cxx
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105626
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
(cherry picked from commit 8f46af565680bef0ff8ca32781e6d813a7446543)
Change-Id: I6b81089a3f58a2de461ad92ca5a891c284f8686a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107084
Tested-by: Michael Stahl <michael.stahl at cib.de>
Reviewed-by: Michael Stahl <michael.stahl at cib.de>
diff --git a/vcl/CppunitTest_vcl_filter_ipdf.mk b/vcl/CppunitTest_vcl_filter_ipdf.mk
new file mode 100644
index 000000000000..403836ac781a
--- /dev/null
+++ b/vcl/CppunitTest_vcl_filter_ipdf.mk
@@ -0,0 +1,49 @@
+# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
+#*************************************************************************
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+#*************************************************************************
+
+$(eval $(call gb_CppunitTest_CppunitTest,vcl_filter_ipdf))
+
+$(eval $(call gb_CppunitTest_use_externals,vcl_filter_ipdf,\
+ boost_headers \
+ pdfium \
+))
+
+$(eval $(call gb_CppunitTest_add_exception_objects,vcl_filter_ipdf, \
+ vcl/qa/cppunit/filter/ipdf/ipdf \
+))
+
+$(eval $(call gb_CppunitTest_use_libraries,vcl_filter_ipdf, \
+ comphelper \
+ cppu \
+ sal \
+ sfx \
+ svx \
+ test \
+ tl \
+ unotest \
+ utl \
+ vcl \
+))
+
+$(eval $(call gb_CppunitTest_use_sdk_api,vcl_filter_ipdf))
+
+$(eval $(call gb_CppunitTest_use_ure,vcl_filter_ipdf))
+$(eval $(call gb_CppunitTest_use_vcl,vcl_filter_ipdf))
+
+$(eval $(call gb_CppunitTest_use_rdb,vcl_filter_ipdf,services))
+
+$(eval $(call gb_CppunitTest_use_custom_headers,vcl_filter_ipdf,\
+ officecfg/registry \
+))
+
+$(eval $(call gb_CppunitTest_use_configuration,vcl_filter_ipdf))
+
+# vim: set noet sw=4 ts=4:
diff --git a/vcl/Module_vcl.mk b/vcl/Module_vcl.mk
index cd50d11771f1..380393498fe6 100644
--- a/vcl/Module_vcl.mk
+++ b/vcl/Module_vcl.mk
@@ -257,4 +257,10 @@ $(eval $(call gb_Module_add_slowcheck_targets,vcl,\
))
endif
+ifneq (,$(filter PDFIUM,$(BUILD_TYPE)))
+$(eval $(call gb_Module_add_slowcheck_targets,vcl,\
+ CppunitTest_vcl_filter_ipdf \
+))
+endif
+
# vim: set noet sw=4 ts=4:
diff --git a/vcl/qa/cppunit/filter/ipdf/data/dict-array-dict.pdf b/vcl/qa/cppunit/filter/ipdf/data/dict-array-dict.pdf
new file mode 100644
index 000000000000..73de3117b9a6
--- /dev/null
+++ b/vcl/qa/cppunit/filter/ipdf/data/dict-array-dict.pdf
@@ -0,0 +1,55 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+>>
+endobj
+2 0 obj <<
+ /Type /Pages
+ /MediaBox [0 0 200 300]
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+ /Contents 4 0 R
+ /Key[<</InnerKey 42>>]
+>>
+endobj
+4 0 obj <<
+ /Length 188
+>>
+stream
+q
+0 0 0 rg
+0 290 10 10 re B*
+10 150 50 30 re B*
+0 0 1 rg
+190 290 10 10 re B*
+70 232 50 30 re B*
+0 1 0 rg
+190 0 10 10 re B*
+130 150 50 30 re B*
+1 0 0 rg
+0 0 10 10 re B*
+70 67 50 30 re B*
+Q
+endstream
+endobj
+xref
+0 5
+0000000000 65535 f
+0000000015 00000 n
+0000000068 00000 n
+0000000157 00000 n
+0000000251 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 5
+>>
+startxref
+491
+%%EOF
diff --git a/vcl/qa/cppunit/filter/ipdf/ipdf.cxx b/vcl/qa/cppunit/filter/ipdf/ipdf.cxx
new file mode 100644
index 000000000000..917630265a0f
--- /dev/null
+++ b/vcl/qa/cppunit/filter/ipdf/ipdf.cxx
@@ -0,0 +1,81 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <test/bootstrapfixture.hxx>
+#include <unotest/macros_test.hxx>
+
+#include <com/sun/star/drawing/XDrawPagesSupplier.hpp>
+#include <com/sun/star/drawing/XDrawView.hpp>
+#include <com/sun/star/frame/Desktop.hpp>
+#include <com/sun/star/security/XCertificate.hpp>
+#include <com/sun/star/view/XSelectionSupplier.hpp>
+#include <com/sun/star/xml/crypto/SEInitializer.hpp>
+
+#include <comphelper/propertyvalue.hxx>
+#include <osl/file.hxx>
+#include <unotools/tempfile.hxx>
+#include <sfx2/sfxbasemodel.hxx>
+#include <svx/svdview.hxx>
+#include <sfx2/viewsh.hxx>
+#include <sfx2/objsh.hxx>
+#include <vcl/filter/PDFiumLibrary.hxx>
+#include <vcl/filter/pdfdocument.hxx>
+
+using namespace ::com::sun::star;
+
+namespace
+{
+char const DATA_DIRECTORY[] = "/vcl/qa/cppunit/filter/ipdf/data/";
+}
+
+/// Covers vcl/source/filter/ipdf/ fixes.
+class VclFilterIpdfTest : public test::BootstrapFixture, public unotest::MacrosTest
+{
+public:
+ void setUp() override;
+
+ void testDictArrayDict();
+
+ CPPUNIT_TEST_SUITE(VclFilterIpdfTest);
+ CPPUNIT_TEST(testDictArrayDict);
+ CPPUNIT_TEST_SUITE_END();
+};
+
+void VclFilterIpdfTest::setUp()
+{
+ test::BootstrapFixture::setUp();
+
+ mxDesktop.set(frame::Desktop::create(m_xContext));
+}
+
+void VclFilterIpdfTest::testDictArrayDict()
+{
+ // Load a file that has markup like this:
+ // 3 0 obj <<
+ // /Key[<</InnerKey 42>>]
+ // >>
+ OUString aSourceURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "dict-array-dict.pdf";
+ SvFileStream aFile(aSourceURL, StreamMode::READ);
+ vcl::filter::PDFDocument aDocument;
+ CPPUNIT_ASSERT(aDocument.Read(aFile));
+ std::vector<vcl::filter::PDFObjectElement*> aPages = aDocument.GetPages();
+ CPPUNIT_ASSERT(!aPages.empty());
+ vcl::filter::PDFObjectElement* pPage = aPages[0];
+ auto pKey = dynamic_cast<vcl::filter::PDFArrayElement*>(pPage->Lookup("Key"));
+
+ // Without the accompanying fix in place, this test would have failed, because the value of Key
+ // was a dictionary element, not an array element.
+ CPPUNIT_ASSERT(pKey);
+}
+
+CPPUNIT_TEST_SUITE_REGISTRATION(VclFilterIpdfTest);
+
+CPPUNIT_PLUGIN_IMPLEMENT();
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/filter/ipdf/pdfdocument.cxx b/vcl/source/filter/ipdf/pdfdocument.cxx
index 84b65705b595..655450e28aa0 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -2228,8 +2228,17 @@ size_t PDFDictionaryElement::Parse(const std::vector<std::unique_ptr<PDFElement>
if (nexti >= i) // ensure we go forwards and not endlessly loop
{
i = nexti;
- rDictionary[aName] = pDictionary;
- aName.clear();
+ if (pArray)
+ {
+ // Dictionary value inside an array.
+ pArray->PushBack(pDictionary);
+ }
+ else
+ {
+ // Dictionary toplevel value.
+ rDictionary[aName] = pDictionary;
+ aName.clear();
+ }
}
}
}
commit e9057467c508e656e44c75e957673de80a1e9b4b
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Sep 4 17:17:48 2020 +0200
Commit: Vasily Melenchuk <vasily.melenchuk at cib.de>
CommitDate: Tue Apr 6 21:57:10 2021 +0300
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
diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx
index b9bceabb8acf..ffc70874c19b 100644
--- a/include/vcl/filter/PDFiumLibrary.hxx
+++ b/include/vcl/filter/PDFiumLibrary.hxx
@@ -17,11 +17,16 @@
#include <memory>
#include <rtl/instance.hxx>
#include <vcl/dllapi.h>
+#include <vcl/checksum.hxx>
+
+#include <fpdf_doc.h>
namespace vcl
{
namespace pdf
{
+class PDFiumDocument;
+
class VCL_DLLPUBLIC PDFium final
{
private:
@@ -33,6 +38,49 @@ public:
~PDFium();
};
+class VCL_DLLPUBLIC PDFiumPage final
+{
+private:
+ FPDF_PAGE mpPage;
+
+private:
+ PDFiumPage(const PDFiumPage&) = delete;
+ PDFiumPage& operator=(const PDFiumPage&) = delete;
+
+public:
+ PDFiumPage(FPDF_PAGE pPage)
+ : mpPage(pPage)
+ {
+ }
+
+ ~PDFiumPage()
+ {
+ if (mpPage)
+ FPDF_ClosePage(mpPage);
+ }
+
+ /// Get bitmap checksum of the page, without annotations/commenting.
+ BitmapChecksum getChecksum();
+};
+
+class VCL_DLLPUBLIC PDFiumDocument final
+{
+private:
+ FPDF_DOCUMENT mpPdfDocument;
+
+private:
+ PDFiumDocument(const PDFiumDocument&) = delete;
+ PDFiumDocument& operator=(const PDFiumDocument&) = delete;
+
+public:
+ PDFiumDocument(FPDF_DOCUMENT pPdfDocument);
+ ~PDFiumDocument();
+
+ int getPageCount();
+
+ std::unique_ptr<PDFiumPage> openPage(int nIndex);
+};
+
struct PDFiumLibrary : public rtl::StaticWithInit<std::shared_ptr<PDFium>, PDFiumLibrary>
{
std::shared_ptr<PDFium> operator()() { return std::make_shared<PDFium>(); }
diff --git a/vcl/source/pdf/PDFiumLibrary.cxx b/vcl/source/pdf/PDFiumLibrary.cxx
index 5082c2567cce..3ad6b5ee0780 100644
--- a/vcl/source/pdf/PDFiumLibrary.cxx
+++ b/vcl/source/pdf/PDFiumLibrary.cxx
@@ -15,6 +15,10 @@
#include <vcl/filter/PDFiumLibrary.hxx>
#include <fpdf_doc.h>
+#include <vcl/bitmap.hxx>
+
+#include <bitmapwriteaccess.hxx>
+
namespace vcl
{
namespace pdf
@@ -30,6 +34,58 @@ PDFium::PDFium()
}
PDFium::~PDFium() { FPDF_DestroyLibrary(); }
+
+PDFiumDocument::PDFiumDocument(FPDF_DOCUMENT pPdfDocument)
+ : mpPdfDocument(pPdfDocument)
+{
+}
+
+PDFiumDocument::~PDFiumDocument()
+{
+ if (mpPdfDocument)
+ FPDF_CloseDocument(mpPdfDocument);
+}
+
+std::unique_ptr<PDFiumPage> PDFiumDocument::openPage(int nIndex)
+{
+ std::unique_ptr<PDFiumPage> pPDFiumPage;
+ FPDF_PAGE pPage = FPDF_LoadPage(mpPdfDocument, nIndex);
+ if (pPage)
+ {
+ pPDFiumPage = o3tl::make_unique<PDFiumPage>(pPage);
+ }
+ return pPDFiumPage;
+}
+
+int PDFiumDocument::getPageCount() { return FPDF_GetPageCount(mpPdfDocument); }
+
+BitmapChecksum PDFiumPage::getChecksum()
+{
+ size_t nPageWidth = FPDF_GetPageWidth(mpPage);
+ size_t nPageHeight = FPDF_GetPageHeight(mpPage);
+ FPDF_BITMAP pPdfBitmap = FPDFBitmap_Create(nPageWidth, nPageHeight, /*alpha=*/1);
+ if (!pPdfBitmap)
+ {
+ return 0;
+ }
+
+ // Intentionally not using FPDF_ANNOT here, annotations/commenting is OK to not affect the
+ // checksum, signature verification wants this.
+ FPDF_RenderPageBitmap(pPdfBitmap, mpPage, /*start_x=*/0, /*start_y=*/0, nPageWidth, nPageHeight,
+ /*rotate=*/0, /*flags=*/0);
+ Bitmap aBitmap(Size(nPageWidth, nPageHeight), 24);
+ {
+ BitmapScopedWriteAccess pWriteAccess(aBitmap);
+ const auto pPdfBuffer = static_cast<ConstScanline>(FPDFBitmap_GetBuffer(pPdfBitmap));
+ const int nStride = FPDFBitmap_GetStride(pPdfBitmap);
+ for (size_t nRow = 0; nRow < nPageHeight; ++nRow)
+ {
+ ConstScanline pPdfLine = pPdfBuffer + (nStride * nRow);
+ pWriteAccess->CopyScanline(nRow, pPdfLine, ScanlineFormat::N32BitTcBgra, nStride);
+ }
+ }
+ return aBitmap.GetChecksum();
+}
}
} // end vcl::pdf
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
new file mode 100644
index 000000000000..f2b1a71096b2
Binary files /dev/null and b/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf differ
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;
}
More information about the Libreoffice-commits
mailing list