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>
diff --git a/include/svl/sigstruct.hxx b/include/svl/sigstruct.hxx
index 3498aff..ceed929 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 2620bc6..e84ea1d 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 0000000..4f96d4b
--- /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 d950798..bed9d0d 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 6844162..457ef66 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 f6d1f89..5fdc45c 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 d24f5f5..532ba07 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 da1122c..d8089b1 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 edd7890..305e5aa 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
