xmlsecurity/qa/unit/signing/signing.cxx | 136 ++++++++++++++++--
xmlsecurity/source/helper/documentsignaturehelper.cxx | 2
xmlsecurity/source/helper/ooxmlsecexporter.cxx | 4
xmlsecurity/source/helper/xsecctl.cxx | 23 +--
xmlsecurity/source/helper/xsecsign.cxx | 10 -
5 files changed, 152 insertions(+), 23 deletions(-)
New commits:
commit b883bc9d8ca4a9c6037166b2eff09095aef145e0
Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk>
AuthorDate: Wed Oct 27 14:15:17 2021 +0200
Commit: Tomaž Vajngerl <qui...@gmail.com>
CommitDate: Thu Oct 28 19:09:14 2021 +0200
xmlsec: signing the document fails the 3rd time (invalid signature)
Signing the document 3 or more times produces an invalid signature.
The cause of this is that xmlsec is confused because we have 3
signatures, which all have the same SignedProperties with the ID
"idSignedProperties", but it expect them to be unique.
This issue is fixed by making the ID unique with adding the ID of
the Signature to the SignedProperties ID, so this makes them unique
inside the same Signature.
Also UnsignedProperties have a unique ID usign the same approach,
but they aren't referenced - luckily.
Change-Id: I53c7249a82fc0623586548db9fa25bdc0e7c4101
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124278
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <qui...@gmail.com>
(cherry picked from commit fd5463343ab7f784070f1ab87a345eed20803d07)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124327
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx
b/xmlsecurity/qa/unit/signing/signing.cxx
index 9695a84af3bf..3f632366bf94 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -937,13 +937,13 @@ CPPUNIT_TEST_FIXTURE(SigningTest, testXAdESNotype)
// attribute", i.e. the signature without such an attribute was not
preserved correctly.
assertXPathNoAttribute(pXmlDoc,
"/odfds:document-signatures/dsig:Signature[1]/dsig:SignedInfo/"
- "dsig:Reference[@URI='#idSignedProperties']",
+ "dsig:Reference[starts-with(@URI,
'#idSignedProperties')]",
"Type");
// New signature always has the Type attribute.
assertXPath(pXmlDoc,
"/odfds:document-signatures/dsig:Signature[2]/dsig:SignedInfo/"
- "dsig:Reference[@URI='#idSignedProperties']",
+ "dsig:Reference[starts-with(@URI, '#idSignedProperties')]",
"Type", "http://uri.etsi.org/01903#SignedProperties";);
}
@@ -1002,12 +1002,132 @@ CPPUNIT_TEST_FIXTURE(SigningTest, testXAdES)
// Assert that the digest of the signing certificate is included.
assertXPath(pXmlDoc, "//xd:CertDigest", 1);
- // Assert that the Type attribute on the idSignedProperties reference is
- // not missing.
- assertXPath(pXmlDoc,
- "/odfds:document-signatures/dsig:Signature/dsig:SignedInfo/"
- "dsig:Reference[@URI='#idSignedProperties']",
- "Type", "http://uri.etsi.org/01903#SignedProperties";);
+ // Assert that the Type attribute is set on all URI's that start with
#idSignedProperties
+ assertXPath(pXmlDoc, "//dsig:Reference[starts-with(@URI,
'#idSignedProperties')]", "Type",
+ "http://uri.etsi.org/01903#SignedProperties";);
+}
+
+CPPUNIT_TEST_FIXTURE(SigningTest, testSigningMultipleTimes_ODT)
+{
+ createDoc("");
+
+ utl::TempFile aTempFile;
+ aTempFile.EnableKillingFile();
+ uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY);
+ utl::MediaDescriptor aMediaDescriptor;
+ 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);
+ }
+
+ 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);
+ }
+}
+
+CPPUNIT_TEST_FIXTURE(SigningTest, testSigningMultipleTimes_OOXML)
+{
+ createDoc("");
+
+ utl::TempFile aTempFile;
+ aTempFile.EnableKillingFile();
+ uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY);
+ utl::MediaDescriptor aMediaDescriptor;
+ 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);
+ }
+
+ 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();
}
/// Works with an existing good XAdES signature.
diff --git a/xmlsecurity/source/helper/documentsignaturehelper.cxx
b/xmlsecurity/source/helper/documentsignaturehelper.cxx
index 94f7d9a813fe..05c7b0beb205 100644
--- a/xmlsecurity/source/helper/documentsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/documentsignaturehelper.cxx
@@ -521,7 +521,7 @@ void DocumentSignatureHelper::writeSignedProperties(
{
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("Id", "idSignedProperties");
+ pAttributeList->AddAttribute("Id", "idSignedProperties_" +
signatureInfo.ouSignatureId);
xDocumentHandler->startElement("xd:SignedProperties",
uno::Reference<xml::sax::XAttributeList>(pAttributeList.get()));
}
diff --git a/xmlsecurity/source/helper/ooxmlsecexporter.cxx
b/xmlsecurity/source/helper/ooxmlsecexporter.cxx
index 8a60279404f8..133e78cc4416 100644
--- a/xmlsecurity/source/helper/ooxmlsecexporter.cxx
+++ b/xmlsecurity/source/helper/ooxmlsecexporter.cxx
@@ -160,7 +160,7 @@ void OOXMLSecExporter::Impl::writeSignedInfoReferences()
{
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- if (rReference.ouURI != "idSignedProperties")
+ if (!rReference.ouURI.startsWith("idSignedProperties"))
pAttributeList->AddAttribute("Type",
"http://www.w3.org/2000/09/xmldsig#Object";);
else
@@ -170,7 +170,7 @@ void OOXMLSecExporter::Impl::writeSignedInfoReferences()
m_xDocumentHandler->startElement(
"Reference",
uno::Reference<xml::sax::XAttributeList>(pAttributeList.get()));
}
- if (rReference.ouURI == "idSignedProperties")
+ if (rReference.ouURI.startsWith("idSignedProperties"))
{
m_xDocumentHandler->startElement(
"Transforms",
diff --git a/xmlsecurity/source/helper/xsecctl.cxx
b/xmlsecurity/source/helper/xsecctl.cxx
index 3f18d109494d..49a048e4ca15 100644
--- a/xmlsecurity/source/helper/xsecctl.cxx
+++ b/xmlsecurity/source/helper/xsecctl.cxx
@@ -532,7 +532,7 @@ void writeUnsignedProperties(
{
{
rtl::Reference<SvXMLAttributeList> pAttributeList(new
SvXMLAttributeList());
- pAttributeList->AddAttribute("Id", "idUnsignedProperties");
+ pAttributeList->AddAttribute("Id", "idUnsignedProperties_" +
signatureInfo.ouSignatureId);
xDocumentHandler->startElement("xd:UnsignedProperties",
uno::Reference<xml::sax::XAttributeList>(pAttributeList.get()));
}
@@ -649,16 +649,21 @@ void XSecController::exportSignature(
* same-document reference
*/
{
- pAttributeList->AddAttribute(
+ if (refInfor.ouURI.startsWith("idSignedProperties"))
+ {
+ pAttributeList->AddAttribute("URI",
"#idSignedProperties_" + signatureInfo.ouSignatureId);
+ if (bXAdESCompliantIfODF && !refInfor.ouType.isEmpty())
+ {
+ // The reference which points to the
SignedProperties
+ // shall have this specific type.
+ pAttributeList->AddAttribute("Type",
refInfor.ouType);
+ }
+ }
+ else
+ {
+ pAttributeList->AddAttribute(
"URI",
"#" + refInfor.ouURI);
-
- if (bXAdESCompliantIfODF && refInfor.ouURI ==
"idSignedProperties" && !refInfor.ouType.isEmpty())
- {
- // The reference which points to the SignedProperties
- // shall have this specific type.
- pAttributeList->AddAttribute("Type",
- refInfor.ouType);
}
}
diff --git a/xmlsecurity/source/helper/xsecsign.cxx
b/xmlsecurity/source/helper/xsecsign.cxx
index fd33a320d9bd..e0cda6c43695 100644
--- a/xmlsecurity/source/helper/xsecsign.cxx
+++ b/xmlsecurity/source/helper/xsecsign.cxx
@@ -134,8 +134,9 @@ css::uno::Reference<
css::xml::crypto::sax::XReferenceResolvedListener > XSecCon
if (bXAdESCompliantIfODF)
{
+ OUString aId = "idSignedProperties_" +
internalSignatureInfor.signatureInfor.ouSignatureId;
// We write a new reference, so it's possible to use the correct
type URI.
-
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idSignedProperties", -1,
"http://uri.etsi.org/01903#SignedProperties";);
+
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, aId, -1, "http://uri.etsi.org/01903#SignedProperties";);
size++;
}
@@ -147,13 +148,16 @@ css::uno::Reference<
css::xml::crypto::sax::XReferenceResolvedListener > XSecCon
size++;
}
}
- else
+ else // OOXML
{
+ internalSignatureInfor.signatureInfor.ouSignatureId =
"idPackageSignature";
+
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idPackageObject", -1, OUString());
size++;
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idOfficeObject", -1, OUString());
size++;
-
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, "idSignedProperties", -1, OUString());
+ OUString aId = "idSignedProperties_" +
internalSignatureInfor.signatureInfor.ouSignatureId;
+
internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT,
digestID, aId, -1, OUString());
size++;
}
