xmlsecurity: improve handling of multiple certificates per X509Data

It turns out that an X509Data element can contain an arbitrary number of
each of its child elements.

How exactly certificates of an issuer chain may or should be distributed
across multiple X509Data elements isn't terribly obvious.

One thing that is clear is that any element that refers to or contains
one particular certificate has to be a child of the same X509Data
element, although in no particular order, so try to match the 2 such
elements that the parser supports in XSecController::setX509Data().

Presumably the only way it makes sense to have multiple signing
certificates is if they all contain the same key but are signed by
different CAs. This case isn't handled currently; CheckX509Data() will
complain there's not a single chain and validation of the certificates
will fail.

Change-Id: I9633a980b0c18d58dfce24fc59396a833498a77d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111500
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

11 files changed, 200 insertions, 105 deletions

diff --git a/xmlsecurity/inc/xsecctl.hxx b/xmlsecurity/inc/xsecctl.hxx
index 698b1eb72c6f..9fa4fec004a0 100644
--- a/xmlsecurity/inc/xsecctl.hxx
+++ b/xmlsecurity/inc/xsecctl.hxx
@@ -262,7 +262,9 @@ private:
         sal_Int32 nDigestID );
     void setReferenceCount() const;
 
-    void setX509Data(SignatureInformation::X509Data const& rData);
+    void setX509Data(
+        std::vector<std::pair<OUString, OUString>> & rX509IssuerSerials,
+        std::vector<OUString> const& rX509Certificates);
     void setX509CertDigest(
         OUString const& rCertDigest, sal_Int32 const nReferenceDigestID,
         std::u16string_view const& rX509IssuerName, std::u16string_view const& rX509SerialNumber);
diff --git a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
index c848c3ea5049..044676688389 100644
--- a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
+++ b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
@@ -593,7 +593,7 @@ void DigitalSignaturesDialog::ImplFillSignaturesBox()
                 if (!rInfo.ouGpgCertificate.isEmpty())
                     aType = "OpenPGP";
                 // XML based: XAdES or not.
-                else if (!rInfo.X509Datas.empty() && !rInfo.X509Datas.back().CertDigest.isEmpty())
+                else if (rInfo.GetSigningCertificate() && !rInfo.GetSigningCertificate()->CertDigest.isEmpty())
                     aType = "XAdES";
                 else
                     aType = "XML-DSig";
@@ -705,8 +705,8 @@ uno::Reference<security::XCertificate> DigitalSignaturesDialog::getCertificate(c
     uno::Reference<security::XCertificate> xCert;
 
     //First we try to get the certificate which is embedded in the XML Signature
-    if (xSecEnv.is() && !rInfo.X509Datas.empty() && !rInfo.X509Datas.back().X509Certificate.isEmpty())
-        xCert = xSecEnv->createCertificateFromAscii(rInfo.X509Datas.back().X509Certificate);
+    if (xSecEnv.is() && rInfo.GetSigningCertificate() && !rInfo.GetSigningCertificate()->X509Certificate.isEmpty())
+        xCert = xSecEnv->createCertificateFromAscii(rInfo.GetSigningCertificate()->X509Certificate);
     else {
         //There must be an embedded certificate because we use it to get the
         //issuer name. We cannot use /Signature/KeyInfo/X509Data/X509IssuerName
@@ -718,10 +718,10 @@ uno::Reference<security::XCertificate> DigitalSignaturesDialog::getCertificate(c
     }
 
     //In case there is no embedded certificate we try to get it from a local store
-    if (!xCert.is() && xSecEnv.is() && !rInfo.X509Datas.empty())
+    if (!xCert.is() && xSecEnv.is() && rInfo.GetSigningCertificate())
     {
-        xCert = xSecEnv->getCertificate(rInfo.X509Datas.back().X509IssuerName,
-            xmlsecurity::numericStringToBigInteger(rInfo.X509Datas.back().X509SerialNumber));
+        xCert = xSecEnv->getCertificate(rInfo.GetSigningCertificate()->X509IssuerName,
+            xmlsecurity::numericStringToBigInteger(rInfo.GetSigningCertificate()->X509SerialNumber));
     }
     if (!xCert.is() && xGpgSecEnv.is())
         xCert = xGpgSecEnv->getCertificate( rInfo.ouGpgKeyID, xmlsecurity::numericStringToBigInteger(u"") );
diff --git a/xmlsecurity/source/helper/documentsignaturehelper.cxx b/xmlsecurity/source/helper/documentsignaturehelper.cxx
index 92331a41a7e6..e05e1ee1aac2 100644
--- a/xmlsecurity/source/helper/documentsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/documentsignaturehelper.cxx
@@ -500,22 +500,22 @@ void DocumentSignatureHelper::writeDigestMethod(
 
 static void WriteXadesCert(
     uno::Reference<xml::sax::XDocumentHandler> const& xDocumentHandler,
-    SignatureInformation::X509Data const& rData)
+    SignatureInformation::X509CertInfo const& rCertInfo)
 {
     xDocumentHandler->startElement("xd:Cert", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
     xDocumentHandler->startElement("xd:CertDigest", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
     DocumentSignatureHelper::writeDigestMethod(xDocumentHandler);
     xDocumentHandler->startElement("DigestValue", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-    assert(!rData.CertDigest.isEmpty());
-    xDocumentHandler->characters(rData.CertDigest);
+    assert(!rCertInfo.CertDigest.isEmpty());
+    xDocumentHandler->characters(rCertInfo.CertDigest);
     xDocumentHandler->endElement("DigestValue");
     xDocumentHandler->endElement("xd:CertDigest");
     xDocumentHandler->startElement("xd:IssuerSerial", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
     xDocumentHandler->startElement("X509IssuerName", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-    xDocumentHandler->characters(rData.X509IssuerName);
+    xDocumentHandler->characters(rCertInfo.X509IssuerName);
     xDocumentHandler->endElement("X509IssuerName");
     xDocumentHandler->startElement("X509SerialNumber", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-    xDocumentHandler->characters(rData.X509SerialNumber);
+    xDocumentHandler->characters(rCertInfo.X509SerialNumber);
     xDocumentHandler->endElement("X509SerialNumber");
     xDocumentHandler->endElement("xd:IssuerSerial");
     xDocumentHandler->endElement("xd:Cert");
@@ -537,18 +537,23 @@ void DocumentSignatureHelper::writeSignedProperties(
     xDocumentHandler->characters(sDate);
     xDocumentHandler->endElement("xd:SigningTime");
     xDocumentHandler->startElement("xd:SigningCertificate", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-    assert(!signatureInfo.X509Datas.empty() || !signatureInfo.ouGpgKeyID.isEmpty());
-    if (!signatureInfo.X509Datas.empty())
+    assert(signatureInfo.GetSigningCertificate() || !signatureInfo.ouGpgKeyID.isEmpty());
+    if (signatureInfo.GetSigningCertificate())
     {
-        for (auto const& it : signatureInfo.X509Datas)
+        // how should this deal with multiple X509Data elements?
+        // for now, let's write all of the certificates ...
+        for (auto const& rData : signatureInfo.X509Datas)
         {
-            WriteXadesCert(xDocumentHandler, it);
+            for (auto const& it : rData)
+            {
+                WriteXadesCert(xDocumentHandler, it);
+            }
         }
     }
     else
     {
         // for PGP, write empty mandatory X509IssuerName, X509SerialNumber
-        SignatureInformation::X509Data temp;
+        SignatureInformation::X509CertInfo temp;
         temp.CertDigest = signatureInfo.ouGpgKeyID;
         WriteXadesCert(xDocumentHandler, temp);
     }
diff --git a/xmlsecurity/source/helper/ooxmlsecexporter.cxx b/xmlsecurity/source/helper/ooxmlsecexporter.cxx
index e9a4e02fb222..324657a96ee0 100644
--- a/xmlsecurity/source/helper/ooxmlsecexporter.cxx
+++ b/xmlsecurity/source/helper/ooxmlsecexporter.cxx
@@ -201,15 +201,19 @@ void OOXMLSecExporter::Impl::writeKeyInfo()
 {
     m_xDocumentHandler->startElement(
         "KeyInfo", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-    assert(!m_rInformation.X509Datas.empty());
-    for (auto const& it : m_rInformation.X509Datas)
+    assert(m_rInformation.GetSigningCertificate());
+    for (auto const& rData : m_rInformation.X509Datas)
     {
         m_xDocumentHandler->startElement(
             "X509Data", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-        m_xDocumentHandler->startElement(
-            "X509Certificate", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
-        m_xDocumentHandler->characters(it.X509Certificate);
-        m_xDocumentHandler->endElement("X509Certificate");
+        for (auto const& it : rData)
+        {
+            m_xDocumentHandler->startElement(
+                "X509Certificate",
+                uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));
+            m_xDocumentHandler->characters(it.X509Certificate);
+            m_xDocumentHandler->endElement("X509Certificate");
+        }
         m_xDocumentHandler->endElement("X509Data");
     }
     m_xDocumentHandler->endElement("KeyInfo");
diff --git a/xmlsecurity/source/helper/ooxmlsecparser.cxx b/xmlsecurity/source/helper/ooxmlsecparser.cxx
index 363b0f7bba59..a25872fc057d 100644
--- a/xmlsecurity/source/helper/ooxmlsecparser.cxx
+++ b/xmlsecurity/source/helper/ooxmlsecparser.cxx
@@ -187,11 +187,17 @@ void SAL_CALL OOXMLSecParser::endElement(const OUString& rName)
     }
     else if (rName == "X509Data")
     {
-        SignatureInformation::X509Data temp;
-        temp.X509Certificate = m_aX509Certificate;
-        temp.X509IssuerName = m_aX509IssuerName;
-        temp.X509SerialNumber = m_aX509SerialNumber;
-        m_pXSecController->setX509Data(temp);
+        std::vector<std::pair<OUString, OUString>> X509IssuerSerials;
+        std::vector<OUString> X509Certificates;
+        if (!m_aX509Certificate.isEmpty())
+        {
+            X509Certificates.emplace_back(m_aX509Certificate);
+        }
+        if (!m_aX509IssuerName.isEmpty() && !m_aX509SerialNumber.isEmpty())
+        {
+            X509IssuerSerials.emplace_back(m_aX509IssuerName, m_aX509SerialNumber);
+        }
+        m_pXSecController->setX509Data(X509IssuerSerials, X509Certificates);
     }
     else if (rName == "X509Certificate")
     {
diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
index 6ebdeb0a9c4f..d336b696cb18 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -506,10 +506,11 @@ PDFSignatureHelper::GetDocumentSignatureInformations(
         security::DocumentSignatureInformation& rExternal = aRet[i];
         rExternal.SignatureIsValid
             = rInternal.nStatus == xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;
-        if (!rInternal.X509Datas.empty() && !rInternal.X509Datas.back().X509Certificate.isEmpty())
+        if (rInternal.GetSigningCertificate()
+            && !rInternal.GetSigningCertificate()->X509Certificate.isEmpty())
         {
-            rExternal.Signer
-                = xSecEnv->createCertificateFromAscii(rInternal.X509Datas.back().X509Certificate);
+            rExternal.Signer = xSecEnv->createCertificateFromAscii(
+                rInternal.GetSigningCertificate()->X509Certificate);
         }
         rExternal.PartialDocumentSignature = rInternal.bPartialDocumentSignature;
 
diff --git a/xmlsecurity/source/helper/xmlsignaturehelper.cxx b/xmlsecurity/source/helper/xmlsignaturehelper.cxx
index ba6717c32ff0..94a128b05d75 100644
--- a/xmlsecurity/source/helper/xmlsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/xmlsignaturehelper.cxx
@@ -557,19 +557,19 @@ void XMLSignatureHelper::CreateAndWriteOOXMLSignature(const uno::Reference<embed
  */
 static auto CheckX509Data(
     uno::Reference<xml::crypto::XSecurityEnvironment> const& xSecEnv,
-    std::vector<SignatureInformation::X509Data> const& rX509Datas,
+    std::vector<SignatureInformation::X509CertInfo> const& rX509CertInfos,
     std::vector<uno::Reference<security::XCertificate>> & rCerts,
-    std::vector<SignatureInformation::X509Data> & rSorted) -> bool
+    std::vector<SignatureInformation::X509CertInfo> & rSorted) -> bool
 {
     assert(rCerts.empty());
     assert(rSorted.empty());
-    if (rX509Datas.empty())
+    if (rX509CertInfos.empty())
     {
         SAL_WARN("xmlsecurity.comp", "no X509Data");
         return false;
     }
     std::vector<uno::Reference<security::XCertificate>> certs;
-    for (SignatureInformation::X509Data const& it : rX509Datas)
+    for (SignatureInformation::X509CertInfo const& it : rX509CertInfos)
     {
         if (!it.X509Certificate.isEmpty())
         {
@@ -664,10 +664,10 @@ static auto CheckX509Data(
     }
 
     // success
-    assert(chain.size() == rX509Datas.size());
+    assert(chain.size() == rX509CertInfos.size());
     for (auto const& it : chain)
     {
-        rSorted.emplace_back(rX509Datas[it]);
+        rSorted.emplace_back(rX509CertInfos[it]);
         rCerts.emplace_back(certs[it]);
     }
     return true;
@@ -685,7 +685,22 @@ XMLSignatureHelper::CheckAndUpdateSignatureInformation(
 
     std::vector<uno::Reference<security::XCertificate>> certs;
     std::vector<SignatureInformation::X509Data> datas;
-    CheckX509Data(xSecEnv, rInfo.X509Datas, certs, datas);
+    // TODO: for now, just merge all X509Datas together for checking...
+    // (this will probably break round-trip of signature with multiple X509Data,
+    // no idea if that is a problem)
+    SignatureInformation::X509Data temp;
+    SignatureInformation::X509Data tempResult;
+    for (auto const& rData : rInfo.X509Datas)
+    {
+        for (auto const& it : rData)
+        {
+            temp.emplace_back(it);
+        }
+    }
+    if (CheckX509Data(xSecEnv, temp, certs, tempResult))
+    {
+        datas.emplace_back(tempResult);
+    }
 
     // rInfo is a copy, update the original
     mpXSecController->UpdateSignatureInformation(rInfo.nSecurityId, datas);
diff --git a/xmlsecurity/source/helper/xsecctl.cxx b/xmlsecurity/source/helper/xsecctl.cxx
index a05d54ab1755..1bf0ffad199b 100644
--- a/xmlsecurity/source/helper/xsecctl.cxx
+++ b/xmlsecurity/source/helper/xsecctl.cxx
@@ -758,43 +758,46 @@ void XSecController::exportSignature(
             }
             else
             {
-                assert(!signatureInfo.X509Datas.empty());
-                for (auto const& it : signatureInfo.X509Datas)
+                assert(signatureInfo.GetSigningCertificate());
+                for (auto const& rData : signatureInfo.X509Datas)
                 {
                     /* Write X509Data element */
                     xDocumentHandler->startElement(
                         "X509Data",
                         css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
                     {
-                        /* Write X509IssuerSerial element */
-                        xDocumentHandler->startElement(
-                            "X509IssuerSerial",
-                            css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
-                        {
-                            /* Write X509IssuerName element */
-                            xDocumentHandler->startElement(
-                                "X509IssuerName",
-                                css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
-                            xDocumentHandler->characters(it.X509IssuerName);
-                            xDocumentHandler->endElement( "X509IssuerName" );
-
-                            /* Write X509SerialNumber element */
-                            xDocumentHandler->startElement(
-                                "X509SerialNumber",
-                                css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
-                            xDocumentHandler->characters(it.X509SerialNumber);
-                            xDocumentHandler->endElement( "X509SerialNumber" );
-                        }
-                        xDocumentHandler->endElement( "X509IssuerSerial" );
-
-                        /* Write X509Certificate element */
-                        if (!it.X509Certificate.isEmpty())
+                        for (auto const& it : rData)
                         {
+                            /* Write X509IssuerSerial element */
                             xDocumentHandler->startElement(
-                                "X509Certificate",
+                                "X509IssuerSerial",
                                 css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
-                            xDocumentHandler->characters(it.X509Certificate);
-                            xDocumentHandler->endElement( "X509Certificate" );
+                            {
+                                /* Write X509IssuerName element */
+                                xDocumentHandler->startElement(
+                                    "X509IssuerName",
+                                    css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
+                                xDocumentHandler->characters(it.X509IssuerName);
+                                xDocumentHandler->endElement( "X509IssuerName" );
+
+                                /* Write X509SerialNumber element */
+                                xDocumentHandler->startElement(
+                                    "X509SerialNumber",
+                                    css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
+                                xDocumentHandler->characters(it.X509SerialNumber);
+                                xDocumentHandler->endElement( "X509SerialNumber" );
+                            }
+                            xDocumentHandler->endElement( "X509IssuerSerial" );
+
+                            /* Write X509Certificate element */
+                            if (!it.X509Certificate.isEmpty())
+                            {
+                                xDocumentHandler->startElement(
+                                    "X509Certificate",
+                                    css::uno::Reference< css::xml::sax::XAttributeList > (new SvXMLAttributeList()));
+                                xDocumentHandler->characters(it.X509Certificate);
+                                xDocumentHandler->endElement( "X509Certificate" );
+                            }
                         }
                     }
                     xDocumentHandler->endElement( "X509Data" );
diff --git a/xmlsecurity/source/helper/xsecparser.cxx b/xmlsecurity/source/helper/xsecparser.cxx
index f09620823ba2..326515ba5b39 100644
--- a/xmlsecurity/source/helper/xsecparser.cxx
+++ b/xmlsecurity/source/helper/xsecparser.cxx
@@ -336,11 +336,15 @@ class XSecParser::DsX509IssuerSerialContext
         }
 };
 
+/// can't be sure what is supposed to happen here because the spec is clear as mud
 class XSecParser::DsX509DataContext
     : public XSecParser::Context
 {
     private:
-        SignatureInformation::X509Data m_X509Data;
+        // sigh... "No ordering is implied by the above constraints."
+        // so store the ball of mud in vectors and try to figure it out later.
+        std::vector<std::pair<OUString, OUString>> m_X509IssuerSerials;
+        std::vector<OUString> m_X509Certificates;
 
     public:
         DsX509DataContext(XSecParser & rParser,
@@ -351,7 +355,7 @@ class XSecParser::DsX509DataContext
 
         virtual void EndElement() override
         {
-            m_rParser.m_pXSecController->setX509Data(m_X509Data);
+            m_rParser.m_pXSecController->setX509Data(m_X509IssuerSerials, m_X509Certificates);
         }
 
         virtual std::unique_ptr<Context> CreateChildContext(
@@ -360,12 +364,13 @@ class XSecParser::DsX509DataContext
         {
             if (nNamespace == XML_NAMESPACE_DS && rName == "X509IssuerSerial")
             {
-                // can't require KeyInfo to be signed so pass in *true*
-                return std::make_unique<DsX509IssuerSerialContext>(m_rParser, std::move(pOldNamespaceMap), m_X509Data.X509IssuerName, m_X509Data.X509SerialNumber);
+                m_X509IssuerSerials.emplace_back();
+                return std::make_unique<DsX509IssuerSerialContext>(m_rParser, std::move(pOldNamespaceMap), m_X509IssuerSerials.back().first, m_X509IssuerSerials.back().second);
             }
             if (nNamespace == XML_NAMESPACE_DS && rName == "X509Certificate")
             {
-                return std::make_unique<DsX509CertificateContext>(m_rParser, std::move(pOldNamespaceMap), m_X509Data.X509Certificate);
+                m_X509Certificates.emplace_back();
+                return std::make_unique<DsX509CertificateContext>(m_rParser, std::move(pOldNamespaceMap), m_X509Certificates.back());
             }
             // missing: ds:X509SKI, ds:X509SubjectName, ds:X509CRL
             return XSecParser::Context::CreateChildContext(std::move(pOldNamespaceMap), nNamespace, rName);
diff --git a/xmlsecurity/source/helper/xsecsign.cxx b/xmlsecurity/source/helper/xsecsign.cxx
index 5cf213b6dada..3d9cac6dc485 100644
--- a/xmlsecurity/source/helper/xsecsign.cxx
+++ b/xmlsecurity/source/helper/xsecsign.cxx
@@ -209,10 +209,11 @@ void XSecController::setX509Certificate(
         InternalSignatureInformation isi(nSecurityId, nullptr);
         isi.signatureInfor.X509Datas.clear();
         isi.signatureInfor.X509Datas.emplace_back();
-        isi.signatureInfor.X509Datas.back().X509IssuerName = ouX509IssuerName;
-        isi.signatureInfor.X509Datas.back().X509SerialNumber = ouX509SerialNumber;
-        isi.signatureInfor.X509Datas.back().X509Certificate = ouX509Cert;
-        isi.signatureInfor.X509Datas.back().CertDigest = ouX509CertDigest;
+        isi.signatureInfor.X509Datas.back().emplace_back();
+        isi.signatureInfor.X509Datas.back().back().X509IssuerName = ouX509IssuerName;
+        isi.signatureInfor.X509Datas.back().back().X509SerialNumber = ouX509SerialNumber;
+        isi.signatureInfor.X509Datas.back().back().X509Certificate = ouX509Cert;
+        isi.signatureInfor.X509Datas.back().back().CertDigest = ouX509CertDigest;
         isi.signatureInfor.eAlgorithmID = eAlgorithmID;
         m_vInternalSignatureInformations.push_back( isi );
     }
@@ -222,10 +223,11 @@ void XSecController::setX509Certificate(
             = m_vInternalSignatureInformations[index].signatureInfor;
         si.X509Datas.clear();
         si.X509Datas.emplace_back();
-        si.X509Datas.back().X509IssuerName = ouX509IssuerName;
-        si.X509Datas.back().X509SerialNumber = ouX509SerialNumber;
-        si.X509Datas.back().X509Certificate = ouX509Cert;
-        si.X509Datas.back().CertDigest = ouX509CertDigest;
+        si.X509Datas.back().emplace_back();
+        si.X509Datas.back().back().X509IssuerName = ouX509IssuerName;
+        si.X509Datas.back().back().X509SerialNumber = ouX509SerialNumber;
+        si.X509Datas.back().back().X509Certificate = ouX509Cert;
+        si.X509Datas.back().back().CertDigest = ouX509CertDigest;
     }
 }
 
diff --git a/xmlsecurity/source/helper/xsecverify.cxx b/xmlsecurity/source/helper/xsecverify.cxx
index c630fd361ac2..b5b4af76b1cf 100644
--- a/xmlsecurity/source/helper/xsecverify.cxx
+++ b/xmlsecurity/source/helper/xsecverify.cxx
@@ -241,7 +241,9 @@ void XSecController::setReferenceCount() const
     xReferenceCollector->setReferenceCount( referenceCount );
 }
 
-void XSecController::setX509Data(SignatureInformation::X509Data const& rData)
+void XSecController::setX509Data(
+        std::vector<std::pair<OUString, OUString>> & rX509IssuerSerials,
+        std::vector<OUString> const& rX509Certificates)
 {
     if (m_vInternalSignatureInformations.empty())
     {
@@ -249,8 +251,52 @@ void XSecController::setX509Data(SignatureInformation::X509Data const& rData)
         return;
     }
     InternalSignatureInformation &isi = m_vInternalSignatureInformations.back();
-    // TODO: ImplVerifySignatures() handles all-empty case?
-    isi.signatureInfor.X509Datas.push_back(rData);
+    SignatureInformation::X509Data data;
+    // due to the excessive flexibility of the spec it's possible that there
+    // is both a reference to a cert and the cert itself in one X509Data
+    for (OUString const& it : rX509Certificates)
+    {
+        try
+        {
+            data.emplace_back();
+            data.back().X509Certificate = it;
+            uno::Reference<xml::crypto::XSecurityEnvironment> const xSecEnv(m_xSecurityContext->getSecurityEnvironment());
+            uno::Reference<security::XCertificate> const xCert(xSecEnv->createCertificateFromAscii(it));
+            if (!xCert.is())
+            {
+                SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate");
+                continue; // will be handled in CheckX509Data
+            }
+            OUString const issuerName(xCert->getIssuerName());
+            OUString const serialNumber(xmlsecurity::bigIntegerToNumericString(xCert->getSerialNumber()));
+            auto const iter = std::find_if(rX509IssuerSerials.begin(), rX509IssuerSerials.end(),
+                [&](auto const& rX509IssuerSerial) {
+                    return xmlsecurity::EqualDistinguishedNames(issuerName, rX509IssuerSerial.first)
+                        && serialNumber == rX509IssuerSerial.second;
+                });
+            if (iter != rX509IssuerSerials.end())
+            {
+                data.back().X509IssuerName = iter->first;
+                data.back().X509SerialNumber = iter->second;
+                rX509IssuerSerials.erase(iter);
+            }
+        }
+        catch (uno::Exception const&)
+        {
+            SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate");
+        }
+    }
+    // now handle any that are left...
+    for (auto const& it : rX509IssuerSerials)
+    {
+        data.emplace_back();
+        data.back().X509IssuerName = it.first;
+        data.back().X509SerialNumber = it.second;
+    }
+    if (!data.empty())
+    {
+        isi.signatureInfor.X509Datas.push_back(data);
+    }
 }
 
 void XSecController::setSignatureValue( OUString const & ouSignatureValue )
@@ -368,42 +414,48 @@ void XSecController::setX509CertDigest(
         return;
 
     InternalSignatureInformation& rInformation = m_vInternalSignatureInformations.back();
-    for (auto & it : rInformation.signatureInfor.X509Datas)
+    for (auto & rData : rInformation.signatureInfor.X509Datas)
     {
-        if (xmlsecurity::EqualDistinguishedNames(it.X509IssuerName, rX509IssuerName)
-            && it.X509SerialNumber == rX509SerialNumber)
+        for (auto & it : rData)
         {
-            it.CertDigest = rCertDigest;
-            return;
+            if (xmlsecurity::EqualDistinguishedNames(it.X509IssuerName, rX509IssuerName)
+                && it.X509SerialNumber == rX509SerialNumber)
+            {
+                it.CertDigest = rCertDigest;
+                return;
+            }
         }
     }
     // fall-back: read the actual certificates
-    for (auto & it : rInformation.signatureInfor.X509Datas)
+    for (auto & rData : rInformation.signatureInfor.X509Datas)
     {
-        if (!it.X509Certificate.isEmpty())
+        for (auto & it : rData)
         {
-            try
+            if (!it.X509Certificate.isEmpty())
             {
-                uno::Reference<xml::crypto::XSecurityEnvironment> const xSecEnv(m_xSecurityContext->getSecurityEnvironment());
-                uno::Reference<security::XCertificate> const xCert(xSecEnv->createCertificateFromAscii(it.X509Certificate));
-                if (!xCert.is())
+                try
                 {
-                    SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate");
+                    uno::Reference<xml::crypto::XSecurityEnvironment> const xSecEnv(m_xSecurityContext->getSecurityEnvironment());
+                    uno::Reference<security::XCertificate> const xCert(xSecEnv->createCertificateFromAscii(it.X509Certificate));
+                    if (!xCert.is())
+                    {
+                        SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate");
+                    }
+                    else if (xmlsecurity::EqualDistinguishedNames(xCert->getIssuerName(),rX509IssuerName)
+                        && xmlsecurity::bigIntegerToNumericString(xCert->getSerialNumber()) == rX509SerialNumber)
+                    {
+                        it.CertDigest = rCertDigest;
+                        // note: testInsertCertificate_PEM_DOCX requires these!
+                        it.X509SerialNumber = rX509SerialNumber;
+                        it.X509IssuerName = rX509IssuerName;
+                        return;
+                    }
                 }
-                else if (xmlsecurity::EqualDistinguishedNames(xCert->getIssuerName(),rX509IssuerName)
-                    && xmlsecurity::bigIntegerToNumericString(xCert->getSerialNumber()) == rX509SerialNumber)
+                catch (uno::Exception const&)
                 {
-                    it.CertDigest = rCertDigest;
-                    // note: testInsertCertificate_PEM_DOCX requires these!
-                    it.X509SerialNumber = rX509SerialNumber;
-                    it.X509IssuerName = rX509IssuerName;
-                    return;
+                    SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate");
                 }
             }
-            catch (uno::Exception const&)
-            {
-                SAL_INFO("xmlsecurity.helper", "cannot parse X509Certificate");
-            }
         }
     }
     if (!rInformation.signatureInfor.ouGpgCertificate.isEmpty())