summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2019-04-08 21:37:23 +0200
committerMichael Stahl <michael.stahl@allotropia.de>2021-03-29 16:46:58 +0200
commitfdd9cbbff99210b8892016ec8171e3520f4fa73a (patch)
tree2b2f0106ec9b47a9394b1bcd26cf2e702665a876
parent858abc7fed2cb94e54ac0b8185372a9ec3728d68 (diff)
tdf#123747 xmlsecurity, ODF sign roundtrip: preserve invalid reference type
Only add the correct type to new signatures to avoid breaking the hash of old ones. (cherry picked from commit 8a9d8238bd8f903393ff1184aa37f8973c81e2ba) Conflicts: xmlsecurity/qa/unit/signing/signing.cxx Change-Id: I30f892b292f84a0575a3d4ef5ccf3eddbe0090ca Reviewed-on: https://gerrit.libreoffice.org/70451 Tested-by: Jenkins Tested-by: Xisco Faulí <xiscofauli@libreoffice.org> Reviewed-by: Michael Stahl <Michael.Stahl@cib.de> (cherry picked from commit f82e3b03162bff8ecd0409be21744f2c2b2c9144)
-rw-r--r--include/svl/sigstruct.hxx5
-rw-r--r--xmlsecurity/inc/xsecctl.hxx7
-rw-r--r--xmlsecurity/qa/unit/signing/data/notype-xades.odtbin0 -> 13918 bytes
-rw-r--r--xmlsecurity/qa/unit/signing/signing.cxx61
-rw-r--r--xmlsecurity/source/helper/ooxmlsecparser.cxx2
-rw-r--r--xmlsecurity/source/helper/xsecctl.cxx4
-rw-r--r--xmlsecurity/source/helper/xsecparser.cxx4
-rw-r--r--xmlsecurity/source/helper/xsecsign.cxx17
-rw-r--r--xmlsecurity/source/helper/xsecverify.cxx6
9 files changed, 87 insertions, 19 deletions
diff --git a/include/svl/sigstruct.hxx b/include/svl/sigstruct.hxx
index 414e0cd88a41..a35043f54ec0 100644
--- a/include/svl/sigstruct.hxx
+++ b/include/svl/sigstruct.hxx
@@ -47,6 +47,8 @@ struct SignatureReferenceInformation
// For ODF: XAdES digests (SHA256) or the old SHA1, from css::xml::crypto::DigestID
sal_Int32 nDigestID;
OUString ouDigestValue;
+ /// Type of the reference: an URI (newer idSignedProperties references) or empty.
+ OUString ouType;
SignatureReferenceInformation() :
nType(SignatureReferenceType::SAMEDOCUMENT),
@@ -56,12 +58,13 @@ struct SignatureReferenceInformation
{
}
- SignatureReferenceInformation( SignatureReferenceType type, sal_Int32 digestID, const OUString& uri ) :
+ SignatureReferenceInformation( SignatureReferenceType type, sal_Int32 digestID, const OUString& uri, const OUString& rType ) :
SignatureReferenceInformation()
{
nType = type;
nDigestID = digestID;
ouURI = uri;
+ ouType = rType;
}
};
diff --git a/xmlsecurity/inc/xsecctl.hxx b/xmlsecurity/inc/xsecctl.hxx
index 2620bc6cbea9..e84ea1dbe042 100644
--- a/xmlsecurity/inc/xsecctl.hxx
+++ b/xmlsecurity/inc/xsecctl.hxx
@@ -89,10 +89,10 @@ public:
xReferenceResolvedListener = xListener;
}
- void addReference( SignatureReferenceType type, sal_Int32 digestID, const OUString& uri, sal_Int32 keeperId )
+ void addReference( SignatureReferenceType type, sal_Int32 digestID, const OUString& uri, sal_Int32 keeperId, const OUString& rType )
{
signatureInfor.vSignatureReferenceInfors.push_back(
- SignatureReferenceInformation(type, digestID, uri));
+ SignatureReferenceInformation(type, digestID, uri, rType));
vKeeperIds.push_back( keeperId );
}
};
@@ -261,7 +261,8 @@ private:
void switchGpgSignature();
void addReference(
const OUString& ouUri,
- sal_Int32 nDigestID );
+ sal_Int32 nDigestID,
+ const OUString& ouType );
void addStreamReference(
const OUString& ouUri,
bool isBinary,
diff --git a/xmlsecurity/qa/unit/signing/data/notype-xades.odt b/xmlsecurity/qa/unit/signing/data/notype-xades.odt
new file mode 100644
index 000000000000..4f96d4bd89c0
--- /dev/null
+++ b/xmlsecurity/qa/unit/signing/data/notype-xades.odt
Binary files differ
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index 9dd3335536b4..0cf7ed8cd637 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -102,6 +102,7 @@ public:
#endif
void test96097Calc();
void test96097Doc();
+ void testXAdESNotype();
/// Creates a XAdES signature from scratch.
void testXAdES();
/// Works with an existing good XAdES signature.
@@ -148,6 +149,7 @@ public:
#endif
CPPUNIT_TEST(test96097Calc);
CPPUNIT_TEST(test96097Doc);
+ CPPUNIT_TEST(testXAdESNotype);
CPPUNIT_TEST(testXAdES);
CPPUNIT_TEST(testXAdESGood);
CPPUNIT_TEST(testSignatureLineImages);
@@ -732,6 +734,65 @@ void SigningTest::test96097Doc()
}
}
+void SigningTest::testXAdESNotype()
+{
+ // Create a working copy.
+ utl::TempFile aTempFile;
+ aTempFile.EnableKillingFile();
+ OUString aURL = aTempFile.GetURL();
+ CPPUNIT_ASSERT_EQUAL(
+ osl::File::RC::E_None,
+ osl::File::copy(m_directories.getURLFromSrc(DATA_DIRECTORY) + "notype-xades.odt", aURL));
+
+ // Read existing signature.
+ 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.mxStore = xStorage;
+ aManager.maSignatureHelper.SetStorage(xStorage, "1.2");
+ aManager.read(/*bUseTempStream=*/false);
+
+ // Create a new 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);
+
+ // Write to storage.
+ aManager.read(/*bUseTempStream=*/true);
+ aManager.write(/*bXAdESCompliantIfODF=*/true);
+ uno::Reference<embed::XTransactedObject> xTransactedObject(xStorage, uno::UNO_QUERY);
+ xTransactedObject->commit();
+
+ // Parse the resulting XML.
+ uno::Reference<embed::XStorage> xMetaInf
+ = xStorage->openStorageElement("META-INF", embed::ElementModes::READ);
+ uno::Reference<io::XInputStream> xInputStream(
+ xMetaInf->openStreamElement("documentsignatures.xml", embed::ElementModes::READ),
+ uno::UNO_QUERY);
+ std::shared_ptr<SvStream> pStream(utl::UcbStreamHelper::CreateStream(xInputStream, true));
+ xmlDocPtr pXmlDoc = parseXmlStream(pStream.get());
+
+ // Without the accompanying fix in place, this test would have failed with "unexpected 'Type'
+ // 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']",
+ "Type");
+
+ // New signature always has the Type attribute.
+ assertXPath(pXmlDoc,
+ "/odfds:document-signatures/dsig:Signature[2]/dsig:SignedInfo/"
+ "dsig:Reference[@URI='#idSignedProperties']",
+ "Type", "http://uri.etsi.org/01903#SignedProperties");
+}
+
void SigningTest::testXAdES()
{
// Create an empty document, store it to a tempfile and load it as a storage.
diff --git a/xmlsecurity/source/helper/ooxmlsecparser.cxx b/xmlsecurity/source/helper/ooxmlsecparser.cxx
index cb5334cc8a59..92c302a05449 100644
--- a/xmlsecurity/source/helper/ooxmlsecparser.cxx
+++ b/xmlsecurity/source/helper/ooxmlsecparser.cxx
@@ -64,7 +64,7 @@ void SAL_CALL OOXMLSecParser::startElement(const OUString& rName, const uno::Ref
{
OUString aURI = xAttribs->getValueByName("URI");
if (aURI.startsWith("#"))
- m_pXSecController->addReference(aURI.copy(1), xml::crypto::DigestID::SHA1);
+ m_pXSecController->addReference(aURI.copy(1), xml::crypto::DigestID::SHA1, OUString());
else
{
m_aReferenceURI = aURI;
diff --git a/xmlsecurity/source/helper/xsecctl.cxx b/xmlsecurity/source/helper/xsecctl.cxx
index c587ae16ca0f..13ef9eb2e299 100644
--- a/xmlsecurity/source/helper/xsecctl.cxx
+++ b/xmlsecurity/source/helper/xsecctl.cxx
@@ -662,12 +662,12 @@ void XSecController::exportSignature(
"URI",
"#" + refInfor.ouURI);
- if (bXAdESCompliantIfODF && refInfor.ouURI == "idSignedProperties")
+ if (bXAdESCompliantIfODF && refInfor.ouURI == "idSignedProperties" && !refInfor.ouType.isEmpty())
{
// The reference which points to the SignedProperties
// shall have this specific type.
pAttributeList->AddAttribute("Type",
- "http://uri.etsi.org/01903#SignedProperties");
+ refInfor.ouType);
}
}
diff --git a/xmlsecurity/source/helper/xsecparser.cxx b/xmlsecurity/source/helper/xsecparser.cxx
index e20716f0a487..153a4aeddd77 100644
--- a/xmlsecurity/source/helper/xsecparser.cxx
+++ b/xmlsecurity/source/helper/xsecparser.cxx
@@ -128,12 +128,14 @@ void SAL_CALL XSecParser::startElement(
{
OUString ouUri = xAttribs->getValueByName("URI");
SAL_WARN_IF( ouUri.isEmpty(), "xmlsecurity.helper", "URI is empty" );
+ // Remember the type of this reference.
+ OUString ouType = xAttribs->getValueByName("Type");
if (ouUri.startsWith("#"))
{
/*
* remove the first character '#' from the attribute value
*/
- m_pXSecController->addReference( ouUri.copy(1), m_nReferenceDigestID );
+ m_pXSecController->addReference( ouUri.copy(1), m_nReferenceDigestID, ouType );
}
else
{
diff --git a/xmlsecurity/source/helper/xsecsign.cxx b/xmlsecurity/source/helper/xsecsign.cxx
index b4c050e3b7a2..657440b6753b 100644
--- a/xmlsecurity/source/helper/xsecsign.cxx
+++ b/xmlsecurity/source/helper/xsecsign.cxx
@@ -137,12 +137,13 @@ cssu::Reference< cssxc::sax::XReferenceResolvedListener > XSecController::prepar
{
internalSignatureInfor.signatureInfor.ouSignatureId = createId();
internalSignatureInfor.signatureInfor.ouPropertyId = createId();
- internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, internalSignatureInfor.signatureInfor.ouPropertyId, -1 );
+ internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, internalSignatureInfor.signatureInfor.ouPropertyId, -1, OUString() );
size++;
if (bXAdESCompliantIfODF)
{
- internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idSignedProperties", -1);
+ // 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");
size++;
}
@@ -150,17 +151,17 @@ cssu::Reference< cssxc::sax::XReferenceResolvedListener > XSecController::prepar
{
// Only mention the hash of the description in the signature if it's non-empty.
internalSignatureInfor.signatureInfor.ouDescriptionPropertyId = createId();
- internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, internalSignatureInfor.signatureInfor.ouDescriptionPropertyId, -1);
+ internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, internalSignatureInfor.signatureInfor.ouDescriptionPropertyId, -1, OUString());
size++;
}
}
else
{
- internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idPackageObject", -1);
+ internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idPackageObject", -1, OUString());
size++;
- internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idOfficeObject", -1);
+ internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idOfficeObject", -1, OUString());
size++;
- internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idSignedProperties", -1);
+ internalSignatureInfor.addReference(SignatureReferenceType::SAMEDOCUMENT, digestID, "idSignedProperties", -1, OUString());
size++;
}
@@ -188,7 +189,7 @@ void XSecController::signAStream( sal_Int32 securityId, const OUString& uri, boo
if (index == -1)
{
InternalSignatureInformation isi(securityId, nullptr);
- isi.addReference(type, digestID, uri, -1);
+ isi.addReference(type, digestID, uri, -1, OUString());
m_vInternalSignatureInformations.push_back( isi );
}
else
@@ -196,7 +197,7 @@ void XSecController::signAStream( sal_Int32 securityId, const OUString& uri, boo
// use sha512 for gpg signing unconditionally
if (!m_vInternalSignatureInformations[index].signatureInfor.ouGpgCertificate.isEmpty())
digestID = cssxc::DigestID::SHA512;
- m_vInternalSignatureInformations[index].addReference(type, digestID, uri, -1);
+ m_vInternalSignatureInformations[index].addReference(type, digestID, uri, -1, OUString());
}
}
diff --git a/xmlsecurity/source/helper/xsecverify.cxx b/xmlsecurity/source/helper/xsecverify.cxx
index 5a23de6f2619..cc63f2ef3a90 100644
--- a/xmlsecurity/source/helper/xsecverify.cxx
+++ b/xmlsecurity/source/helper/xsecverify.cxx
@@ -149,7 +149,7 @@ void XSecController::switchGpgSignature()
#endif
}
-void XSecController::addReference( const OUString& ouUri, sal_Int32 nDigestID )
+void XSecController::addReference( const OUString& ouUri, sal_Int32 nDigestID, const OUString& ouType )
{
if (m_vInternalSignatureInformations.empty())
{
@@ -157,7 +157,7 @@ void XSecController::addReference( const OUString& ouUri, sal_Int32 nDigestID )
return;
}
InternalSignatureInformation &isi = m_vInternalSignatureInformations.back();
- isi.addReference(SignatureReferenceType::SAMEDOCUMENT, nDigestID, ouUri, -1 );
+ isi.addReference(SignatureReferenceType::SAMEDOCUMENT, nDigestID, ouUri, -1, ouType );
}
void XSecController::addStreamReference(
@@ -190,7 +190,7 @@ void XSecController::addStreamReference(
}
}
- isi.addReference(type, nDigestID, ouUri, -1);
+ isi.addReference(type, nDigestID, ouUri, -1, OUString());
}
void XSecController::setReferenceCount() const