xmlsecurity/qa/unit/signing/signing.cxx | 209 ++++++++++++++-----------
xmlsecurity/source/helper/ooxmlsecexporter.cxx | 53 +++---
xmlsecurity/source/helper/xsecsign.cxx | 10 -
3 files changed, 153 insertions(+), 119 deletions(-)
New commits:
commit f2e1e4ff085962a08a5d7738325b383c07afcbbd
Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Mon Nov 1 21:46:43 2021 +0100
Commit: Tomaž Vajngerl <qui...@gmail.com>
CommitDate: Tue Nov 2 14:05:50 2021 +0100
xmlsec: fix OOXML signing with multiple certs, extend the test
Signing OOXML with 3 or more times didn't work as other ids
("idPackageObject", "idOfficeObject", ...) were not uniqe. This
change makes those ids unique by appending the signature id. The
signature ID is now generated for OOXML too, while previously it
was a hardcoded string ("idPackageSignature").
The test for signing multiple OOXML was written before, but didn't
catch the issues because it didn't assert the status of the
document after loading it again. This is which is now fixed (and
also added changed for the ODF test case).
Change-Id: Ifa20ea17498b117a4c57f6eddf82f8e83bc640bc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124571
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx
b/xmlsecurity/qa/unit/signing/signing.cxx
index ed50ade74bf8..de54150d2108 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -60,6 +60,7 @@
#include <sfx2/viewsh.hxx>
#include <comphelper/propertyvalue.hxx>
#include <vcl/filter/PDFiumLibrary.hxx>
+#include <vcl/scheduler.hxx>
using namespace com::sun::star;
@@ -1062,55 +1063,72 @@ CPPUNIT_TEST_FIXTURE(SigningTest,
testSigningMultipleTimes_ODT)
aMediaDescriptor["FilterName"] <<= OUString("writer8");
xStorable->storeAsURL(aTempFile.GetURL(),
aMediaDescriptor.getAsConstPropertyValueList());
- DocumentSignatureManager aManager(mxComponentContext,
DocumentSignatureMode::Content);
- CPPUNIT_ASSERT(aManager.init());
- uno::Reference<embed::XStorage> xStorage
- = comphelper::OStorageHelper::GetStorageOfFormatFromURL(
- ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(),
embed::ElementModes::READWRITE);
- CPPUNIT_ASSERT(xStorage.is());
- aManager.setStore(xStorage);
- aManager.getSignatureHelper().SetStorage(xStorage, "1.2");
-
- // Create a signature.
- uno::Reference<security::XCertificate> xCertificate
- = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::RSA);
- if (!xCertificate.is())
- return;
- sal_Int32 nSecurityId;
- aManager.add(xCertificate, mxSecurityContext, /*rDescription=*/OUString(),
nSecurityId,
- /*bAdESCompliant=*/true);
-
- // Read back the signature and make sure that it's valid.
- aManager.read(/*bUseTempStream=*/true);
{
- std::vector<SignatureInformation>& rInformations
- = aManager.getCurrentSignatureInformations();
- CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(1),
rInformations.size());
-
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
- rInformations[0].nStatus);
+ DocumentSignatureManager aManager(mxComponentContext,
DocumentSignatureMode::Content);
+ CPPUNIT_ASSERT(aManager.init());
+ uno::Reference<embed::XStorage> xStorage
+ = comphelper::OStorageHelper::GetStorageOfFormatFromURL(
+ ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(),
embed::ElementModes::READWRITE);
+ CPPUNIT_ASSERT(xStorage.is());
+ aManager.setStore(xStorage);
+ aManager.getSignatureHelper().SetStorage(xStorage, "1.2");
+
+ // Create a signature.
+ uno::Reference<security::XCertificate> xCertificate
+ = getCertificate(aManager,
svl::crypto::SignatureMethodAlgorithm::RSA);
+
+ if (!xCertificate.is())
+ return;
+ sal_Int32 nSecurityId;
+ aManager.add(xCertificate, mxSecurityContext,
/*rDescription=*/OUString(), nSecurityId,
+ /*bAdESCompliant=*/true);
+
+ // Read back the signature and make sure that it's valid.
+ aManager.read(/*bUseTempStream=*/true);
+ {
+ std::vector<SignatureInformation>& rInformations
+ = aManager.getCurrentSignatureInformations();
+ CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(1),
rInformations.size());
+
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+ rInformations[0].nStatus);
+ }
+
+ aManager.add(xCertificate, mxSecurityContext,
/*rDescription=*/OUString(), nSecurityId,
+ /*bAdESCompliant=*/true);
+ aManager.read(/*bUseTempStream=*/true);
+ {
+ std::vector<SignatureInformation>& rInformations
+ = aManager.getCurrentSignatureInformations();
+ CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(2),
rInformations.size());
+
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+ rInformations[1].nStatus);
+ }
+
+ aManager.add(xCertificate, mxSecurityContext,
/*rDescription=*/OUString(), nSecurityId,
+ /*bAdESCompliant=*/true);
+ aManager.read(/*bUseTempStream=*/true);
+ {
+ std::vector<SignatureInformation>& rInformations
+ = aManager.getCurrentSignatureInformations();
+ CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(3),
rInformations.size());
+
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+ rInformations[2].nStatus);
+ }
+
+ aManager.write(/*bXAdESCompliantIfODF=*/true);
+ uno::Reference<embed::XTransactedObject> xTransactedObject(xStorage,
uno::UNO_QUERY);
+ xTransactedObject->commit();
}
- aManager.add(xCertificate, mxSecurityContext, /*rDescription=*/OUString(),
nSecurityId,
- /*bAdESCompliant=*/true);
- aManager.read(/*bUseTempStream=*/true);
- {
- std::vector<SignatureInformation>& rInformations
- = aManager.getCurrentSignatureInformations();
- CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(2),
rInformations.size());
-
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
- rInformations[1].nStatus);
- }
+ Scheduler::ProcessEventsToIdle();
- aManager.add(xCertificate, mxSecurityContext, /*rDescription=*/OUString(),
nSecurityId,
- /*bAdESCompliant=*/true);
- aManager.read(/*bUseTempStream=*/true);
- {
- std::vector<SignatureInformation>& rInformations
- = aManager.getCurrentSignatureInformations();
- CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(3),
rInformations.size());
-
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
- rInformations[2].nStatus);
- }
+ createDoc(aTempFile.GetURL());
+
+ SfxBaseModel* pBaseModel = dynamic_cast<SfxBaseModel*>(mxComponent.get());
+ CPPUNIT_ASSERT(pBaseModel);
+ SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell();
+ CPPUNIT_ASSERT(pObjectShell);
+ CPPUNIT_ASSERT_EQUAL(SignatureState::OK,
pObjectShell->GetDocumentSignatureState());
}
CPPUNIT_TEST_FIXTURE(SigningTest, testSigningMultipleTimes_OOXML)
@@ -1124,54 +1142,67 @@ CPPUNIT_TEST_FIXTURE(SigningTest,
testSigningMultipleTimes_OOXML)
aMediaDescriptor["FilterName"] <<= OUString("MS Word 2007 XML");
xStorable->storeAsURL(aTempFile.GetURL(),
aMediaDescriptor.getAsConstPropertyValueList());
- DocumentSignatureManager aManager(mxComponentContext,
DocumentSignatureMode::Content);
- CPPUNIT_ASSERT(aManager.init());
- uno::Reference<embed::XStorage> xStorage
- = comphelper::OStorageHelper::GetStorageOfFormatFromURL(
- ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(),
embed::ElementModes::READWRITE);
- CPPUNIT_ASSERT(xStorage.is());
- aManager.setStore(xStorage);
- aManager.getSignatureHelper().SetStorage(xStorage, "1.2");
-
- // Create a signature.
- uno::Reference<security::XCertificate> xCertificate
- = getCertificate(aManager,
svl::crypto::SignatureMethodAlgorithm::ECDSA);
- if (!xCertificate.is())
- return;
-
- sal_Int32 nSecurityId;
- aManager.add(xCertificate, mxSecurityContext, "", nSecurityId,
/*bAdESCompliant=*/false);
- aManager.read(/*bUseTempStream=*/true);
{
- std::vector<SignatureInformation>& rInformations
- = aManager.getCurrentSignatureInformations();
- CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(1),
rInformations.size());
-
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
- rInformations[0].nStatus);
+ DocumentSignatureManager aManager(mxComponentContext,
DocumentSignatureMode::Content);
+ CPPUNIT_ASSERT(aManager.init());
+ uno::Reference<embed::XStorage> xStorage
+ = comphelper::OStorageHelper::GetStorageOfFormatFromURL(
+ ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(),
embed::ElementModes::READWRITE);
+ CPPUNIT_ASSERT(xStorage.is());
+ aManager.setStore(xStorage);
+ aManager.getSignatureHelper().SetStorage(xStorage, "1.2");
+
+ // Create a signature.
+ uno::Reference<security::XCertificate> xCertificate
+ = getCertificate(aManager,
svl::crypto::SignatureMethodAlgorithm::ECDSA);
+ if (!xCertificate.is())
+ return;
+
+ sal_Int32 nSecurityId;
+ aManager.add(xCertificate, mxSecurityContext, "", nSecurityId,
/*bAdESCompliant=*/false);
+ aManager.read(/*bUseTempStream=*/true);
+ {
+ std::vector<SignatureInformation>& rInformations
+ = aManager.getCurrentSignatureInformations();
+ CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(1),
rInformations.size());
+
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+ rInformations[0].nStatus);
+ }
+
+ aManager.add(xCertificate, mxSecurityContext, "", nSecurityId,
/*bAdESCompliant=*/false);
+ aManager.read(/*bUseTempStream=*/true);
+ {
+ std::vector<SignatureInformation>& rInformations
+ = aManager.getCurrentSignatureInformations();
+ CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(2),
rInformations.size());
+
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+ rInformations[1].nStatus);
+ }
+
+ aManager.add(xCertificate, mxSecurityContext, "", nSecurityId,
/*bAdESCompliant=*/false);
+ aManager.read(/*bUseTempStream=*/true);
+ {
+ std::vector<SignatureInformation>& rInformations
+ = aManager.getCurrentSignatureInformations();
+ CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(3),
rInformations.size());
+
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+ rInformations[2].nStatus);
+ }
+
+ aManager.write(/*bXAdESCompliantIfODF=*/true);
+ uno::Reference<embed::XTransactedObject> xTransactedObject(xStorage,
uno::UNO_QUERY);
+ xTransactedObject->commit();
}
- aManager.add(xCertificate, mxSecurityContext, "", nSecurityId,
/*bAdESCompliant=*/false);
- aManager.read(/*bUseTempStream=*/true);
- {
- std::vector<SignatureInformation>& rInformations
- = aManager.getCurrentSignatureInformations();
- CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(2),
rInformations.size());
-
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
- rInformations[1].nStatus);
- }
+ Scheduler::ProcessEventsToIdle();
- aManager.add(xCertificate, mxSecurityContext, "", nSecurityId,
/*bAdESCompliant=*/false);
- aManager.read(/*bUseTempStream=*/true);
- {
- std::vector<SignatureInformation>& rInformations
- = aManager.getCurrentSignatureInformations();
- CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(3),
rInformations.size());
-
CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
- rInformations[2].nStatus);
- }
- aManager.write(/*bXAdESCompliantIfODF=*/true);
- uno::Reference<embed::XTransactedObject> xTransactedObject(xStorage,
uno::UNO_QUERY);
- xTransactedObject->commit();
+ createDoc(aTempFile.GetURL());
+
+ SfxBaseModel* pBaseModel = dynamic_cast<SfxBaseModel*>(mxComponent.get());
+ CPPUNIT_ASSERT(pBaseModel);
+ SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell();
+ CPPUNIT_ASSERT(pObjectShell);
+ CPPUNIT_ASSERT_EQUAL(SignatureState::PARTIAL_OK,
pObjectShell->GetDocumentSignatureState());
}
/// Works with an existing good XAdES signature.
diff --git a/xmlsecurity/source/helper/ooxmlsecexporter.cxx
b/xmlsecurity/source/helper/ooxmlsecexporter.cxx
index 4cdf82ef8561..cac2196a3e28 100644
--- a/xmlsecurity/source/helper/ooxmlsecexporter.cxx
+++ b/xmlsecurity/source/helper/ooxmlsecexporter.cxx
@@ -65,6 +65,7 @@ public:
return m_xDocumentHandler;
}
+ void writeSignature();
void writeSignedInfo();
void writeCanonicalizationMethod();
void writeCanonicalizationTransform();
@@ -224,7 +225,7 @@ void OOXMLSecExporter::Impl::writeKeyInfo()
void OOXMLSecExporter::Impl::writePackageObject()
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("Id", "idPackageObject");
+ pAttributeList->AddAttribute("Id", "idPackageObject_" +
m_rInformation.ouSignatureId);
m_xDocumentHandler->startElement("Object",
uno::Reference<xml::sax::XAttributeList>(pAttributeList));
@@ -302,8 +303,8 @@ void
OOXMLSecExporter::Impl::writePackageObjectSignatureProperties()
"SignatureProperties", uno::Reference<xml::sax::XAttributeList>(new
SvXMLAttributeList()));
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("Id", "idSignatureTime");
- pAttributeList->AddAttribute("Target", "#idPackageSignature");
+ pAttributeList->AddAttribute("Id", "idSignatureTime_" +
m_rInformation.ouSignatureId);
+ pAttributeList->AddAttribute("Target", "#" +
m_rInformation.ouSignatureId);
m_xDocumentHandler->startElement("SignatureProperty",
uno::Reference<xml::sax::XAttributeList>(pAttributeList));
}
@@ -382,7 +383,7 @@ void OOXMLSecExporter::Impl::writeOfficeObject()
{
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("Id", "idOfficeObject");
+ pAttributeList->AddAttribute("Id", "idOfficeObject_" +
m_rInformation.ouSignatureId);
m_xDocumentHandler->startElement("Object",
uno::Reference<xml::sax::XAttributeList>(pAttributeList));
}
@@ -390,8 +391,8 @@ void OOXMLSecExporter::Impl::writeOfficeObject()
"SignatureProperties", uno::Reference<xml::sax::XAttributeList>(new
SvXMLAttributeList()));
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("Id", "idOfficeV1Details");
- pAttributeList->AddAttribute("Target", "#idPackageSignature");
+ pAttributeList->AddAttribute("Id", "idOfficeV1Details_" +
m_rInformation.ouSignatureId);
+ pAttributeList->AddAttribute("Target", "#" +
m_rInformation.ouSignatureId);
m_xDocumentHandler->startElement("SignatureProperty",
uno::Reference<xml::sax::XAttributeList>(pAttributeList));
}
@@ -479,7 +480,7 @@ void OOXMLSecExporter::Impl::writePackageSignature()
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
pAttributeList->AddAttribute("xmlns:xd", NS_XD);
- pAttributeList->AddAttribute("Target", "#idPackageSignature");
+ pAttributeList->AddAttribute("Target", "#" +
m_rInformation.ouSignatureId);
m_xDocumentHandler->startElement("xd:QualifyingProperties",
uno::Reference<xml::sax::XAttributeList>(pAttributeList));
}
@@ -521,6 +522,25 @@ void OOXMLSecExporter::Impl::writeSignatureLineImages()
m_xDocumentHandler->endElement("Object");
}
+void OOXMLSecExporter::Impl::writeSignature()
+{
+ rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
+ pAttributeList->AddAttribute("xmlns", NS_XMLDSIG);
+ pAttributeList->AddAttribute("Id", m_rInformation.ouSignatureId);
+ getDocumentHandler()->startElement("Signature",
+
uno::Reference<xml::sax::XAttributeList>(pAttributeList));
+
+ writeSignedInfo();
+ writeSignatureValue();
+ writeKeyInfo();
+ writePackageObject();
+ writeOfficeObject();
+ writePackageSignature();
+ writeSignatureLineImages();
+
+ getDocumentHandler()->endElement("Signature");
+}
+
OOXMLSecExporter::OOXMLSecExporter(
const uno::Reference<uno::XComponentContext>& xComponentContext,
const uno::Reference<embed::XStorage>& xRootStorage,
@@ -533,23 +553,6 @@ OOXMLSecExporter::OOXMLSecExporter(
OOXMLSecExporter::~OOXMLSecExporter() = default;
-void OOXMLSecExporter::writeSignature()
-{
- rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("xmlns", NS_XMLDSIG);
- pAttributeList->AddAttribute("Id", "idPackageSignature");
- m_pImpl->getDocumentHandler()->startElement(
- "Signature", uno::Reference<xml::sax::XAttributeList>(pAttributeList));
-
- m_pImpl->writeSignedInfo();
- m_pImpl->writeSignatureValue();
- m_pImpl->writeKeyInfo();
- m_pImpl->writePackageObject();
- m_pImpl->writeOfficeObject();
- m_pImpl->writePackageSignature();
- m_pImpl->writeSignatureLineImages();
-
- m_pImpl->getDocumentHandler()->endElement("Signature");
-}
+void OOXMLSecExporter::writeSignature() { m_pImpl->writeSignature(); }
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmlsecurity/source/helper/xsecsign.cxx
b/xmlsecurity/source/helper/xsecsign.cxx
index 76460d906c90..fdbd0f614cfb 100644
--- a/xmlsecurity/source/helper/xsecsign.cxx
+++ b/xmlsecurity/source/helper/xsecsign.cxx
@@ -151,14 +151,14 @@ css::uno::Reference<
css::xml::crypto::sax::XReferenceResolvedListener > XSecCon
}
else // OOXML
{
- internalSignatureInfor.signatureInfor.ouSignatureId =
"idPackageSignature";
+ OUString aID = createId();
+ internalSignatureInfor.signatureInfor.ouSignatureId = aID;
-
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idPackageObject", -1, OUString());
+
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idPackageObject_" + aID, -1, OUString());
size++;
-
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idOfficeObject", -1, OUString());
+
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idOfficeObject_" + aID, -1, OUString());
size++;
- OUString aId = "idSignedProperties_" +
internalSignatureInfor.signatureInfor.ouSignatureId;
-
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, aId, -1, OUString());
+
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idSignedProperties_" + aID, -1, OUString());
size++;
}
