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>

1 files changed, 78 insertions, 26 deletions

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())