include/vcl/filter/pdfdocument.hxx | 2
vcl/source/filter/ipdf/pdfdocument.cxx | 13 +
xmlsecurity/inc/pdfio/pdfdocument.hxx | 4
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 | 139 ++++++++++---
xmlsecurity/workben/pdfverify.cxx | 6
8 files changed, 153 insertions(+), 33 deletions(-)
New commits:
commit 130ce3d0c6461c47b421807e4f5ab1fbf408f170
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Jul 24 11:29:27 2020 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Jul 29 09:38:54 2020 +0200
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 <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/include/vcl/filter/pdfdocument.hxx
b/include/vcl/filter/pdfdocument.hxx
index f06049e55c30..65f72f301c8f 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -383,6 +383,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 48cb17b67fae..ecbf948f1292 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -151,6 +151,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;
@@ -1917,7 +1919,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 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<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));
+ 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<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, /*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<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 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<io::XInputS
{
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 08437b2f8cae..8d6aeafdd9f3 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -52,12 +52,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)
+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<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.
@@ -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;
}
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
