From b970e561433f1cbeebd43c92c92c98c0468cc483 Mon Sep 17 00:00:00 2001

From: Michael Stahl <michael.stahl@allotropia.de>

Date: Fri, 19 Feb 2021 17:56:21 +0100

Subject: [PATCH 5/6] CVE-2021-25633

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

xmlsecurity: ignore elements in ds:Object that aren't signed

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111253

Tested-by: Jenkins

Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

(cherry picked from commit 2bfa00e6bf4b2a310a8b8f5060acec85b5f7a3ce)

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111909

Reviewed-by: Caolán McNamara <caolanm@redhat.com>

(cherry picked from commit 94ce59dd02fcfcaa1eb4f195b45a9a2edbd58242)

Change-Id: I2e4411f0907b89e7ad6e0185cee8f12b600515e8

xmlsecurity: improve handling of multiple X509Data elements

Combine everything related to a certificate in a new struct X509Data.

The CertDigest is not actually written in the X509Data element but in

xades:Cert, so try to find the matching entry in

XSecController::setX509CertDigest().

There was a confusing interaction with PGP signatures, where ouGpgKeyID

was used for import, but export wrote the value from ouCertDigest

instead - this needed fixing.

The main point of this is enforcing a constraint from xmldsig-core 4.5.4:

  All certificates appearing in an X509Data element MUST relate to the

  validation key by either containing it or being part of a certification

  chain that terminates in a certificate containing the validation key.

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111254

Tested-by: Jenkins

Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

(cherry picked from commit 9e82509b09f5fe2eb77bcdb8fd193c71923abb67)

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.

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111500

Tested-by: Jenkins

Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

(cherry picked from commit 5af5ea893bcb8a8eb472ac11133da10e5a604e66)

xmlsecurity: add EqualDistinguishedNames()

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111545

Tested-by: Jenkins

Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

(cherry picked from commit 1d3da3486d827dd5e7a3bf1c7a533f5aa9860e42)

xmlsecurity: avoid exception in DigitalSignaturesDialog::getCertificate()

Fallback to PGP if there's no X509 signing certificate because

CheckX509Data() failed prevents the dialog from popping up.

To avoid confusing the user in this situation, the dialog should

show no certificate, which is already the case.

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111664

Tested-by: Jenkins

Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

(cherry picked from commit 90b725675c2964f4a151d802d9afedd8bc2ae1a7)

xmlsecurity: fix crash in DocumentDigitalSignatures::isAuthorTrusted()

If the argument is null.

This function also should use EqualDistinguishedNames().

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111667

Tested-by: Jenkins

Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

(cherry picked from commit ca98e505cd69bf95d8ddb9387cf3f8e03ae4577d)

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111910

Tested-by: Jenkins

Reviewed-by: Caolán McNamara <caolanm@redhat.com>

(cherry picked from commit a1cf770c2d7ca3e153e0b1f01ddcc313bc2bed7f)

Change-Id: I9633a980b0c18d58dfce24fc59396a833498a77d

---

 include/svl/sigstruct.hxx                     |  32 +-

 svl/source/crypto/cryptosign.cxx              |  16 +-

 sw/source/core/edit/edfcol.cxx                |   3 +-

 xmlsecurity/inc/biginteger.hxx                |   3 +

 xmlsecurity/inc/xmlsignaturehelper.hxx        |  12 +

 xmlsecurity/inc/xsecctl.hxx                   |  15 +-

 .../component/documentdigitalsignatures.cxx   |  54 +--

 .../dialogs/digitalsignaturesdialog.cxx       |  15 +-

 .../source/helper/documentsignaturehelper.cxx |  63 ++--

 .../helper/documentsignaturemanager.cxx       |  12 +

 .../source/helper/ooxmlsecexporter.cxx        |  23 +-

 xmlsecurity/source/helper/ooxmlsecparser.cxx  |  22 +-

 .../source/helper/pdfsignaturehelper.cxx      |   8 +-

 .../source/helper/xmlsignaturehelper.cxx      | 161 +++++++++

 xmlsecurity/source/helper/xsecctl.cxx         |  84 +++--

 xmlsecurity/source/helper/xsecparser.cxx      | 334 ++++++++++++------

 xmlsecurity/source/helper/xsecparser.hxx      |   1 +

 xmlsecurity/source/helper/xsecsign.cxx        |  30 +-

 xmlsecurity/source/helper/xsecverify.cxx      | 143 ++++++--

 .../mscrypt/x509certificate_mscryptimpl.cxx   |  47 +++

 .../mscrypt/xmlsignature_mscryptimpl.cxx      |   2 +

 .../xmlsec/nss/x509certificate_nssimpl.cxx    |  25 ++

 .../xmlsec/nss/xmlsignature_nssimpl.cxx       |   3 +

 23 files changed, 853 insertions(+), 255 deletions(-)

diff --git a/include/svl/sigstruct.hxx b/include/svl/sigstruct.hxx

index 7a0296fa9fae..f00cbce6e4b8 100644

--- a/include/svl/sigstruct.hxx

+++ b/include/svl/sigstruct.hxx

@@ -89,9 +89,30 @@ struct SignatureInformation

     sal_Int32 nSecurityId;

     css::xml::crypto::SecurityOperationStatus nStatus;

     SignatureReferenceInformations  vSignatureReferenceInfors;

-    OUString ouX509IssuerName;

-    OUString ouX509SerialNumber;

-    OUString ouX509Certificate;

+    struct X509CertInfo

+    {

+        OUString X509IssuerName;

+        OUString X509SerialNumber;

+        OUString X509Certificate;

+        /// OOXML certificate SHA-256 digest, empty for ODF except when doing XAdES signature.

+        OUString CertDigest;

+        /// 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;

@@ -124,8 +145,6 @@ struct SignatureInformation

     OUString ouDescription;

     /// The Id attribute of the <SignatureProperty> element that contains the <dc:description>.

     OUString ouDescriptionPropertyId;

-    /// OOXML certificate SHA-256 digest, empty for ODF except when doing XAdES signature.

-    OUString ouCertDigest;

     /// Valid and invalid signature line images

     css::uno::Reference<css::graphic::XGraphic> aValidSignatureImage;

     css::uno::Reference<css::graphic::XGraphic> aInvalidSignatureImage;

@@ -140,9 +159,6 @@ struct SignatureInformation

     /// For PDF: the byte range doesn't cover the whole document.

     bool bPartialDocumentSignature;

-    /// The certificate owner (aka subject).

-    OUString ouSubject;

-

     svl::crypto::SignatureMethodAlgorithm eAlgorithmID;

     SignatureInformation( sal_Int32 nId )

diff --git a/svl/source/crypto/cryptosign.cxx b/svl/source/crypto/cryptosign.cxx

index 5a3f0271c40d..1b882bb89deb 100644

--- a/svl/source/crypto/cryptosign.cxx

+++ b/svl/source/crypto/cryptosign.cxx

@@ -2097,8 +2097,12 @@ bool Signing::Verify(const std::vector<unsigned char>& aData,

             aDerCert[i] = pCertificate->derCert.data[i];

         OUStringBuffer aBuffer;

         comphelper::Base64::encode(aBuffer, aDerCert);

-        rInformation.ouX509Certificate = aBuffer.makeStringAndClear();

-        rInformation.ouSubject = OUString(pCertificate->subjectName, PL_strlen(pCertificate->subjectName), RTL_TEXTENCODING_UTF8);

+        SignatureInformation::X509Data temp;

+        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);

     }

     PRTime nSigningTime;

@@ -2277,8 +2281,12 @@ bool Signing::Verify(const std::vector<unsigned char>& aData,

             aDerCert[i] = pSignerCertContext->pbCertEncoded[i];

         OUStringBuffer aBuffer;

         comphelper::Base64::encode(aBuffer, aDerCert);

-        rInformation.ouX509Certificate = aBuffer.makeStringAndClear();

-        rInformation.ouSubject = GetSubjectName(pSignerCertContext);

+        SignatureInformation::X509Data temp;

+        temp.emplace_back();

+        temp.back().X509Certificate = aBuffer.makeStringAndClear();

+        temp.back().X509Subject = GetSubjectName(pSignerCertContext);

+        rInformation.X509Datas.clear();

+        rInformation.X509Datas.emplace_back(temp);

     }

     if (bNonDetached)

diff --git a/sw/source/core/edit/edfcol.cxx b/sw/source/core/edit/edfcol.cxx

index 2b49ee16ecc8..abbed5e40e94 100644

--- a/sw/source/core/edit/edfcol.cxx

+++ b/sw/source/core/edit/edfcol.cxx

@@ -411,7 +411,8 @@ std::pair<bool, OUString> lcl_MakeParagraphSignatureFieldText(const SignatureDes

             valid = valid

                     && aInfo.nStatus == xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;

-            msg = SwResId(STR_SIGNED_BY) + ": " + aInfo.ouSubject + ", " +

+            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/biginteger.hxx b/xmlsecurity/inc/biginteger.hxx

index d07ecf45d8af..8b4d8a9143b5 100644

--- a/xmlsecurity/inc/biginteger.hxx

+++ b/xmlsecurity/inc/biginteger.hxx

@@ -31,6 +31,9 @@ namespace xmlsecurity

 {

 XSECXMLSEC_DLLPUBLIC OUString bigIntegerToNumericString( const css::uno::Sequence< sal_Int8 >& serial );

 XSECXMLSEC_DLLPUBLIC css::uno::Sequence< sal_Int8 > numericStringToBigInteger ( const OUString& serialNumber );

+

+XSECXMLSEC_DLLPUBLIC bool EqualDistinguishedNames(OUString const& rName1,

+                                                  OUString const& rName2);

 }

 #endif

diff --git a/xmlsecurity/inc/xmlsignaturehelper.hxx b/xmlsecurity/inc/xmlsignaturehelper.hxx

index 0fcbd665251f..2456ddd437ec 100644

--- a/xmlsecurity/inc/xmlsignaturehelper.hxx

+++ b/xmlsecurity/inc/xmlsignaturehelper.hxx

@@ -28,6 +28,9 @@

 #include "xmlsignaturehelper.hxx"

 #include "xsecctl.hxx"

+#include <com/sun/star/security/XCertificate.hpp>

+#include <com/sun/star/xml/crypto/XSecurityEnvironment.hpp>

+

 class DateTime;

 class UriBindingHelper;

@@ -93,6 +96,15 @@ public:

                 // After signing/verifying, get information about signatures

     SignatureInformation  GetSignatureInformation( sal_Int32 nSecurityId ) const;

     SignatureInformations GetSignatureInformations() const;

+    /// ImplVerifySignature calls this to figure out which X509Data is the

+    /// signing certificate and update the internal state with the result.

+    /// @return

+    ///    A sequence with the signing certificate at the back on success.

+    ///    An empty sequence on failure.

+    std::vector<css::uno::Reference<css::security::XCertificate>>

+    CheckAndUpdateSignatureInformation(

+        css::uno::Reference<css::xml::crypto::XSecurityEnvironment> const& xSecEnv,

+        SignatureInformation const& rInfo);

                 // See XSecController for documentation

     void        StartMission(const css::uno::Reference<css::xml::crypto::XXMLSecurityContext>& xSecurityContext);

diff --git a/xmlsecurity/inc/xsecctl.hxx b/xmlsecurity/inc/xsecctl.hxx

index 7baa219fb13c..7ce35cea22bf 100644

--- a/xmlsecurity/inc/xsecctl.hxx

+++ b/xmlsecurity/inc/xsecctl.hxx

@@ -252,6 +252,7 @@ private:

     /// Sets algorithm from <SignatureMethod Algorithm="...">.

     void setSignatureMethod(svl::crypto::SignatureMethodAlgorithm eAlgorithmID);

     void switchGpgSignature();

+    bool haveReferenceForId(OUString const& rId) const;

     void addReference(

         const OUString& ouUri,

         sal_Int32 nDigestID,

@@ -262,9 +263,13 @@ private:

         sal_Int32 nDigestID );

     void setReferenceCount() const;

-    void setX509IssuerName( OUString const & ouX509IssuerName );

-    void setX509SerialNumber( OUString const & ouX509SerialNumber );

-    void setX509Certificate( OUString const & ouX509Certificate );

+    void setX509Data(

+        std::vector<std::pair<OUString, OUString>> & rX509IssuerSerials,

+        std::vector<OUString> const& rX509Certificates);

+    void setX509CertDigest(

+        OUString const& rCertDigest, sal_Int32 const nReferenceDigestID,

+        OUString const& rX509IssuerName, OUString const& rX509SerialNumber);

+

     void setSignatureValue( OUString const & ouSignatureValue );

     void setDigestValue( sal_Int32 nDigestID, OUString const & ouDigestValue );

     void setGpgKeyID( OUString const & ouKeyID );

@@ -273,7 +278,6 @@ private:

     void setDate(OUString const& rId, OUString const& ouDate);

     void setDescription(OUString const& rId, OUString const& rDescription);

-    void setCertDigest(const OUString& rCertDigest);

     void setValidSignatureImage(const OUString& rValidSigImg);

     void setInvalidSignatureImage(const OUString& rInvalidSigImg);

     void setSignatureLineId(const OUString& rSignatureLineId);

@@ -302,6 +306,9 @@ public:

     SignatureInformation    getSignatureInformation( sal_Int32 nSecurityId ) const;

     SignatureInformations   getSignatureInformations() const;

+    /// only verify can figure out which X509Data is the signing certificate

+    void UpdateSignatureInformation(sal_Int32 nSecurityId,

+            std::vector<SignatureInformation::X509Data> const& rDatas);

     static void exportSignature(

         const css::uno::Reference< css::xml::sax::XDocumentHandler >& xDocumentHandler,

diff --git a/xmlsecurity/source/component/documentdigitalsignatures.cxx b/xmlsecurity/source/component/documentdigitalsignatures.cxx

index 52cb938a8e0a..8a5bf37a0432 100644

--- a/xmlsecurity/source/component/documentdigitalsignatures.cxx

+++ b/xmlsecurity/source/component/documentdigitalsignatures.cxx

@@ -510,30 +510,36 @@ DocumentDigitalSignatures::ImplVerifySignatures(

         const SignatureInformation& rInfo = aSignInfos[n];

         css::security::DocumentSignatureInformation& rSigInfo = arInfos[n];

-        if (rInfo.ouGpgCertificate.isEmpty()) // X.509

+        if (!rInfo.X509Datas.empty()) // X.509

         {

-            if (!rInfo.ouX509Certificate.isEmpty())

-                rSigInfo.Signer = xSecEnv->createCertificateFromAscii(rInfo.ouX509Certificate);

-            if (!rSigInfo.Signer.is())

-                rSigInfo.Signer = xSecEnv->getCertificate(

-                    rInfo.ouX509IssuerName,

-                    xmlsecurity::numericStringToBigInteger(rInfo.ouX509SerialNumber));

-

-            // On Windows checking the certificate path is buggy. It does name matching (issuer, subject name)

-            // to find the parent certificate. It does not take into account that there can be several certificates

-            // with the same subject name.

-            try

+            std::vector<uno::Reference<XCertificate>> certs(

+                rSignatureHelper.CheckAndUpdateSignatureInformation(

+                    xSecEnv, rInfo));

+            if (certs.empty())

             {

-                rSigInfo.CertificateStatus = xSecEnv->verifyCertificate(

-                    rSigInfo.Signer, Sequence<Reference<css::security::XCertificate>>());

+                rSigInfo.CertificateStatus = css::security::CertificateValidity::INVALID;

             }

-            catch (SecurityException&)

+            else

             {

-                OSL_FAIL("Verification of certificate failed");

-                rSigInfo.CertificateStatus = css::security::CertificateValidity::INVALID;

+                rSigInfo.Signer = certs.back();

+                // get only intermediates

+                certs.pop_back();

+            // On Windows checking the certificate path is buggy. It does name matching (issuer, subject name)

+            // to find the parent certificate. It does not take into account that there can be several certificates

+            // with the same subject name.

+                try

+                {

+                    rSigInfo.CertificateStatus = xSecEnv->verifyCertificate(

+                        rSigInfo.Signer, comphelper::containerToSequence(certs));

+                }

+                catch (SecurityException&)

+                {

+                    SAL_WARN("xmlsecurity.comp", "Verification of certificate failed");

+                    rSigInfo.CertificateStatus = css::security::CertificateValidity::INVALID;

+                }

             }

         }

-        else if (xGpgSecEnv.is()) // GPG

+        else if (!rInfo.ouGpgCertificate.isEmpty() && xGpgSecEnv.is()) // GPG

         {

             // TODO not ideal to retrieve cert by keyID, might

             // collide, or PGPKeyID format might change - can't we

@@ -619,15 +625,19 @@ void DocumentDigitalSignatures::showCertificate(

 }

 sal_Bool DocumentDigitalSignatures::isAuthorTrusted(

-    const Reference< css::security::XCertificate >& Author )

+    const Reference<css::security::XCertificate>& xAuthor)

 {

-    OUString sSerialNum = xmlsecurity::bigIntegerToNumericString( Author->getSerialNumber() );

+    if (!xAuthor.is())

+    {

+        return false;

+    }

+    OUString sSerialNum = xmlsecurity::bigIntegerToNumericString(xAuthor->getSerialNumber());

     Sequence< SvtSecurityOptions::Certificate > aTrustedAuthors = SvtSecurityOptions().GetTrustedAuthors();

     return std::any_of(aTrustedAuthors.begin(), aTrustedAuthors.end(),

-        [&Author, &sSerialNum](const SvtSecurityOptions::Certificate& rAuthor) {

-            return ( rAuthor[0] == Author->getIssuerName() )

+        [&xAuthor, &sSerialNum](const SvtSecurityOptions::Certificate& rAuthor) {

+            return xmlsecurity::EqualDistinguishedNames(rAuthor[0], xAuthor->getIssuerName())

                 && ( rAuthor[1] == sSerialNum );

         });

 }

diff --git a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx

index ef67c7167c04..18ccaf2d2166 100644

--- a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx

+++ b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx

@@ -588,7 +588,7 @@ void DigitalSignaturesDialog::ImplFillSignaturesBox()

                 if (!rInfo.ouGpgCertificate.isEmpty())

                     aType = "OpenPGP";

                 // XML based: XAdES or not.

-                else if (!rInfo.ouCertDigest.isEmpty())

+                else if (rInfo.GetSigningCertificate() && !rInfo.GetSigningCertificate()->CertDigest.isEmpty())

                     aType = "XAdES";

                 else

                     aType = "XML-DSig";

@@ -700,8 +700,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.ouX509Certificate.isEmpty())

-        xCert = xSecEnv->createCertificateFromAscii(rInfo.ouX509Certificate);

+    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

@@ -713,9 +713,12 @@ 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())

-        xCert = xSecEnv->getCertificate( rInfo.ouX509IssuerName, xmlsecurity::numericStringToBigInteger( rInfo.ouX509SerialNumber ) );

-    if (!xCert.is() && xGpgSecEnv.is())

+    if (!xCert.is() && xSecEnv.is() && rInfo.GetSigningCertificate())

+    {

+        xCert = xSecEnv->getCertificate(rInfo.GetSigningCertificate()->X509IssuerName,

+            xmlsecurity::numericStringToBigInteger(rInfo.GetSigningCertificate()->X509SerialNumber));

+    }

+    if (!xCert.is() && xGpgSecEnv.is() && !rInfo.ouGpgKeyID.isEmpty())

         xCert = xGpgSecEnv->getCertificate( rInfo.ouGpgKeyID, xmlsecurity::numericStringToBigInteger("") );

     SAL_WARN_IF( !xCert.is(), "xmlsecurity.dialogs", "Certificate not found and can't be created!" );

diff --git a/xmlsecurity/source/helper/documentsignaturehelper.cxx b/xmlsecurity/source/helper/documentsignaturehelper.cxx

index 482ae6cc4126..ddff308ee52f 100644

--- a/xmlsecurity/source/helper/documentsignaturehelper.cxx

+++ b/xmlsecurity/source/helper/documentsignaturehelper.cxx

@@ -492,6 +492,29 @@ void DocumentSignatureHelper::writeDigestMethod(

     xDocumentHandler->endElement("DigestMethod");

 }

+static void WriteXadesCert(

+    uno::Reference<xml::sax::XDocumentHandler> const& xDocumentHandler,

+    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(!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(rCertInfo.X509IssuerName);

+    xDocumentHandler->endElement("X509IssuerName");

+    xDocumentHandler->startElement("X509SerialNumber", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

+    xDocumentHandler->characters(rCertInfo.X509SerialNumber);

+    xDocumentHandler->endElement("X509SerialNumber");

+    xDocumentHandler->endElement("xd:IssuerSerial");

+    xDocumentHandler->endElement("xd:Cert");

+}

+

 void DocumentSignatureHelper::writeSignedProperties(

     const uno::Reference<xml::sax::XDocumentHandler>& xDocumentHandler,

     const SignatureInformation& signatureInfo,

@@ -508,26 +531,26 @@ void DocumentSignatureHelper::writeSignedProperties(

     xDocumentHandler->characters(sDate);

     xDocumentHandler->endElement("xd:SigningTime");

     xDocumentHandler->startElement("xd:SigningCertificate", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

-    xDocumentHandler->startElement("xd:Cert", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

-    xDocumentHandler->startElement("xd:CertDigest", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

-    writeDigestMethod(xDocumentHandler);

-

-    xDocumentHandler->startElement("DigestValue", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

-    // TODO: this is empty for gpg signatures currently

-    //assert(!signatureInfo.ouCertDigest.isEmpty());

-    xDocumentHandler->characters(signatureInfo.ouCertDigest);

-    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(signatureInfo.ouX509IssuerName);

-    xDocumentHandler->endElement("X509IssuerName");

-    xDocumentHandler->startElement("X509SerialNumber", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

-    xDocumentHandler->characters(signatureInfo.ouX509SerialNumber);

-    xDocumentHandler->endElement("X509SerialNumber");

-    xDocumentHandler->endElement("xd:IssuerSerial");

-    xDocumentHandler->endElement("xd:Cert");

+    assert(signatureInfo.GetSigningCertificate() || !signatureInfo.ouGpgKeyID.isEmpty());

+    if (signatureInfo.GetSigningCertificate())

+    {

+        // how should this deal with multiple X509Data elements?

+        // for now, let's write all of the certificates ...

+        for (auto const& rData : signatureInfo.X509Datas)

+        {

+            for (auto const& it : rData)

+            {

+                WriteXadesCert(xDocumentHandler, it);

+            }

+        }

+    }

+    else

+    {

+        // for PGP, write empty mandatory X509IssuerName, X509SerialNumber

+        SignatureInformation::X509CertInfo temp;

+        temp.CertDigest = signatureInfo.ouGpgKeyID;

+        WriteXadesCert(xDocumentHandler, temp);

+    }

     xDocumentHandler->endElement("xd:SigningCertificate");

     xDocumentHandler->startElement("xd:SignaturePolicyIdentifier", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

     xDocumentHandler->startElement("xd:SignaturePolicyImplied", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

diff --git a/xmlsecurity/source/helper/documentsignaturemanager.cxx b/xmlsecurity/source/helper/documentsignaturemanager.cxx

index a0e674c3bd1b..aa08dc5c499e 100644

--- a/xmlsecurity/source/helper/documentsignaturemanager.cxx

+++ b/xmlsecurity/source/helper/documentsignaturemanager.cxx

@@ -585,6 +585,18 @@ void DocumentSignatureManager::read(bool bUseTempStream, bool bCacheLastSignatur

                                                             bCacheLastSignature);

         maSignatureHelper.EndMission();

+        // this parses the XML independently from ImplVerifySignatures() - check

+        // certificates here too ...

+        for (auto const& it : maSignatureHelper.GetSignatureInformations())

+        {

+            if (!it.X509Datas.empty())

+            {

+                uno::Reference<xml::crypto::XSecurityEnvironment> const xSecEnv(

+                    getSecurityEnvironment());

+                getSignatureHelper().CheckAndUpdateSignatureInformation(xSecEnv, it);

+            }

+        }

+

         maCurrentSignatureInformations = maSignatureHelper.GetSignatureInformations();

     }

     else

diff --git a/xmlsecurity/source/helper/ooxmlsecexporter.cxx b/xmlsecurity/source/helper/ooxmlsecexporter.cxx

index cf87d6e1ad17..cae2ef3f5b16 100644

--- a/xmlsecurity/source/helper/ooxmlsecexporter.cxx

+++ b/xmlsecurity/source/helper/ooxmlsecexporter.cxx

@@ -194,12 +194,23 @@ void OOXMLSecExporter::Impl::writeSignatureValue()

 void OOXMLSecExporter::Impl::writeKeyInfo()

 {

-    m_xDocumentHandler->startElement("KeyInfo", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

-    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(m_rInformation.ouX509Certificate);

-    m_xDocumentHandler->endElement("X509Certificate");

-    m_xDocumentHandler->endElement("X509Data");

+    m_xDocumentHandler->startElement(

+        "KeyInfo", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

+    assert(m_rInformation.GetSigningCertificate());

+    for (auto const& rData : m_rInformation.X509Datas)

+    {

+        m_xDocumentHandler->startElement(

+            "X509Data", uno::Reference<xml::sax::XAttributeList>(new SvXMLAttributeList()));

+        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 a200de60c07a..a25872fc057d 100644

--- a/xmlsecurity/source/helper/ooxmlsecparser.cxx

+++ b/xmlsecurity/source/helper/ooxmlsecparser.cxx

@@ -185,9 +185,22 @@ void SAL_CALL OOXMLSecParser::endElement(const OUString& rName)

         m_pXSecController->setSignatureValue(m_aSignatureValue);

         m_bInSignatureValue = false;

     }

+    else if (rName == "X509Data")

+    {

+        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")

     {

-        m_pXSecController->setX509Certificate(m_aX509Certificate);

         m_bInX509Certificate = false;

     }

     else if (rName == "mdssi:Value")

@@ -202,17 +215,18 @@ void SAL_CALL OOXMLSecParser::endElement(const OUString& rName)

     }

     else if (rName == "X509IssuerName")

     {

-        m_pXSecController->setX509IssuerName(m_aX509IssuerName);

         m_bInX509IssuerName = false;

     }

     else if (rName == "X509SerialNumber")

     {

-        m_pXSecController->setX509SerialNumber(m_aX509SerialNumber);

         m_bInX509SerialNumber = false;

     }

+    else if (rName == "xd:Cert")

+    {

+        m_pXSecController->setX509CertDigest(m_aCertDigest, css::xml::crypto::DigestID::SHA1, m_aX509IssuerName, m_aX509SerialNumber);

+    }

     else if (rName == "xd:CertDigest")

     {

-        m_pXSecController->setCertDigest(m_aCertDigest);

         m_bInCertDigest = false;

     }

     else if (rName == "Object")

diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx

index f10f29c61840..8a90b73901db 100644

--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx

+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx

@@ -82,8 +82,12 @@ PDFSignatureHelper::GetDocumentSignatureInformations(

         security::DocumentSignatureInformation& rExternal = aRet[i];

         rExternal.SignatureIsValid

             = rInternal.nStatus == xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;

-        if (!rInternal.ouX509Certificate.isEmpty())

-            rExternal.Signer = xSecEnv->createCertificateFromAscii(rInternal.ouX509Certificate);

+        if (rInternal.GetSigningCertificate()

+            && !rInternal.GetSigningCertificate()->X509Certificate.isEmpty())

+        {

+            rExternal.Signer = xSecEnv->createCertificateFromAscii(

+                rInternal.GetSigningCertificate()->X509Certificate);

+        }

         rExternal.PartialDocumentSignature = rInternal.bPartialDocumentSignature;

         // Verify certificate.

diff --git a/xmlsecurity/source/helper/xmlsignaturehelper.cxx b/xmlsecurity/source/helper/xmlsignaturehelper.cxx

index 22c056e70da1..bcb79039e342 100644

--- a/xmlsecurity/source/helper/xmlsignaturehelper.cxx

+++ b/xmlsecurity/source/helper/xmlsignaturehelper.cxx

@@ -21,6 +21,7 @@

 #include <xmlsignaturehelper.hxx>

 #include <documentsignaturehelper.hxx>

 #include <xsecctl.hxx>

+#include <biginteger.hxx>

 #include <xmlsignaturehelper2.hxx>

@@ -45,6 +46,8 @@

 #include <tools/diagnose_ex.h>

 #include <sal/log.hxx>

+#include <optional>

+

 #define NS_DOCUMENTSIGNATURES   "http://openoffice.org/2004/documentsignatures"

 #define NS_DOCUMENTSIGNATURES_ODF_1_2 "urn:oasis:names:tc:opendocument:xmlns:digitalsignature:1.0"

 #define OOXML_SIGNATURE_ORIGIN "http://schemas.openxmlformats.org/package/2006/relationships/digital-signature/origin"

@@ -547,4 +550,162 @@ void XMLSignatureHelper::CreateAndWriteOOXMLSignature(const uno::Reference

     xSaxWriter->endDocument();

 }

+/** check this constraint from xmldsig-core 4.5.4:

+

+  All certificates appearing in an X509Data element MUST relate to the

+  validation key by either containing it or being part of a certification

+  chain that terminates in a certificate containing the validation key.

+ */

+static auto CheckX509Data(

+    uno::Reference<xml::crypto::XSecurityEnvironment> const& xSecEnv,

+    std::vector<SignatureInformation::X509CertInfo> const& rX509CertInfos,

+    std::vector<uno::Reference<security::XCertificate>> & rCerts,

+    std::vector<SignatureInformation::X509CertInfo> & rSorted) -> bool

+{

+    assert(rCerts.empty());

+    assert(rSorted.empty());

+    if (rX509CertInfos.empty())

+    {

+        SAL_WARN("xmlsecurity.comp", "no X509Data");

+        return false;

+    }

+    std::vector<uno::Reference<security::XCertificate>> certs;

+    for (SignatureInformation::X509CertInfo const& it : rX509CertInfos)

+    {

+        if (!it.X509Certificate.isEmpty())

+        {

+            certs.emplace_back(xSecEnv->createCertificateFromAscii(it.X509Certificate));

+        }

+        else

+        {

+            certs.emplace_back(xSecEnv->getCertificate(

+                it.X509IssuerName,

+                xmlsecurity::numericStringToBigInteger(it.X509SerialNumber)));

+        }

+        if (!certs.back().is())

+        {

+            SAL_WARN("xmlsecurity.comp", "X509Data cannot be parsed");

+            return false;

+        }

+    }