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 05bf387b685228c0558f27e0f93cdd7aa211c196
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Jul 24 11:29:27 2020 +0200
Commit: Thorsten Behrens <thorsten.behr...@cib.de>
CommitDate: Wed Jul 29 09:48:53 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 <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99563
Tested-by: Thorsten Behrens <thorsten.behr...@cib.de>
Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de>
diff --git a/include/vcl/filter/pdfdocument.hxx
b/include/vcl/filter/pdfdocument.hxx
index 03180fd0597f..50ffb29ee977 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -408,6 +408,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 c74bcbbade84..291f30387094 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 442323deb6ad..ea6a67f4e64f 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -75,6 +75,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.
@@ -96,6 +97,7 @@ public:
CPPUNIT_TEST(testPDF14LOWin);
CPPUNIT_TEST(testPDFPAdESGood);
CPPUNIT_TEST(testPartial);
+ CPPUNIT_TEST(testPartialInBetween);
CPPUNIT_TEST(testSigningCertificateAttribute);
CPPUNIT_TEST(testGood);
CPPUNIT_TEST(testTokenize);
@@ -142,9 +144,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())
@@ -286,7 +287,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.
@@ -526,6 +527,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 a54d121d97eb..bcca4f78245f 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -55,8 +55,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 b8639aec75c6..5763d8e6906d 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -157,8 +157,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;
@@ -167,6 +167,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
