xmlsecurity: fix new tests on WNT

Tests added in commit 40d70d427edddb589eda64fafc2e56536953d274 don't
actually run on WNT but that wasn't obvious because commit
149df1fec6472e30582162e17e04c75aee91d26a prevented running them in
Jenkins on master, they failed only in the libreoffice-7-1 backport.

xmlsecurity/qa/unit/signing/signing.cxx(631) : error : Assertion
Test name: testODFDoubleX509Certificate::TestBody
assertion failed
- Expression: (nActual == SignatureState::NOTVALIDATED || nActual == SignatureState::OK)
- 2

This is an oddity where NSS claims the signature in the document is
valid but CryptoAPI claims it is invalid; the hashes passed into the
validation functions are the same.  Just allow BROKEN as an additional
result value on WNT.

xmlsecurity/qa/unit/signing/signing.cxx(550) : error : Assertion
Test name: testODFX509CertificateChain::TestBody
equality assertion failed
- Expected: 0
- Actual  : 1

The problem here is that with NSS the tests use a custom NSS database
in test/signing-keys so we need to make these certificates available for
CryptoAPI too.

The following one-liner converts the NSS database to a PKCS#7 that can
be loaded by CrytpAPI:

> openssl crl2pkcs7 -nocrl -certfile <(certutil -d sql:test/signing-keys -L | awk '/^[^ ].*,[^ ]*,/ { printf "%s", $1; for (i = 2; i < NF; i++) { printf " %s", $i; } printf "\n"; }' | while read name; do certutil -L -d sql:test/signing-keys -a -n "${name}" ; done) > test/signing-keys/test.p7b

Then one might naively assume that something like this would allow these
certificates to be added temporarily as trusted CAs:

+                HCERTSTORE hRoot = CertOpenSystemStoreW( 0, L"Root" ) ;
+                HCERTSTORE const hExtra = CertOpenStore(
+                        CERT_STORE_PROV_FILENAME_A,
+                        PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
+                        NULL,
+                        CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG,
+                        path);
+                if (hExtra != NULL && hRoot != NULL)
+                {
+                    BOOL ret = CertAddStoreToCollection(
+                        hRoot,
+                        hExtra,
+                        CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG,
+                        0);
+                    SAL_DEBUG("XXX hExtra done " << ret);
+                }

There is no error from this, but it doesn't work.

Instead, check if CertGetCertificateChain() sets the
CERT_TRUST_IS_UNTRUSTED_ROOT flag and then look up the certificate
manually in the extra PKCS#7 store.

Change-Id: Ic9865e0b5783211c2128ce0327c4583b7784ff62
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123667
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

2 files changed, 72 insertions, 5 deletions

diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index 298ee6d38f38..32dba85f951b 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -552,7 +552,8 @@ CPPUNIT_TEST_FIXTURE(SigningTest, testODFX509CertificateChain)
     CPPUNIT_ASSERT(infos[0].Signer.is());
     CPPUNIT_ASSERT_EQUAL(
         OUString("CN=Xmlsecurity RSA Test example Alice,O=Xmlsecurity RSA Test,ST=England,C=UK"),
-        infos[0].Signer->getSubjectName());
+        // CryptoAPI puts a space after comma, NSS does not...
+        infos[0].Signer->getSubjectName().replaceAll(", ", ","));
 }
 
 CPPUNIT_TEST_FIXTURE(SigningTest, testODFDoubleX509Data)
@@ -627,9 +628,15 @@ CPPUNIT_TEST_FIXTURE(SigningTest, testODFDoubleX509Certificate)
     SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell();
     CPPUNIT_ASSERT(pObjectShell);
     SignatureState nActual = pObjectShell->GetDocumentSignatureState();
-    CPPUNIT_ASSERT_MESSAGE(
-        (OString::number(o3tl::underlyingEnumValue(nActual)).getStr()),
-        (nActual == SignatureState::NOTVALIDATED || nActual == SignatureState::OK));
+    bool const nTemp((nActual == SignatureState::NOTVALIDATED
+                      || nActual == SignatureState::OK
+#if defined(_WIN32)
+                      // oddly BCryptVerifySignature returns STATUS_INVALID_SIGNATURE
+                      // while the same succeeds with NSS _SGN_VerifyPKCS1DigestInfo
+                      || nActual == SignatureState::BROKEN
+#endif
+                      ));
+    CPPUNIT_ASSERT_MESSAGE((OString::number(o3tl::underlyingEnumValue(nActual)).getStr()), nTemp);
     uno::Sequence<security::DocumentSignatureInformation> const infos(
         pObjectShell->GetDocumentSignatureInformation(false));
     CPPUNIT_ASSERT_EQUAL(sal_Int32(1), infos.getLength());
diff --git a/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx
index 3d298384549d..70617264e8fd 100644
--- a/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx
+++ b/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx
@@ -738,6 +738,60 @@ static HCERTSTORE getCertStoreForIntermediatCerts(
     return store;
 }
 
+static bool CheckUnitTestStore(PCCERT_CHAIN_CONTEXT const pChainContext, DWORD ignoreFlags)
+{
+    bool ret = false;
+    static char const*const pVar = getenv("LIBO_TEST_CRYPTOAPI_PKCS7");
+    if (!pVar)
+    {
+        return ret;
+    }
+    if (pChainContext->cChain == 0)
+    {
+        return ret;
+    }
+    PCERT_SIMPLE_CHAIN pSimpleChain = pChainContext->rgpChain[0];
+    // check if untrusted root is the only problem
+    if (pSimpleChain->TrustStatus.dwErrorStatus & ~(CERT_TRUST_IS_UNTRUSTED_ROOT | ignoreFlags))
+    {
+        return ret;
+    }
+
+    // leak this store, re-opening is a waste of time in tests
+    static HCERTSTORE const hExtra = CertOpenStore(
+            CERT_STORE_PROV_FILENAME_A,
+            PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
+            NULL,
+            CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG,
+            OString(OString::Concat(pVar) + "/test.p7b").getStr());
+    assert(hExtra != NULL);
+    if (pSimpleChain->cElement < 1)
+    {
+        SAL_WARN("xmlsecurity.xmlsec", "unexpected empty chain");
+        return ret;
+    }
+    PCCERT_CONTEXT const pRoot(pSimpleChain->rgpElement[pSimpleChain->cElement-1]->pCertContext);
+    PCCERT_CONTEXT const pIssuerCert = CertFindCertificateInStore(
+            hExtra,
+            PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
+            0,
+            CERT_FIND_SUBJECT_NAME,
+            &pRoot->pCertInfo->Subject,
+            NULL);
+    if (pIssuerCert)
+    {
+        // check that it signed itself
+        DWORD flags = CERT_STORE_SIGNATURE_FLAG;
+        BOOL result = CertVerifySubjectCertificateContext(pRoot, pIssuerCert, &flags);
+        if (result == TRUE && flags == 0)
+        {
+            ret = true;
+        }
+    }
+    CertFreeCertificateContext(pIssuerCert);
+    return ret;
+}
+
 //We return only valid or invalid, as long as the API documentation expresses
 //explicitly that all validation steps are carried out even if one or several
 //errors occur. See also
@@ -839,7 +893,8 @@ sal_Int32 SecurityEnvironment_MSCryptImpl::verifyCertificate(
             DWORD revocationFlags = CERT_TRUST_REVOCATION_STATUS_UNKNOWN |
                 CERT_TRUST_IS_OFFLINE_REVOCATION;
             DWORD otherErrorsMask = ~revocationFlags;
-            if( !(pSimpleChain->TrustStatus.dwErrorStatus & otherErrorsMask))
+            if (!(pSimpleChain->TrustStatus.dwErrorStatus & otherErrorsMask)
+                || CheckUnitTestStore(pChainContext, revocationFlags))
 
             {
                 //No errors except maybe those caused by missing revocation information
@@ -868,6 +923,11 @@ sal_Int32 SecurityEnvironment_MSCryptImpl::verifyCertificate(
                         SAL_INFO("xmlsecurity.xmlsec", "Certificate is valid.");
                         validity = css::security::CertificateValidity::VALID;
                     }
+                    else if (CheckUnitTestStore(pChainContext, 0))
+                    {
+                        SAL_INFO("xmlsecurity.xmlsec", "root certificate found in extra test store");
+                        validity = css::security::CertificateValidity::VALID;
+                    }
                     else
                     {
                         SAL_INFO("xmlsecurity.xmlsec", "Certificate is invalid.");