diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2020-10-19 16:50:07 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2020-11-27 15:31:09 +0100 |
commit | 9fda2bf1da07f89c1e8613d2b92ea423f68d0c6e (patch) | |
tree | fe7a74e1bce1d182db8c75324e110b1c4a40c45f /xmlsecurity | |
parent | xmlsecurity: pdf incremental updates that are non-commenting are invalid (diff) | |
download | core-9fda2bf1da07f89c1e8613d2b92ea423f68d0c6e.tar.gz core-9fda2bf1da07f89c1e8613d2b92ea423f68d0c6e.zip |
xmlsecurity: handle MDP permission during PDF verify
(cherry picked from commit 586f6abee92af3cdabdce034b607b9a046ed3946)
Conflicts:
include/vcl/filter/PDFiumLibrary.hxx
vcl/source/filter/ipdf/pdfdocument.cxx
vcl/source/pdf/PDFiumLibrary.cxx
xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
xmlsecurity/source/helper/pdfsignaturehelper.cxx
(cherry picked from commit 00479937dc071246cc27f33fd6397668448a7ed9)
Change-Id: I626fca7c03079fb0374c577dcfe024e7db6ed5b3
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106766
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Diffstat (limited to 'xmlsecurity')
-rw-r--r-- | xmlsecurity/inc/pdfio/pdfdocument.hxx | 2 | ||||
-rw-r--r-- | xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf | bin | 0 -> 29646 bytes | |||
-rw-r--r-- | xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 27 | ||||
-rw-r--r-- | xmlsecurity/source/helper/pdfsignaturehelper.cxx | 5 | ||||
-rw-r--r-- | xmlsecurity/source/pdfio/pdfdocument.cxx | 18 | ||||
-rw-r--r-- | xmlsecurity/workben/pdfverify.cxx | 3 |
6 files changed, 40 insertions, 15 deletions
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 Binary files differnew file mode 100644 index 000000000000..04d9950582b0 --- /dev/null +++ b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx index 8511f20eeae4..60dcd0fc7544 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -77,6 +77,7 @@ public: /// Test a valid signature that does not cover the whole file. void testPartial(); void testPartialInBetween(); + void testBadCertP1(); /// Test writing a PAdES signature. void testSigningCertificateAttribute(); /// Test that we accept files which are supposed to be good. @@ -99,6 +100,7 @@ public: CPPUNIT_TEST(testPDFPAdESGood); CPPUNIT_TEST(testPartial); CPPUNIT_TEST(testPartialInBetween); + CPPUNIT_TEST(testBadCertP1); CPPUNIT_TEST(testSigningCertificateAttribute); CPPUNIT_TEST(testGood); CPPUNIT_TEST(testTokenize); @@ -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,21 @@ 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); +} + 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 a506f3e0c3ed..3cedf7b62273 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); } FPDF_CloseDocument(pPdfDocument); @@ -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 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; |