[Libreoffice-commits] core.git: Branch 'libreoffice-6-2-3' - include/svl xmlsecurity/inc xmlsecurity/qa xmlsecurity/source
Miklos Vajna (via logerrit)
logerrit at kemper.freedesktop.org
Wed Apr 10 06:05:55 UTC 2019
include/svl/sigstruct.hxx | 5 +
xmlsecurity/inc/xsecctl.hxx | 7 +-
xmlsecurity/qa/unit/signing/data/notype-xades.odt |binary
xmlsecurity/qa/unit/signing/signing.cxx | 61 ++++++++++++++++++++++
xmlsecurity/source/helper/ooxmlsecparser.cxx | 2
xmlsecurity/source/helper/xsecctl.cxx | 4 -
xmlsecurity/source/helper/xsecparser.cxx | 4 +
xmlsecurity/source/helper/xsecsign.cxx | 17 +++---
xmlsecurity/source/helper/xsecverify.cxx | 6 +-
9 files changed, 87 insertions(+), 19 deletions(-)
New commits:
commit 12973e1449d981989588b2ca80593e494bd156f5
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon Apr 8 21:37:23 2019 +0200
Commit: Tomaž Vajngerl <quikee at gmail.com>
CommitDate: Wed Apr 10 08:05:32 2019 +0200
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
Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>
(cherry picked from commit f82e3b03162bff8ecd0409be21744f2c2b2c9144)
Change-Id: I30f892b292f84a0575a3d4ef5ccf3eddbe0090ca
Reviewed-on: https://gerrit.libreoffice.org/70458
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
Tested-by: Xisco Faulí <xiscofauli at libreoffice.org>
Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>
diff --git a/include/svl/sigstruct.hxx b/include/svl/sigstruct.hxx
index 3498aff008fa..ceed929752ab 100644
--- a/include/svl/sigstruct.hxx
+++ b/include/svl/sigstruct.hxx
@@ -48,6 +48,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),
@@ -57,12 +59,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
Binary files /dev/null and b/xmlsecurity/qa/unit/signing/data/notype-xades.odt differ
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index d9507982486b..bed9d0df8b0f 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -101,6 +101,7 @@ public:
#endif
void test96097Calc();
void test96097Doc();
+ void testXAdESNotype();
/// Creates a XAdES signature from scratch.
void testXAdES();
/// Works with an existing good XAdES signature.
@@ -144,6 +145,7 @@ public:
#endif
CPPUNIT_TEST(test96097Calc);
CPPUNIT_TEST(test96097Doc);
+ CPPUNIT_TEST(testXAdESNotype);
CPPUNIT_TEST(testXAdES);
CPPUNIT_TEST(testXAdESGood);
CPPUNIT_TEST(testSignatureLineOOXML);
@@ -781,6 +783,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 6844162c0151..457ef66bf24b 100644
--- a/xmlsecurity/source/helper/ooxmlsecparser.cxx
+++ b/xmlsecurity/source/helper/ooxmlsecparser.cxx
@@ -72,7 +72,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 f6d1f89b6a6b..5fdc45c2c8c0 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 d24f5f5c5ec8..532ba07a0298 100644
--- a/xmlsecurity/source/helper/xsecparser.cxx
+++ b/xmlsecurity/source/helper/xsecparser.cxx
@@ -129,12 +129,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 da1122ccc3ec..d8089b1773b3 100644
--- a/xmlsecurity/source/helper/xsecsign.cxx
+++ b/xmlsecurity/source/helper/xsecsign.cxx
@@ -138,12 +138,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++;
}
@@ -151,17 +152,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++;
}
@@ -189,7 +190,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
@@ -197,7 +198,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 edd78902313b..305e5aa28128 100644
--- a/xmlsecurity/source/helper/xsecverify.cxx
+++ b/xmlsecurity/source/helper/xsecverify.cxx
@@ -148,7 +148,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())
{
@@ -156,7 +156,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(
@@ -189,7 +189,7 @@ void XSecController::addStreamReference(
}
}
- isi.addReference(type, nDigestID, ouUri, -1);
+ isi.addReference(type, nDigestID, ouUri, -1, OUString());
}
void XSecController::setReferenceCount() const
More information about the Libreoffice-commits
mailing list