[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.2' - include/vcl vcl/source xmlsecurity/inc xmlsecurity/qa xmlsecurity/source xmlsecurity/workben
Miklos Vajna (via logerrit)
logerrit at kemper.freedesktop.org
Mon Jul 27 21:28:55 UTC 2020
include/vcl/filter/pdfdocument.hxx | 2
vcl/source/filter/ipdf/pdfdocument.cxx | 13 +
xmlsecurity/inc/pdfio/pdfdocument.hxx | 6
xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf |binary
xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 19 +
xmlsecurity/source/helper/pdfsignaturehelper.cxx | 3
xmlsecurity/source/pdfio/pdfdocument.cxx | 140 ++++++++++---
xmlsecurity/workben/pdfverify.cxx | 6
8 files changed, 155 insertions(+), 34 deletions(-)
New commits:
commit e1c1f1332942fc4f15122cf5e894a64da8e1be0e
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Jul 24 11:29:27 2020 +0200
Commit: Miklos Vajna <vmiklos at collabora.com>
CommitDate: Mon Jul 27 23:28:22 2020 +0200
xmlsecurity: detect unsigned incremental update between signatures
(cherry picked from commit 7468d5df5ec79783eae84b62bdc5ecf12f0ca255)
Conflicts:
vcl/source/filter/ipdf/pdfdocument.cxx
xmlsecurity/source/pdfio/pdfdocument.cxx
Change-Id: I269ed858852ee7d1275adf340c8cc1565fc30693
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99480
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
diff --git a/include/vcl/filter/pdfdocument.hxx b/include/vcl/filter/pdfdocument.hxx
index 3ec08020044a..56debd53e8c7 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -402,6 +402,8 @@ public:
std::vector<PDFObjectElement*> GetSignatureWidgets();
/// 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.
+ const std::vector<size_t>& GetEOFs() const;
//@}
};
diff --git a/vcl/source/filter/ipdf/pdfdocument.cxx b/vcl/source/filter/ipdf/pdfdocument.cxx
index 092df7cfbee7..84b65705b595 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -147,6 +147,8 @@ bool PDFDocument::RemoveSignature(size_t nPosition)
return m_aEditBuffer.good();
}
+const std::vector<size_t>& PDFDocument::GetEOFs() const { return m_aEOFs; }
+
sal_uInt32 PDFDocument::GetNextSignature()
{
sal_uInt32 nRet = 0;
@@ -1978,7 +1980,16 @@ bool PDFCommentElement::Read(SvStream& rStream)
m_aComment = aBuf.makeStringAndClear();
if (m_aComment.startsWith("%%EOF"))
- m_rDoc.PushBackEOF(rStream.Tell());
+ {
+ sal_uInt64 nPos = rStream.Tell();
+ if (ch == '\r')
+ {
+ // If the comment ends with a \r\n, count the \n as well to match Adobe Acrobat
+ // behavior.
+ nPos += 1;
+ }
+ m_rDoc.PushBackEOF(nPos);
+ }
SAL_INFO("vcl.filter", "PDFCommentElement::Read: m_aComment is '" << m_aComment << "'");
return true;
diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx b/xmlsecurity/inc/pdfio/pdfdocument.hxx
index 996bb1527bb8..f7e36492e746 100644
--- a/xmlsecurity/inc/pdfio/pdfdocument.hxx
+++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx
@@ -18,6 +18,7 @@ namespace vcl
namespace filter
{
class PDFObjectElement;
+class PDFDocument;
}
}
struct SignatureInformation;
@@ -29,12 +30,13 @@ 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);
+ 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 d608129d6254..8511f20eeae4 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 testPartialInBetween();
/// Test writing a PAdES signature.
void testSigningCertificateAttribute();
/// Test that we accept files which are supposed to be good.
@@ -97,6 +98,7 @@ public:
CPPUNIT_TEST(testPDF14LOWin);
CPPUNIT_TEST(testPDFPAdESGood);
CPPUNIT_TEST(testPartial);
+ CPPUNIT_TEST(testPartialInBetween);
CPPUNIT_TEST(testSigningCertificateAttribute);
CPPUNIT_TEST(testGood);
CPPUNIT_TEST(testTokenize);
@@ -143,9 +145,8 @@ std::vector<SignatureInformation> 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));
+ xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, aVerifyDocument));
aRet.push_back(aInfo);
if (!rExpectedSubFilter.isEmpty())
@@ -287,7 +288,7 @@ void PDFSigningTest::testPDFRemove()
CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aSignatures.size());
SignatureInformation aInfo(0);
CPPUNIT_ASSERT(
- xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, /*bLast=*/true));
+ xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, aDocument));
}
// Remove the signature and write out the result as remove.pdf.
@@ -527,6 +528,18 @@ void PDFSigningTest::testUnknownSubFilter()
CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(2), rInformations.size());
}
+void PDFSigningTest::testPartialInBetween()
+{
+ std::vector<SignatureInformation> 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 1f11b7856f4a..0398acac7ea0 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -57,8 +57,7 @@ bool PDFSignatureHelper::ReadAndVerifySignature(
{
SignatureInformation aInfo(i);
- bool bLast = i == aSignatures.size() - 1;
- if (!xmlsecurity::pdfio::ValidateSignature(*pStream, aSignatures[i], aInfo, bLast))
+ if (!xmlsecurity::pdfio::ValidateSignature(*pStream, aSignatures[i], aInfo, aDocument))
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 febcd2e3a1ea..7cf2c137c1c4 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -23,12 +23,124 @@
using namespace com::sun::star;
+namespace
+{
+/// Turns an array of floats into offset + length pairs.
+bool GetByteRangesFromPDF(vcl::filter::PDFArrayElement& rArray,
+ std::vector<std::pair<size_t, size_t>>& rByteRanges)
+{
+ size_t nByteRangeOffset = 0;
+ const std::vector<vcl::filter::PDFElement*>& rByteRangeElements = rArray.GetElements();
+ for (size_t i = 0; i < rByteRangeElements.size(); ++i)
+ {
+ auto pNumber = dynamic_cast<vcl::filter::PDFNumberElement*>(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<vcl::filter::PDFArrayElement*>(pValue->Lookup("ByteRange"));
+ if (!pByteRange || pByteRange->GetElements().size() < 2)
+ {
+ return false;
+ }
+
+ std::vector<std::pair<size_t, size_t>> 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<size_t> 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<size_t>& 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)
+ SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument)
{
vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V");
if (!pValue)
@@ -110,25 +222,9 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
// Build a list of offset-length pairs, representing the signed bytes.
std::vector<std::pair<size_t, size_t>> aByteRanges;
- size_t nByteRangeOffset = 0;
- const std::vector<vcl::filter::PDFElement*>& rByteRangeElements = pByteRange->GetElements();
- for (size_t i = 0; i < rByteRangeElements.size(); ++i)
+ if (!GetByteRangesFromPDF(*pByteRange, aByteRanges))
{
- auto pNumber = dynamic_cast<vcl::filter::PDFNumberElement*>(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.
@@ -150,11 +246,7 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
"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 efe0461b9f14..2754ea2f58c1 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -158,8 +158,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;
@@ -168,6 +168,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;
}
}
More information about the Libreoffice-commits
mailing list