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)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113058
Tested-by: Michael Stahl <michael.stahl@allotropia.de>
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

Change-Id: I9633a980b0c18d58dfce24fc59396a833498a77d

1 files changed, 73 insertions, 71 deletions

diff --git a/xmlsecurity/source/helper/xsecparser.cxx b/xmlsecurity/source/helper/xsecparser.cxx
index 2ce46b1e18f1..1c8bf28ab287 100644
--- a/xmlsecurity/source/helper/xsecparser.cxx
+++ b/xmlsecurity/source/helper/xsecparser.cxx
@@ -244,98 +244,79 @@ class XSecParser::DsX509CertificateContext
     : public XSecParser::Context
 {
     private:
-        OUString m_Value;
+        OUString & m_rValue;
 
     public:
         DsX509CertificateContext(XSecParser & rParser,
-                std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap)
+                std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
+                OUString & rValue)
             : XSecParser::Context(rParser, std::move(pOldNamespaceMap))
+            , m_rValue(rValue)
         {
         }
 
-        virtual void EndElement() override
-        {
-            m_rParser.m_pXSecController->setX509Certificate(m_Value);
-        }
-
         virtual void Characters(OUString const& rChars) override
         {
-            m_Value += rChars;
+            m_rValue += rChars;
         }
 };
 
 class XSecParser::DsX509SerialNumberContext
-    : public XSecParser::ReferencedContextImpl
+    : public XSecParser::Context
 {
     private:
-        OUString m_Value;
+        OUString & m_rValue;
 
     public:
         DsX509SerialNumberContext(XSecParser & rParser,
                 std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
-                bool const isReferenced)
-            : ReferencedContextImpl(rParser, std::move(pOldNamespaceMap), isReferenced)
-        {
-        }
-
-        virtual void EndElement() override
+                OUString & rValue)
+            : XSecParser::Context(rParser, std::move(pOldNamespaceMap))
+            , m_rValue(rValue)
         {
-            if (m_isReferenced)
-            {
-                m_rParser.m_pXSecController->setX509SerialNumber(m_Value);
-            }
-            else
-            {
-                SAL_INFO("xmlsecurity.helper", "ignoring unsigned X509SerialNumber");
-            }
         }
 
         virtual void Characters(OUString const& rChars) override
         {
-            m_Value += rChars;
+            m_rValue += rChars;
         }
 };
 
 class XSecParser::DsX509IssuerNameContext
-    : public XSecParser::ReferencedContextImpl
+    : public XSecParser::Context
 {
     private:
-        OUString m_Value;
+        OUString & m_rValue;
 
     public:
         DsX509IssuerNameContext(XSecParser & rParser,
                 std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
-                bool const isReferenced)
-            : ReferencedContextImpl(rParser, std::move(pOldNamespaceMap), isReferenced)
-        {
-        }
-
-        virtual void EndElement() override
+                OUString & rValue)
+            : XSecParser::Context(rParser, std::move(pOldNamespaceMap))
+            , m_rValue(rValue)
         {
-            if (m_isReferenced)
-            {
-                m_rParser.m_pXSecController->setX509IssuerName(m_Value);
-            }
-            else
-            {
-                SAL_INFO("xmlsecurity.helper", "ignoring unsigned X509IssuerName");
-            }
         }
 
         virtual void Characters(OUString const& rChars) override
         {
-            m_Value += rChars;
+            m_rValue += rChars;
         }
 };
 
 class XSecParser::DsX509IssuerSerialContext
-    : public XSecParser::ReferencedContextImpl
+    : public XSecParser::Context
 {
+    private:
+        OUString & m_rX509IssuerName;
+        OUString & m_rX509SerialNumber;
+
     public:
         DsX509IssuerSerialContext(XSecParser & rParser,
                 std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
-                bool const isReferenced)
-            : ReferencedContextImpl(rParser, std::move(pOldNamespaceMap), isReferenced)
+                OUString & rIssuerName, OUString & rSerialNumber)
+            : XSecParser::Context(rParser, std::move(pOldNamespaceMap))
+            , m_rX509IssuerName(rIssuerName)
+            , m_rX509SerialNumber(rSerialNumber)
         {
         }
 
@@ -345,20 +326,27 @@ class XSecParser::DsX509IssuerSerialContext
         {
             if (nNamespace == XML_NAMESPACE_DS && rName == "X509IssuerName")
             {
-                return std::make_unique<DsX509IssuerNameContext>(m_rParser, std::move(pOldNamespaceMap), m_isReferenced);
+                return std::make_unique<DsX509IssuerNameContext>(m_rParser, std::move(pOldNamespaceMap), m_rX509IssuerName);
             }
             if (nNamespace == XML_NAMESPACE_DS && rName == "X509SerialNumber")
             {
-                return std::make_unique<DsX509SerialNumberContext>(m_rParser, std::move(pOldNamespaceMap), m_isReferenced);
+                return std::make_unique<DsX509SerialNumberContext>(m_rParser, std::move(pOldNamespaceMap), m_rX509SerialNumber);
             }
             // missing: ds:X509SKI, ds:X509SubjectName, ds:X509CRL
             return XSecParser::Context::CreateChildContext(std::move(pOldNamespaceMap), nNamespace, rName);
         }
 };
 
+/// can't be sure what is supposed to happen here because the spec is clear as mud
 class XSecParser::DsX509DataContext
     : public XSecParser::Context
 {
+    private:
+        // 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,
                 std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap)
@@ -366,18 +354,24 @@ class XSecParser::DsX509DataContext
         {
         }
 
+        virtual void EndElement() override
+        {
+            m_rParser.m_pXSecController->setX509Data(m_X509IssuerSerials, m_X509Certificates);
+        }
+
         virtual std::unique_ptr<Context> CreateChildContext(
             std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
             sal_uInt16 const nNamespace, OUString const& rName) override
         {
             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), true);
+                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_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);
@@ -969,30 +963,20 @@ class XSecParser::LoSignatureLineContext
 };
 
 class XSecParser::XadesCertDigestContext
-    : public XSecParser::ReferencedContextImpl
+    : public XSecParser::Context
 {
     private:
-        OUString m_Value;
-        sal_Int32 m_nReferenceDigestID = css::xml::crypto::DigestID::SHA1;
+        OUString & m_rDigestValue;
+        sal_Int32 & m_rReferenceDigestID;
 
     public:
         XadesCertDigestContext(XSecParser & rParser,
                 std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
-                bool const isReferenced)
-            : ReferencedContextImpl(rParser, std::move(pOldNamespaceMap), isReferenced)
-        {
-        }
-
-        virtual void EndElement() override
+                OUString & rDigestValue, sal_Int32 & rReferenceDigestID)
+            : XSecParser::Context(rParser, std::move(pOldNamespaceMap))
+            , m_rDigestValue(rDigestValue)
+            , m_rReferenceDigestID(rReferenceDigestID)
         {
-            if (m_isReferenced)
-            {
-                m_rParser.m_pXSecController->setCertDigest(m_Value/* FIXME , m_nReferenceDigestID*/);
-            }
-            else
-            {
-                SAL_INFO("xmlsecurity.helper", "ignoring unsigned CertDigest");
-            }
         }
 
         virtual std::unique_ptr<Context> CreateChildContext(
@@ -1001,11 +985,11 @@ class XSecParser::XadesCertDigestContext
         {
             if (nNamespace == XML_NAMESPACE_DS && rName == "DigestMethod")
             {
-                return std::make_unique<DsDigestMethodContext>(m_rParser, std::move(pOldNamespaceMap), m_nReferenceDigestID);
+                return std::make_unique<DsDigestMethodContext>(m_rParser, std::move(pOldNamespaceMap), m_rReferenceDigestID);
             }
             if (nNamespace == XML_NAMESPACE_DS && rName == "DigestValue")
             {
-                return std::make_unique<DsDigestValueContext>(m_rParser, std::move(pOldNamespaceMap), m_Value);
+                return std::make_unique<DsDigestValueContext>(m_rParser, std::move(pOldNamespaceMap), m_rDigestValue);
             }
             return XSecParser::Context::CreateChildContext(std::move(pOldNamespaceMap), nNamespace, rName);
         }
@@ -1014,6 +998,12 @@ class XSecParser::XadesCertDigestContext
 class XSecParser::XadesCertContext
     : public XSecParser::ReferencedContextImpl
 {
+    private:
+        sal_Int32 m_nReferenceDigestID = css::xml::crypto::DigestID::SHA1;
+        OUString m_CertDigest;
+        OUString m_X509IssuerName;
+        OUString m_X509SerialNumber;
+
     public:
         XadesCertContext(XSecParser & rParser,
                 std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
@@ -1022,17 +1012,29 @@ class XSecParser::XadesCertContext
         {
         }
 
+        virtual void EndElement() override
+        {
+            if (m_isReferenced)
+            {
+                m_rParser.m_pXSecController->setX509CertDigest(m_CertDigest, m_nReferenceDigestID, m_X509IssuerName, m_X509SerialNumber);
+            }
+            else
+            {
+                SAL_INFO("xmlsecurity.helper", "ignoring unsigned xades:Cert");
+            }
+        }
+
         virtual std::unique_ptr<Context> CreateChildContext(
             std::unique_ptr<SvXMLNamespaceMap> pOldNamespaceMap,
             sal_uInt16 const nNamespace, OUString const& rName) override
         {
             if (nNamespace == XML_NAMESPACE_XADES132 && rName == "CertDigest")
             {
-                return std::make_unique<XadesCertDigestContext>(m_rParser, std::move(pOldNamespaceMap), m_isReferenced);
+                return std::make_unique<XadesCertDigestContext>(m_rParser, std::move(pOldNamespaceMap), m_CertDigest, m_nReferenceDigestID);
             }
             if (nNamespace == XML_NAMESPACE_XADES132 && rName == "IssuerSerial")
             {
-                return std::make_unique<DsX509IssuerSerialContext>(m_rParser, std::move(pOldNamespaceMap), m_isReferenced);
+                return std::make_unique<DsX509IssuerSerialContext>(m_rParser, std::move(pOldNamespaceMap), m_X509IssuerName, m_X509SerialNumber);
             }
             return XSecParser::Context::CreateChildContext(std::move(pOldNamespaceMap), nNamespace, rName);
         }