diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2019-04-08 21:37:23 +0200 |
---|---|---|
committer | Michael Stahl <michael.stahl@allotropia.de> | 2021-03-29 16:46:58 +0200 |
commit | fdd9cbbff99210b8892016ec8171e3520f4fa73a (patch) | |
tree | 2b2f0106ec9b47a9394b1bcd26cf2e702665a876 | |
parent | 858abc7fed2cb94e54ac0b8185372a9ec3728d68 (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.hxx | 5 | ||||
-rw-r--r-- | xmlsecurity/inc/xsecctl.hxx | 7 | ||||
-rw-r--r-- | xmlsecurity/qa/unit/signing/data/notype-xades.odt | bin | 0 -> 13918 bytes | |||
-rw-r--r-- | xmlsecurity/qa/unit/signing/signing.cxx | 61 | ||||
-rw-r--r-- | xmlsecurity/source/helper/ooxmlsecparser.cxx | 2 | ||||
-rw-r--r-- | xmlsecurity/source/helper/xsecctl.cxx | 4 | ||||
-rw-r--r-- | xmlsecurity/source/helper/xsecparser.cxx | 4 | ||||
-rw-r--r-- | xmlsecurity/source/helper/xsecsign.cxx | 17 | ||||
-rw-r--r-- | xmlsecurity/source/helper/xsecverify.cxx | 6 |
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 Binary files differnew file mode 100644 index 000000000000..4f96d4bd89c0 --- /dev/null +++ b/xmlsecurity/qa/unit/signing/data/notype-xades.odt 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 |