From 998d42ac58e5532e2a07cc10204d9804ff3411cb Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Fri, 24 Jul 2020 11:29:27 +0200 Subject: xmlsecurity: detect unsigned incremental update between signatures (cherry picked from commit 7468d5df5ec79783eae84b62bdc5ecf12f0ca255) Conflicts: vcl/source/filter/ipdf/pdfdocument.cxx xmlsecurity/inc/pdfio/pdfdocument.hxx xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx xmlsecurity/source/pdfio/pdfdocument.cxx Change-Id: I269ed858852ee7d1275adf340c8cc1565fc30693 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99510 Tested-by: Jenkins CollaboraOffice Reviewed-by: Miklos Vajna --- xmlsecurity/inc/pdfio/pdfdocument.hxx | 4 +- .../qa/unit/pdfsigning/data/partial-in-between.pdf | Bin 0 -> 104501 bytes xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 19 ++- xmlsecurity/source/helper/pdfsignaturehelper.cxx | 3 +- xmlsecurity/source/pdfio/pdfdocument.cxx | 139 +++++++++++++++++---- xmlsecurity/workben/pdfverify.cxx | 6 +- 6 files changed, 139 insertions(+), 32 deletions(-) create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf (limited to 'xmlsecurity') diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx b/xmlsecurity/inc/pdfio/pdfdocument.hxx index 622bc1ffb678..aaacae9db29f 100644 --- a/xmlsecurity/inc/pdfio/pdfdocument.hxx +++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx @@ -26,10 +26,10 @@ namespace pdfio /** * @param rInformation The actual result. - * @param bLast If this is the last signature in the file, so it covers the whole file physically. + * @param rDocument the parsed document to see if the signature is partial. * @return If we can determinate a result. */ -XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, bool bLast); +XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument); } // namespace pdfio } // namespace xmlsecurity diff --git a/xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf b/xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf new file mode 100644 index 000000000000..211a111cb394 Binary files /dev/null and b/xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf differ diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx index 2a65ae004e43..54fe2e7912f5 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -71,6 +71,7 @@ public: void testPDFPAdESGood(); /// Test a valid signature that does not cover the whole file. void testPartial(); + void testPartialInBetween(); /// Test writing a PAdES signature. void testSigningCertificateAttribute(); /// Test that we accept files which are supposed to be good. @@ -92,6 +93,7 @@ public: CPPUNIT_TEST(testPDF14LOWin); CPPUNIT_TEST(testPDFPAdESGood); CPPUNIT_TEST(testPartial); + CPPUNIT_TEST(testPartialInBetween); CPPUNIT_TEST(testSigningCertificateAttribute); CPPUNIT_TEST(testGood); CPPUNIT_TEST(testTokenize); @@ -137,8 +139,7 @@ std::vector PDFSigningTest::verify(const OUString& rURL, s for (size_t i = 0; i < aSignatures.size(); ++i) { SignatureInformation aInfo(i); - bool bLast = i == aSignatures.size() - 1; - CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, bLast)); + CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, aVerifyDocument)); aRet.push_back(aInfo); if (!rExpectedSubFilter.isEmpty()) @@ -267,7 +268,7 @@ void 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, /*bLast=*/true)); + CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, aDocument)); } // Remove the signature and write out the result as remove.pdf. @@ -483,6 +484,18 @@ void PDFSigningTest::testUnknownSubFilter() CPPUNIT_ASSERT_EQUAL(static_cast(2), rInformations.size()); } +void PDFSigningTest::testPartialInBetween() +{ + std::vector aInfos + = verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "partial-in-between.pdf", 2, + /*rExpectedSubFilter=*/OString()); + CPPUNIT_ASSERT(!aInfos.empty()); + SignatureInformation& rInformation = aInfos[0]; + // Without the accompanying fix in place, this test would have failed, as unsigned incremental + // update between two signatures were not detected. + CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature); +} + CPPUNIT_TEST_SUITE_REGISTRATION(PDFSigningTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx index 5356705f7b17..5e7684ca24c3 100644 --- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx +++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx @@ -52,8 +52,7 @@ bool PDFSignatureHelper::ReadAndVerifySignature(const uno::Reference>& rByteRanges) +{ + size_t nByteRangeOffset = 0; + const std::vector& rByteRangeElements = rArray.GetElements(); + for (size_t i = 0; i < rByteRangeElements.size(); ++i) + { + auto pNumber = dynamic_cast(rByteRangeElements[i]); + if (!pNumber) + { + SAL_WARN("xmlsecurity.pdfio", + "ValidateSignature: signature offset and length has to be a number"); + return false; + } + + if (i % 2 == 0) + { + nByteRangeOffset = pNumber->GetValue(); + continue; + } + size_t nByteRangeLength = pNumber->GetValue(); + rByteRanges.emplace_back(nByteRangeOffset, nByteRangeLength); + } + + return true; +} + +/// Determines the last position that is covered by a signature. +bool GetEOFOfSignature(vcl::filter::PDFObjectElement* pSignature, size_t& rEOF) +{ + vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V"); + if (!pValue) + { + return false; + } + + auto pByteRange = dynamic_cast(pValue->Lookup("ByteRange")); + if (!pByteRange || pByteRange->GetElements().size() < 2) + { + return false; + } + + std::vector> aByteRanges; + if (!GetByteRangesFromPDF(*pByteRange, aByteRanges)) + { + return false; + } + + rEOF = aByteRanges[1].first + aByteRanges[1].second; + return true; +} + +/// Checks if there are unsigned incremental updates between the signatures or after the last one. +bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument, + vcl::filter::PDFObjectElement* pSignature) +{ + std::set aSignedEOFs; + for (const auto& i : rDocument.GetSignatureWidgets()) + { + size_t nEOF = 0; + if (!GetEOFOfSignature(i, nEOF)) + { + return false; + } + + aSignedEOFs.insert(nEOF); + } + + size_t nSignatureEOF = 0; + if (!GetEOFOfSignature(pSignature, nSignatureEOF)) + { + return false; + } + + const std::vector& rAllEOFs = rDocument.GetEOFs(); + bool bFoundOwn = false; + for (const auto& rEOF : rAllEOFs) + { + if (rEOF == nSignatureEOF) + { + bFoundOwn = true; + continue; + } + + if (!bFoundOwn) + { + continue; + } + + if (aSignedEOFs.find(rEOF) == aSignedEOFs.end()) + { + // Unsigned incremental update found. + return false; + } + } + + // Make sure we find the incremental update of the signature itself. + if (!bFoundOwn) + { + return false; + } + + // No additional content after the last incremental update. + rStream.Seek(STREAM_SEEK_TO_END); + size_t nFileEnd = rStream.Tell(); + return std::find(rAllEOFs.begin(), rAllEOFs.end(), nFileEnd) != rAllEOFs.end(); +} +} + namespace xmlsecurity { namespace pdfio { -bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, bool bLast) +bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument) { vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V"); if (!pValue) @@ -136,24 +248,9 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat // Build a list of offset-length pairs, representing the signed bytes. std::vector> aByteRanges; - size_t nByteRangeOffset = 0; - const std::vector& rByteRangeElements = pByteRange->GetElements(); - for (size_t i = 0; i < rByteRangeElements.size(); ++i) + if (!GetByteRangesFromPDF(*pByteRange, aByteRanges)) { - auto pNumber = dynamic_cast(rByteRangeElements[i]); - if (!pNumber) - { - SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: signature offset and length has to be a number"); - return false; - } - - if (i % 2 == 0) - { - nByteRangeOffset = pNumber->GetValue(); - continue; - } - size_t nByteRangeLength = pNumber->GetValue(); - aByteRanges.emplace_back(nByteRangeOffset, nByteRangeLength); + return false; } // Detect if the byte ranges don't cover everything, but the signature itself. @@ -174,11 +271,7 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: second range start is not the end of the signature"); return false; } - rStream.Seek(STREAM_SEEK_TO_END); - size_t nFileEnd = rStream.Tell(); - if (bLast && (aByteRanges[1].first + aByteRanges[1].second) != nFileEnd) - // Second range end is not the end of the file. - rInformation.bPartialDocumentSignature = true; + rInformation.bPartialDocumentSignature = !IsCompleteSignature(rStream, rDocument, pSignature); // 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 55a20d51a6b9..7ddb2623944c 100644 --- a/xmlsecurity/workben/pdfverify.cxx +++ b/xmlsecurity/workben/pdfverify.cxx @@ -149,8 +149,8 @@ int pdfVerify(int nArgc, char** pArgv) for (size_t i = 0; i < aSignatures.size(); ++i) { SignatureInformation aInfo(i); - bool bLast = i == aSignatures.size() - 1; - if (!xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, bLast)) + if (!xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, + aDocument)) { SAL_WARN("xmlsecurity.pdfio", "failed to determine digest match"); return 1; @@ -158,6 +158,8 @@ int pdfVerify(int nArgc, char** pArgv) bool bSuccess = aInfo.nStatus == xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED; std::cerr << "signature #" << i << ": digest match? " << bSuccess << std::endl; + std::cerr << "signature #" << i << ": partial? " << aInfo.bPartialDocumentSignature + << std::endl; } } -- cgit