[Libreoffice-commits] core.git: include/svl svl/source sw/source xmlsecurity/inc xmlsecurity/source

Michael Stahl (via logerrit) logerrit at kemper.freedesktop.org
Wed Mar 3 11:47:50 UTC 2021


 include/svl/sigstruct.hxx                              |   13 +-
 svl/source/crypto/cryptosign.cxx                       |   10 -
 sw/source/core/edit/edfcol.cxx                         |    4 
 xmlsecurity/inc/xsecctl.hxx                            |    4 
 xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx |   12 -
 xmlsecurity/source/helper/documentsignaturehelper.cxx  |   25 ++--
 xmlsecurity/source/helper/ooxmlsecexporter.cxx         |   16 +-
 xmlsecurity/source/helper/ooxmlsecparser.cxx           |   16 +-
 xmlsecurity/source/helper/pdfsignaturehelper.cxx       |    7 -
 xmlsecurity/source/helper/xmlsignaturehelper.cxx       |   29 +++-
 xmlsecurity/source/helper/xsecctl.cxx                  |   59 +++++----
 xmlsecurity/source/helper/xsecparser.cxx               |   15 +-
 xmlsecurity/source/helper/xsecsign.cxx                 |   18 +-
 xmlsecurity/source/helper/xsecverify.cxx               |  104 ++++++++++++-----
 14 files changed, 220 insertions(+), 112 deletions(-)

New commits:
commit 5af5ea893bcb8a8eb472ac11133da10e5a604e66
Author:     Michael Stahl <michael.stahl at allotropia.de>
AuthorDate: Wed Feb 24 19:18:51 2021 +0100
Commit:     Michael Stahl <michael.stahl at allotropia.de>
CommitDate: Wed Mar 3 12:47:04 2021 +0100

    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 at allotropia.de>

diff --git a/include/svl/sigstruct.hxx b/include/svl/sigstruct.hxx
index 4544fd182d90..77be7c48cae9 100644
--- a/include/svl/sigstruct.hxx
+++ b/include/svl/sigstruct.hxx
@@ -87,7 +87,7 @@ struct SignatureInformation
     sal_Int32 nSecurityId;
     css::xml::crypto::SecurityOperationStatus nStatus;
     SignatureReferenceInformations  vSignatureReferenceInfors;
-    struct X509Data
+    struct X509CertInfo
     {
         OUString X509IssuerName;
         OUString X509SerialNumber;
@@ -97,10 +97,21 @@ struct SignatureInformation
         /// The certificate owner (aka subject).
         OUString X509Subject;
     };
+    typedef std::vector<X509CertInfo> X509Data;
     // note: at parse time, it's unkown which one is the signing certificate;
     // ImplVerifySignatures() figures it out and puts it at the back
     std::vector<X509Data> X509Datas;
 
+    X509CertInfo const* GetSigningCertificate() const
+    {
+        if (X509Datas.empty())
+        {
+            return nullptr;
+        }
+        assert(!X509Datas.back().empty());
+        return & X509Datas.back().back();
+    }
+
     OUString ouGpgKeyID;
     OUString ouGpgCertificate;
     OUString ouGpgOwner;
diff --git a/svl/source/crypto/cryptosign.cxx b/svl/source/crypto/cryptosign.cxx
index f57b3e6639d8..8aa47ee36cba 100644
--- a/svl/source/crypto/cryptosign.cxx
+++ b/svl/source/crypto/cryptosign.cxx
@@ -2023,8 +2023,9 @@ bool Signing::Verify(const std::vector<unsigned char>& aData,
         OUStringBuffer aBuffer;
         comphelper::Base64::encode(aBuffer, aDerCert);
         SignatureInformation::X509Data temp;
-        temp.X509Certificate = aBuffer.makeStringAndClear();
-        temp.X509Subject = OUString(pCertificate->subjectName, PL_strlen(pCertificate->subjectName), RTL_TEXTENCODING_UTF8);
+        temp.emplace_back();
+        temp.back().X509Certificate = aBuffer.makeStringAndClear();
+        temp.back().X509Subject = OUString(pCertificate->subjectName, PL_strlen(pCertificate->subjectName), RTL_TEXTENCODING_UTF8);
         rInformation.X509Datas.clear();
         rInformation.X509Datas.emplace_back(temp);
     }
@@ -2206,8 +2207,9 @@ bool Signing::Verify(const std::vector<unsigned char>& aData,
         OUStringBuffer aBuffer;
         comphelper::Base64::encode(aBuffer, aDerCert);
         SignatureInformation::X509Data temp;
-        temp.X509Certificate = aBuffer.makeStringAndClear();
-        temp.X509Subject = GetSubjectName(pSignerCertContext);
+        temp.emplace_back();
+        temp.back().X509Certificate = aBuffer.makeStringAndClear();
+        temp.back().X509Subject = GetSubjectName(pSignerCertContext);
         rInformation.X509Datas.clear();
         rInformation.X509Datas.emplace_back(temp);
     }
diff --git a/sw/source/core/edit/edfcol.cxx b/sw/source/core/edit/edfcol.cxx
index 69b3cf60438d..ed6e90b12ede 100644
--- a/sw/source/core/edit/edfcol.cxx
+++ b/sw/source/core/edit/edfcol.cxx
@@ -408,8 +408,8 @@ std::pair<bool, OUString> lcl_MakeParagraphSignatureFieldText(const SignatureDes
             valid = valid
                     && aInfo.nStatus == xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;
 
-            assert(!aInfo.X509Datas.empty()); // it was valid
-            msg = SwResId(STR_SIGNED_BY) + ": " + aInfo.X509Datas.back().X509Subject + ", " +
+            assert(aInfo.GetSigningCertificate()); // it was valid
+            msg = SwResId(STR_SIGNED_BY) + ": " + aInfo.GetSigningCertificate()->X509Subject + ", " +
                 aDescr.msDate;
             msg += (!aDescr.msUsage.isEmpty() ? (" (" + aDescr.msUsage + "): ") : OUString(": "));
             msg += (valid ? SwResId(STR_VALID) : SwResId(STR_INVALID));
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())


More information about the Libreoffice-commits mailing list