summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2020-10-19 16:50:07 +0200
committerMiklos Vajna <vmiklos@collabora.com>2020-12-18 16:02:08 +0100
commit255ff5f6f9f11f72fca48b337c8dc6f2d08e8d6b (patch)
treef6440188600488640027123dfe77c9687bb822fa
parent3bd3fbe77ba0fb86e82f157b15f9626d586e96fa (diff)
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/inc/pdfio/pdfdocument.hxx 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/+/107966 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com> Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
-rw-r--r--include/vcl/filter/PDFiumLibrary.hxx2
-rw-r--r--include/vcl/filter/pdfdocument.hxx6
-rw-r--r--vcl/source/filter/ipdf/pdfdocument.cxx82
-rw-r--r--vcl/source/pdf/PDFiumLibrary.cxx12
-rw-r--r--xmlsecurity/inc/pdfio/pdfdocument.hxx2
-rw-r--r--xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdfbin0 -> 29646 bytes
-rw-r--r--xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx25
-rw-r--r--xmlsecurity/source/helper/pdfsignaturehelper.cxx5
-rw-r--r--xmlsecurity/source/pdfio/pdfdocument.cxx17
-rw-r--r--xmlsecurity/workben/pdfverify.cxx3
10 files changed, 130 insertions, 24 deletions
diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx
index 60ece6b7cbc9..34b28787eb37 100644
--- a/include/vcl/filter/PDFiumLibrary.hxx
+++ b/include/vcl/filter/PDFiumLibrary.hxx
@@ -60,7 +60,7 @@ public:
FPDF_PAGE getPointer() { return mpPage; }
/// Get bitmap checksum of the page, without annotations/commenting.
- BitmapChecksum getChecksum();
+ BitmapChecksum getChecksum(int nMDPPerm);
};
struct PDFiumLibrary : public rtl::StaticWithInit<std::shared_ptr<PDFium>, PDFiumLibrary>
diff --git a/include/vcl/filter/pdfdocument.hxx b/include/vcl/filter/pdfdocument.hxx
index 65f72f301c8f..a814a7da3c15 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -359,6 +359,7 @@ public:
size_t GetObjectOffset(size_t nIndex) const;
const std::vector< std::unique_ptr<PDFElement> >& GetElements();
std::vector<PDFObjectElement*> GetPages();
+ PDFObjectElement* GetCatalog();
/// Remember the end location of an EOF token.
void PushBackEOF(size_t nOffset);
/// Look up object based on object number, possibly by parsing object streams.
@@ -381,6 +382,11 @@ public:
bool Write(SvStream& rStream);
/// Get a list of signatures embedded into this document.
std::vector<PDFObjectElement*> GetSignatureWidgets();
+ /**
+ * Get the value of the "modification detection and prevention" permission:
+ * Valid values are 1, 2 and 3: only 3 allows annotations after signing.
+ */
+ int GetMDPPerm();
/// 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.
diff --git a/vcl/source/filter/ipdf/pdfdocument.cxx b/vcl/source/filter/ipdf/pdfdocument.cxx
index 6851bef19b91..b17ffb25999f 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -1793,10 +1793,8 @@ static void visitPages(PDFObjectElement* pPages, std::vector<PDFObjectElement*>&
pPages->setVisiting(false);
}
-std::vector<PDFObjectElement*> PDFDocument::GetPages()
+PDFObjectElement* PDFDocument::GetCatalog()
{
- std::vector<PDFObjectElement*> aRet;
-
PDFReferenceElement* pRoot = nullptr;
@@ -1817,11 +1815,18 @@ std::vector<PDFObjectElement*> PDFDocument::GetPages()
if (!pRoot)
{
- SAL_WARN("vcl.filter", "PDFDocument::GetPages: trailer has no Root key");
- return aRet;
+ SAL_WARN("vcl.filter", "PDFDocument::GetCatalog: trailer has no Root key");
+ return nullptr;
}
- PDFObjectElement* pCatalog = pRoot->LookupObject();
+ return pRoot->LookupObject();
+}
+
+std::vector<PDFObjectElement*> PDFDocument::GetPages()
+{
+ std::vector<PDFObjectElement*> aRet;
+
+ PDFObjectElement* pCatalog = GetCatalog();
if (!pCatalog)
{
SAL_WARN("vcl.filter", "PDFDocument::GetPages: trailer has no catalog");
@@ -1896,6 +1901,71 @@ std::vector<PDFObjectElement*> PDFDocument::GetSignatureWidgets()
return aRet;
}
+int PDFDocument::GetMDPPerm()
+{
+ int nRet = 3;
+
+ std::vector<PDFObjectElement*> aSignatures = GetSignatureWidgets();
+ if (aSignatures.empty())
+ {
+ return nRet;
+ }
+
+ for (const auto& pSignature : aSignatures)
+ {
+ vcl::filter::PDFObjectElement* pSig = pSignature->LookupObject("V");
+ if (!pSig)
+ {
+ SAL_WARN("vcl.filter", "PDFDocument::GetMDPPerm: can't find signature object");
+ continue;
+ }
+
+ auto pReference = dynamic_cast<PDFArrayElement*>(pSig->Lookup("Reference"));
+ if (!pReference || pReference->GetElements().empty())
+ {
+ continue;
+ }
+
+ auto pFirstReference = dynamic_cast<PDFDictionaryElement*>(pReference->GetElements()[0]);
+ if (!pFirstReference)
+ {
+ SAL_WARN("vcl.filter",
+ "PDFDocument::GetMDPPerm: reference array doesn't contain a dictionary");
+ continue;
+ }
+
+ PDFElement* pTransformParams = pFirstReference->LookupElement("TransformParams");
+ auto pTransformParamsDict = dynamic_cast<PDFDictionaryElement*>(pTransformParams);
+ if (!pTransformParamsDict)
+ {
+ auto pTransformParamsRef = dynamic_cast<PDFReferenceElement*>(pTransformParams);
+ if (pTransformParamsRef)
+ {
+ PDFObjectElement* pTransformParamsObj = pTransformParamsRef->LookupObject();
+ if (pTransformParamsObj)
+ {
+ pTransformParamsDict = pTransformParamsObj->GetDictionary();
+ }
+ }
+ }
+
+ if (!pTransformParamsDict)
+ {
+ continue;
+ }
+
+ auto pP = dynamic_cast<PDFNumberElement*>(pTransformParamsDict->LookupElement("P"));
+ if (!pP)
+ {
+ return 2;
+ }
+
+ return pP->GetValue();
+ }
+
+ return nRet;
+}
+
std::vector<unsigned char> PDFDocument::DecodeHexString(PDFHexStringElement const* pElement)
{
return svl::crypto::DecodeHexString(pElement->GetValue());
diff --git a/vcl/source/pdf/PDFiumLibrary.cxx b/vcl/source/pdf/PDFiumLibrary.cxx
index a8774b15bb4e..96dab5f62ff6 100644
--- a/vcl/source/pdf/PDFiumLibrary.cxx
+++ b/vcl/source/pdf/PDFiumLibrary.cxx
@@ -34,7 +34,7 @@ PDFium::PDFium()
PDFium::~PDFium() { FPDF_DestroyLibrary(); }
-BitmapChecksum PDFiumPage::getChecksum()
+BitmapChecksum PDFiumPage::getChecksum(int nMDPPerm)
{
size_t nPageWidth = FPDF_GetPageWidth(mpPage);
size_t nPageHeight = FPDF_GetPageHeight(mpPage);
@@ -44,10 +44,14 @@ BitmapChecksum PDFiumPage::getChecksum()
return 0;
}
- // Intentionally not using FPDF_ANNOT here, annotations/commenting is OK to not affect the
- // checksum, signature verification wants this.
+ int nFlags = 0;
+ if (nMDPPerm != 3)
+ {
+ // Annotations/commenting should affect the checksum, signature verification wants this.
+ nFlags = FPDF_ANNOT;
+ }
FPDF_RenderPageBitmap(pPdfBitmap, mpPage, /*start_x=*/0, /*start_y=*/0, nPageWidth, nPageHeight,
- /*rotate=*/0, /*flags=*/0);
+ /*rotate=*/0, nFlags);
Bitmap aBitmap(Size(nPageWidth, nPageHeight), 24);
{
Bitmap::ScopedWriteAccess pWriteAccess(aBitmap);
diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx b/xmlsecurity/inc/pdfio/pdfdocument.hxx
index aaacae9db29f..5c8bf8b3557b 100644
--- a/xmlsecurity/inc/pdfio/pdfdocument.hxx
+++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx
@@ -29,7 +29,7 @@ namespace pdfio
* @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, vcl::filter::PDFDocument& rDocument);
+XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, 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
new file mode 100644
index 000000000000..04d9950582b0
--- /dev/null
+++ b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p1.pdf
Binary files differ
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 54fe2e7912f5..899a7567c4a3 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -72,6 +72,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.
@@ -94,6 +95,7 @@ public:
CPPUNIT_TEST(testPDFPAdESGood);
CPPUNIT_TEST(testPartial);
CPPUNIT_TEST(testPartialInBetween);
+ CPPUNIT_TEST(testBadCertP1);
CPPUNIT_TEST(testSigningCertificateAttribute);
CPPUNIT_TEST(testGood);
CPPUNIT_TEST(testTokenize);
@@ -139,7 +141,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())
@@ -268,7 +272,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.
@@ -398,6 +404,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 5e7684ca24c3..1f809fda1148 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -48,11 +48,14 @@ bool PDFSignatureHelper::ReadAndVerifySignature(const uno::Reference<io::XInputS
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 21ea3c324d1a..7b697d6d86eb 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -168,7 +168,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();
@@ -184,7 +185,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);
@@ -195,9 +196,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))
@@ -212,7 +213,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();
@@ -221,7 +222,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.
@@ -234,7 +235,7 @@ namespace xmlsecurity
namespace pdfio
{
-bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument)
+bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument, int nMDPPerm)
{
vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V");
if (!pValue)
@@ -337,7 +338,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 92429864080b..bb66edb29981 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -147,11 +147,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;