From bee22fd4333408f341ee377fe52f0b1b6dbf76a0 Mon Sep 17 00:00:00 2001
From: Mike Kaganski <mike.kaganski@collabora.com>
Date: Fri, 29 Jul 2022 15:14:43 +0300
Subject: [PATCH] Verify signing certificate cryptographically before use in
 tests

Documents signed using an invalid (e.g., not having a trusted root)
certificate give signatures that obviously can't pass validity tests.

Change-Id: Id4b097516e06c548ea42cad65d76bbd8a6853cc4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137620
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
---
 filter/qa/pdf.cxx                       |  3 ++-
 include/unotest/macros_test.hxx         |  8 +++++++-
 unotest/source/cpp/macros_test.cxx      | 18 ++++++++++++++----
 vcl/qa/cppunit/filter/ipdf/ipdf.cxx     |  4 ++--
 xmlsecurity/qa/unit/signing/signing.cxx |  3 ++-
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/filter/qa/pdf.cxx b/filter/qa/pdf.cxx
index 7cb713fefce1..3500bb5dae28 100644
--- a/filter/qa/pdf.cxx
+++ b/filter/qa/pdf.cxx
@@ -72,7 +72,8 @@ CPPUNIT_TEST_FIXTURE(Test, testSignCertificateSubjectName)
             OUString(
                 "CN=Xmlsecurity RSA Test example Alice,O=Xmlsecurity RSA Test,ST=England,C=UK")),
     };
-    if (!GetValidCertificate(xSecurityEnvironment->getPersonalCertificates(), aFilterData))
+    if (!GetValidCertificate(xSecurityEnvironment->getPersonalCertificates(), xSecurityEnvironment,
+                             aFilterData))
     {
         return;
     }
diff --git a/include/unotest/macros_test.hxx b/include/unotest/macros_test.hxx
index dc5ca20dd23d..85a99789ccdb 100644
--- a/include/unotest/macros_test.hxx
+++ b/include/unotest/macros_test.hxx
@@ -43,6 +43,10 @@ namespace com::sun::star::security
 {
 class XCertificate;
 }
+namespace com::sun::star::xml::crypto
+{
+class XSecurityEnvironment;
+}
 
 namespace unotest
 {
@@ -93,9 +97,11 @@ public:
     void setUpNssGpg(const test::Directories& rDirectories, const OUString& rTestName);
     void tearDownNssGpg();
 
-    static bool IsValid(const css::uno::Reference<css::security::XCertificate>& cert);
+    static bool IsValid(const css::uno::Reference<css::security::XCertificate>& cert,
+                        const css::uno::Reference<css::xml::crypto::XSecurityEnvironment>& env);
     static css::uno::Reference<css::security::XCertificate> GetValidCertificate(
         const css::uno::Sequence<css::uno::Reference<css::security::XCertificate>>& certs,
+        const css::uno::Reference<css::xml::crypto::XSecurityEnvironment>& env,
         const css::uno::Sequence<css::beans::PropertyValue>& rFilterData = {});
 
 protected:
diff --git a/unotest/source/cpp/macros_test.cxx b/unotest/source/cpp/macros_test.cxx
index 76105b88b1a7..594ff353895a 100644
--- a/unotest/source/cpp/macros_test.cxx
+++ b/unotest/source/cpp/macros_test.cxx
@@ -15,7 +15,9 @@
 #include <com/sun/star/uno/XComponentContext.hpp>
 #include <com/sun/star/frame/DispatchHelper.hpp>
 #include <com/sun/star/packages/zip/ZipFileAccess.hpp>
+#include <com/sun/star/security/CertificateValidity.hpp>
 #include <com/sun/star/security/XCertificate.hpp>
+#include <com/sun/star/xml/crypto/XSecurityEnvironment.hpp>
 
 #include <basic/basrdll.hxx>
 #include <cppunit/TestAssert.h>
@@ -179,8 +181,11 @@ struct Valid
 {
     DateTime now;
     OUString subjectName;
-    Valid(const css::uno::Sequence<css::beans::PropertyValue>& rFilterData)
+    const css::uno::Reference<css::xml::crypto::XSecurityEnvironment>& env;
+    Valid(const css::uno::Sequence<css::beans::PropertyValue>& rFilterData,
+          const css::uno::Reference<css::xml::crypto::XSecurityEnvironment>& rEnv)
         : now(DateTime::SYSTEM)
+        , env(rEnv)
     {
         for (const auto& propVal : rFilterData)
         {
@@ -194,22 +199,27 @@ struct Valid
             return false;
         if (!subjectName.isEmpty() && subjectName != cert->getSubjectName())
             return false;
+        if (env->verifyCertificate(cert, {}) != css::security::CertificateValidity::VALID)
+            return false;
         return true;
     }
 };
 }
 
-bool MacrosTest::IsValid(const css::uno::Reference<css::security::XCertificate>& cert)
+bool MacrosTest::IsValid(const css::uno::Reference<css::security::XCertificate>& cert,
+                         const css::uno::Reference<css::xml::crypto::XSecurityEnvironment>& env)
 {
-    const Valid test({});
+    const Valid test({}, env);
     return test(cert);
 }
 
 css::uno::Reference<css::security::XCertificate> MacrosTest::GetValidCertificate(
     const css::uno::Sequence<css::uno::Reference<css::security::XCertificate>>& certs,
+    const css::uno::Reference<css::xml::crypto::XSecurityEnvironment>& env,
     const css::uno::Sequence<css::beans::PropertyValue>& rFilterData)
 {
-    if (auto it = std::find_if(certs.begin(), certs.end(), Valid(rFilterData)); it != certs.end())
+    if (auto it = std::find_if(certs.begin(), certs.end(), Valid(rFilterData, env));
+        it != certs.end())
         return *it;
     return {};
 }
diff --git a/vcl/qa/cppunit/filter/ipdf/ipdf.cxx b/vcl/qa/cppunit/filter/ipdf/ipdf.cxx
index 620b892fdba8..3beedaad0dc0 100644
--- a/vcl/qa/cppunit/filter/ipdf/ipdf.cxx
+++ b/vcl/qa/cppunit/filter/ipdf/ipdf.cxx
@@ -114,8 +114,8 @@ CPPUNIT_TEST_FIXTURE(VclFilterIpdfTest, testPDFAddVisibleSignatureLastPage)
     uno::Reference<view::XSelectionSupplier> xSelectionSupplier(pBaseModel->getCurrentController(),
                                                                 uno::UNO_QUERY);
     xSelectionSupplier->select(uno::Any(xShape));
-    auto xCert = GetValidCertificate(
-        getSecurityContext()->getSecurityEnvironment()->getPersonalCertificates());
+    auto xEnv = getSecurityContext()->getSecurityEnvironment();
+    auto xCert = GetValidCertificate(xEnv->getPersonalCertificates(), xEnv);
     if (!xCert)
     {
         return;
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index c3c5d254b335..48fc42091e02 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -159,7 +159,8 @@ SigningTest::getCertificate(DocumentSignatureManager& rSignatureManager,
     {
         auto pCertificate = dynamic_cast<xmlsecurity::Certificate*>(xCertificate.get());
         CPPUNIT_ASSERT(pCertificate);
-        if (pCertificate->getSignatureMethodAlgorithm() == eAlgo && IsValid(xCertificate))
+        if (pCertificate->getSignatureMethodAlgorithm() == eAlgo
+            && IsValid(xCertificate, xSecurityEnvironment))
             return xCertificate;
     }
     return uno::Reference<security::XCertificate>();