xmlsecurity windows: let cert picker and PDF sign find ECDSA keys

Need to incrementally migrate the remaining places (ODF, OOXML signing)
to CNG, then flip the default. SVL_CRYPTO_CNG=1 is needed till then.

(The testcase passes with and without the fix when SVL_CRYPTO_CNG is not
specified; it fails without the fix when SVL_CRYPTO_CNG is specified.)

Change-Id: Ide9d3b109bbd955a9cb83b18bba6aa72269f4d34
Reviewed-on: https://gerrit.libreoffice.org/55030
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

6 files changed, 103 insertions, 10 deletions

diff --git a/include/svl/cryptosign.hxx b/include/svl/cryptosign.hxx
index eacb4d78af25..b70b995b23b9 100644
--- a/include/svl/cryptosign.hxx
+++ b/include/svl/cryptosign.hxx
@@ -86,6 +86,9 @@ private:
     OUString m_aSignPassword;
 };
 
+/// Decides if SVL_CRYPTO_MSCRYPTO uses the new CNG API or not.
+SVL_DLLPUBLIC bool isMSCng();
+
 }
 }
 
diff --git a/svl/source/crypto/cryptosign.cxx b/svl/source/crypto/cryptosign.cxx
index 96c349d68861..c7e62d58f836 100644
--- a/svl/source/crypto/cryptosign.cxx
+++ b/svl/source/crypto/cryptosign.cxx
@@ -1401,14 +1401,22 @@ bool Signing::Sign(OStringBuffer& rCMSHexBuffer)
     aPara.cMsgCert = 1;
     aPara.rgpMsgCert = &pCertContext;
 
-    HCRYPTPROV hCryptProv;
+    HCRYPTPROV hCryptProv = 0;
+    NCRYPT_KEY_HANDLE hCryptKey = 0;
+    DWORD dwFlags = CRYPT_ACQUIRE_CACHE_FLAG;
+    HCRYPTPROV_OR_NCRYPT_KEY_HANDLE* phCryptProvOrNCryptKey = &hCryptProv;
+    if (svl::crypto::isMSCng())
+    {
+        dwFlags |= CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG;
+        phCryptProvOrNCryptKey = &hCryptKey;
+    }
     DWORD nKeySpec;
     BOOL bFreeNeeded;
 
     if (!CryptAcquireCertificatePrivateKey(pCertContext,
-                                           CRYPT_ACQUIRE_CACHE_FLAG,
+                                           dwFlags,
                                            nullptr,
-                                           &hCryptProv,
+                                           phCryptProvOrNCryptKey,
                                            &nKeySpec,
                                            &bFreeNeeded))
     {
@@ -1423,7 +1431,10 @@ bool Signing::Sign(OStringBuffer& rCMSHexBuffer)
     memset(&aSignerInfo, 0, sizeof(aSignerInfo));
     aSignerInfo.cbSize = sizeof(aSignerInfo);
     aSignerInfo.pCertInfo = pCertContext->pCertInfo;
-    aSignerInfo.hCryptProv = hCryptProv;
+    if (!svl::crypto::isMSCng())
+        aSignerInfo.hCryptProv = hCryptProv;
+    else
+        aSignerInfo.hNCryptKey = hCryptKey;
     aSignerInfo.dwKeySpec = nKeySpec;
     aSignerInfo.HashAlgorithm.pszObjId = const_cast<LPSTR>(szOID_NIST_sha256);
     aSignerInfo.HashAlgorithm.Parameters.cbData = 0;
@@ -2409,6 +2420,12 @@ bool Signing::Verify(SvStream& rStream,
 #endif
 }
 
+bool isMSCng()
+{
+    static bool bMSCng = getenv("SVL_CRYPTO_CNG");
+    return bMSCng;
+}
+
 }
 
 }
diff --git a/xmlsecurity/Library_xsec_xmlsec.mk b/xmlsecurity/Library_xsec_xmlsec.mk
index ced31bc9c668..445d48e72a32 100644
--- a/xmlsecurity/Library_xsec_xmlsec.mk
+++ b/xmlsecurity/Library_xsec_xmlsec.mk
@@ -98,6 +98,7 @@ $(eval $(call gb_Library_add_libs,xsec_xmlsec,\
 $(eval $(call gb_Library_use_system_win32_libs,xsec_xmlsec,\
 	crypt32 \
 	advapi32 \
+	ncrypt \
 ))
 
 $(eval $(call gb_Library_add_exception_objects,xsec_xmlsec,\
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index 7e326f2467bf..ece73c8dcdb2 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -37,6 +37,7 @@
 #include <osl/file.hxx>
 #include <osl/process.h>
 #include <comphelper/ofopxmlhelper.hxx>
+#include <unotools/streamwrap.hxx>
 
 #include <documentsignaturehelper.hxx>
 #include <xmlsignaturehelper.hxx>
@@ -67,6 +68,7 @@ public:
     void testDescription();
     void testECDSA();
     void testECDSAOOXML();
+    void testECDSAPDF();
     /// Test a typical ODF where all streams are signed.
     void testODFGood();
     /// Test a typical broken ODF signature where one stream is corrupted.
@@ -118,6 +120,7 @@ public:
     CPPUNIT_TEST(testDescription);
     CPPUNIT_TEST(testECDSA);
     CPPUNIT_TEST(testECDSAOOXML);
+    CPPUNIT_TEST(testECDSAPDF);
     CPPUNIT_TEST(testODFGood);
     CPPUNIT_TEST(testODFBroken);
     CPPUNIT_TEST(testODFNo);
@@ -321,7 +324,7 @@ void SigningTest::testECDSAOOXML()
     CPPUNIT_ASSERT(aManager.init());
     uno::Reference<embed::XStorage> xStorage
         = comphelper::OStorageHelper::GetStorageOfFormatFromURL(
-            ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(), embed::ElementModes::READWRITE);
+              ZIP_STORAGE_FORMAT_STRING, aTempFile.GetURL(), embed::ElementModes::READWRITE);
     CPPUNIT_ASSERT(xStorage.is());
     aManager.mxStore = xStorage;
     aManager.maSignatureHelper.SetStorage(xStorage, "1.2");
@@ -346,6 +349,46 @@ void SigningTest::testECDSAOOXML()
                          rInformations[0].nStatus);
 }
 
+void SigningTest::testECDSAPDF()
+{
+    // Create an empty document and store it to a tempfile, finally load it as
+    // a stream.
+    createDoc("");
+
+    utl::TempFile aTempFile;
+    aTempFile.EnableKillingFile();
+    uno::Reference<frame::XStorable> xStorable(mxComponent, uno::UNO_QUERY);
+    utl::MediaDescriptor aMediaDescriptor;
+    aMediaDescriptor["FilterName"] <<= OUString("writer_pdf_Export");
+    xStorable->storeToURL(aTempFile.GetURL(), aMediaDescriptor.getAsConstPropertyValueList());
+
+    DocumentSignatureManager aManager(mxComponentContext, DocumentSignatureMode::Content);
+    CPPUNIT_ASSERT(aManager.init());
+    std::unique_ptr<SvStream> pStream(utl::UcbStreamHelper::CreateStream(aTempFile.GetURL(), StreamMode::READ | StreamMode::WRITE));
+    uno::Reference<io::XStream> xStream(new utl::OStreamWrapper(*pStream));
+    CPPUNIT_ASSERT(xStream.is());
+    aManager.mxSignatureStream = xStream;
+
+    // Then add a document signature.
+    uno::Reference<security::XCertificate> xCertificate
+        = getCertificate(aManager, svl::crypto::SignatureMethodAlgorithm::ECDSA);
+    if (!xCertificate.is())
+        return;
+    OUString aDescription;
+    sal_Int32 nSecurityId;
+    aManager.add(xCertificate, mxSecurityContext, aDescription, nSecurityId,
+                 /*bAdESCompliant=*/true);
+
+    // Read back the signature and make sure that it's valid.
+    aManager.read(/*bUseTempStream=*/false);
+    std::vector<SignatureInformation>& rInformations = aManager.maCurrentSignatureInformations;
+    CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(1), rInformations.size());
+    // This was SecurityOperationStatus_UNKNOWN, signing with an ECDSA key was
+    // broken.
+    CPPUNIT_ASSERT_EQUAL(css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED,
+                         rInformations[0].nStatus);
+}
+
 void SigningTest::testOOXMLDescription()
 {
     // Create an empty document and store it to a tempfile, finally load it as a storage.
diff --git a/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx
index ecfdd15d1895..4f1b7e81221f 100644
--- a/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx
+++ b/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx
@@ -45,6 +45,7 @@
 #include <osl/nlsupport.h>
 #include <osl/process.h>
 #include <o3tl/char16_t2wchar_t.hxx>
+#include <svl/cryptosign.hxx>
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::lang ;
@@ -344,6 +345,7 @@ uno::Sequence< uno::Reference < XCertificate > > SecurityEnvironment_MSCryptImpl
         HCERTSTORE hSystemKeyStore ;
         DWORD      dwKeySpec;
         HCRYPTPROV hCryptProv;
+        NCRYPT_KEY_HANDLE hCryptKey;
 
 #ifdef SAL_LOG_INFO
         CertEnumSystemStore(CERT_SYSTEM_STORE_CURRENT_USER, nullptr, nullptr, cert_enum_system_store_callback);
@@ -355,10 +357,17 @@ uno::Sequence< uno::Reference < XCertificate > > SecurityEnvironment_MSCryptImpl
             while (pCertContext)
             {
                 // for checking whether the certificate is a personal certificate or not.
+                DWORD dwFlags = CRYPT_ACQUIRE_COMPARE_KEY_FLAG;
+                HCRYPTPROV_OR_NCRYPT_KEY_HANDLE* phCryptProvOrNCryptKey = &hCryptProv;
+                if (svl::crypto::isMSCng())
+                {
+                    dwFlags |= CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG;
+                    phCryptProvOrNCryptKey = &hCryptKey;
+                }
                 if(!(CryptAcquireCertificatePrivateKey(pCertContext,
-                        CRYPT_ACQUIRE_COMPARE_KEY_FLAG,
+                        dwFlags,
                         nullptr,
-                        &hCryptProv,
+                        phCryptProvOrNCryptKey,
                         &dwKeySpec,
                         nullptr)))
                 {
@@ -969,10 +978,18 @@ sal_Int32 SecurityEnvironment_MSCryptImpl::getCertificateCharacters( const css::
         BOOL    fCallerFreeProv ;
         DWORD   dwKeySpec ;
         HCRYPTPROV  hProv ;
+        NCRYPT_KEY_HANDLE hKey = 0;
+        DWORD dwFlags = 0;
+        HCRYPTPROV_OR_NCRYPT_KEY_HANDLE* phCryptProvOrNCryptKey = &hProv;
+        if (svl::crypto::isMSCng())
+        {
+            dwFlags |= CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG;
+            phCryptProvOrNCryptKey = &hKey;
+        }
         if( CryptAcquireCertificatePrivateKey( pCertContext ,
-                   0 ,
+                   dwFlags,
                    nullptr ,
-                   &hProv,
+                   phCryptProvOrNCryptKey,
                    &dwKeySpec,
                    &fCallerFreeProv )
         ) {
@@ -980,6 +997,8 @@ sal_Int32 SecurityEnvironment_MSCryptImpl::getCertificateCharacters( const css::
 
             if( hProv != NULL && fCallerFreeProv )
                 CryptReleaseContext( hProv, 0 ) ;
+            else if (hKey && fCallerFreeProv)
+                NCryptFreeObject(hKey);
         } else {
             characters &= ~ css::security::CertificateCharacters::HAS_PRIVATE_KEY ;
         }
diff --git a/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx
index d213f21631f5..1e3bd93880f9 100644
--- a/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx
+++ b/xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx
@@ -573,7 +573,17 @@ uno::Sequence<sal_Int8> X509Certificate_MSCryptImpl::getSHA256Thumbprint()
 
 svl::crypto::SignatureMethodAlgorithm X509Certificate_MSCryptImpl::getSignatureMethodAlgorithm()
 {
-    return svl::crypto::SignatureMethodAlgorithm::RSA;
+    svl::crypto::SignatureMethodAlgorithm nRet = svl::crypto::SignatureMethodAlgorithm::RSA;
+
+    if (!m_pCertContext || !m_pCertContext->pCertInfo)
+        return nRet;
+
+    CRYPT_ALGORITHM_IDENTIFIER algorithm = m_pCertContext->pCertInfo->SubjectPublicKeyInfo.Algorithm;
+    OString aObjId(algorithm.pszObjId);
+    if (aObjId == szOID_ECC_PUBLIC_KEY)
+        nRet = svl::crypto::SignatureMethodAlgorithm::ECDSA;
+
+    return nRet;
 }
 
 css::uno::Sequence< sal_Int8 > SAL_CALL X509Certificate_MSCryptImpl::getSHA1Thumbprint()