Bug 87030

> As I see it, there are two possible ways forward to get this stuff to work on 
> Windows:
>
> 1) Set up a NSS database also on Windows. One would think there is a way to 
> get the stuff to be added to that out of the Windows certificate store, using 
> Windows APIs, and then store it in the database, using NSS APIs. Make sure to 
> call NSS_Init() with the path to this database. Then the PDF signing code 
> needs no change.
>
> 2) Add a Windows-specific implementation of the PDF signing. It is a single 
> function, as far as I can see, PDFWriterImpl::finalizeSignature(), that needs 
> ifdefs. This probably makes more sense. Presumably one just needs to figure 
> out the right WinCrypt APIs to use instead of the NSS APIs. The concepts 
> should be fairly similar, one hopes. It shouldn't be more than a handful 
> function calls, knock on wood.

I think that the signatures should work with both methods. If the user configures a NSS store under Windows in Tools->Options->Security->Certificate Path it should work as well as when the default Keystore is used.

Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7e3c931786c3cbe83ee170b8b0746d141b520ce6

fdo#87030: PDF signing using Windows API, work in progress

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.

Setting to NEW.

@Tor, did you want to take this for yourself as assigned?

I don't believe in "assigning" bugs in bugzilla. That is not what directs what paid developers work on.

For reference, here is a diff with various hacks that I have tried to get it to work, but no luck. It is interesting that if I don't use the certificate created directly by CertCreateCertificateContext() from the DER data, but look it up from the "MY" store using CertFindCertificateInStore(), then CertEnumCertificateContextProperties() finds various properties in the certificate.

Also, it is interesting that the certificate provided to me (the "Collabora Dev Softtoken" one) is an AT_KEYEXCHANGE one!? That is what certutil -dump -v and CryptAcquireCertificatePrivateKey() tell me. I would have expected it to be an AT_SIGNATURE one. But probably I misunderstand something.

diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index b459a8d..ab9e2cf
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6215,13 +6215,31 @@ bool PDFWriterImpl::finalizeSignature()
 
 #else
 
-    PCCERT_CONTEXT pCertContext = CertCreateCertificateContext(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, reinterpret_cast<const BYTE*>(n_derArray), n_derLength);
-    if (pCertContext == NULL)
+    PCCERT_CONTEXT pCertContextFromDER = CertCreateCertificateContext(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, reinterpret_cast<const BYTE*>(n_derArray), n_derLength);
+    if (pCertContextFromDER == NULL)
     {
         SAL_WARN("vcl.pdfwriter", "CertCreateCertificateContext failed: " << WindowsError(GetLastError()));
         return false;
     }
 
+#if 1
+    HCERTSTORE hCertStore = CertOpenSystemStore(NULL, "MY");
+    if (!hCertStore)
+    {
+        SAL_WARN("vcl.pdfwriter", "CertOpenSystemStore failed: " << WindowsError(GetLastError()));
+        return false;
+    }
+
+    PCCERT_CONTEXT pCertContext = CertFindCertificateInStore(hCertStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0, CERT_FIND_EXISTING, (const void*) pCertContextFromDER, NULL);
+    if (pCertContext == NULL)
+    {
+        SAL_WARN("vcl.pdfwriter", "CertFindCertificateInStore failed: " << WindowsError(GetLastError()));
+        return false;
+    }
+#else
+    PCCERT_CONTEXT pCertContext = pCertContextFromDER;
+#endif
+
 #if SAL_LOG_INFO
     DWORD nProperty(0);
     bool first(true);
@@ -6234,8 +6252,7 @@ bool PDFWriterImpl::finalizeSignature()
         DWORD nSize(0);
         if (!CertGetCertificateContextProperty(pCertContext, nProperty, NULL, &nSize))
             SAL_INFO("vcl.pdfwriter", "  " << "(missing?) " << std::hex << nProperty);
-        else
-        {
+        else        {
             boost::scoped_array<char> aData(new char[nSize]);
             if (!CertGetCertificateContextProperty(pCertContext, nProperty, aData.get(), &nSize))
                 SAL_INFO("vcl.pdfwriter", "  " << "(missing?) " << std::hex:: << nProperty);
@@ -6243,7 +6260,7 @@ bool PDFWriterImpl::finalizeSignature()
                 SAL_INFO("vcl.pdfwriter", "  " << CertificatePropertyNameAndData(nProperty, aData, nSize));
         }
 #else
-        SAL_INFO("vcl.pdfwriter", "  " << std::hex << nProperty);
+        SAL_INFO("vcl.pdfwriter", "  " << nProperty);
 #endif
     }
 #endif
@@ -6252,7 +6269,7 @@ bool PDFWriterImpl::finalizeSignature()
     CHECK_RETURN( (osl::File::E_None == m_aFile.setPos(osl_Pos_Absolut, 0)) );
 
     HCRYPTPROV hCryptProvider;
-    if (!CryptAcquireContext(&hCryptProvider, NULL, MS_ENHANCED_PROV, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
+    if (!CryptAcquireContext(&hCryptProvider, NULL, MS_ENH_RSA_AES_PROV, PROV_RSA_AES, CRYPT_VERIFYCONTEXT))
     {
         SAL_WARN("vcl.pdfwriter", "CryptAcquireContext failed: " << WindowsError(GetLastError()));
         CertFreeCertificateContext(pCertContext);
@@ -6268,6 +6285,7 @@ bool PDFWriterImpl::finalizeSignature()
         return false;
     }
 
+#if 0
     DWORD nHashSize;
     DWORD nHashSizeLen(sizeof(DWORD));
     if (!CryptGetHashParam(hHash, HP_HASHSIZE, reinterpret_cast<BYTE *>(&nHashSize), &nHashSizeLen, 0))
@@ -6281,6 +6299,7 @@ bool PDFWriterImpl::finalizeSignature()
 
     assert(nHashSizeLen == sizeof(DWORD));
     assert(nHashSize == 20);
+#endif
 
     boost::scoped_array<char> buffer(new char[m_nSignatureContentOffset + 1]);
     sal_uInt64 bytesRead;
@@ -6337,8 +6356,26 @@ bool PDFWriterImpl::finalizeSignature()
 
     OString pass = OUStringToOString( m_aContext.SignPassword, RTL_TEXTENCODING_UTF8 );
 
+    HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey;
+    DWORD nKeySpecType;
+    BOOL bMustFree;
+    if (!CryptAcquireCertificatePrivateKey(pCertContext, 0, NULL, &hCryptProvOrNCryptKey, &nKeySpecType, &bMustFree))
+    {
+        SAL_WARN("vcl.pdfwriter", "CryptAcquireCertificatePrivateKey failed: " << WindowsError(GetLastError()));
+        CryptDestroyHash(hHash);
+        CryptReleaseContext(hCryptProvider, 0);
+        CertFreeCertificateContext(pCertContext);
+        return false;
+    }
+
+    SAL_INFO("vcl.pdfwriter", "Certificate has private key that is " <<
+             (nKeySpecType == AT_KEYEXCHANGE ? OUString("for key exchange") :
+              (nKeySpecType == AT_SIGNATURE ? OUString("for signatures") :
+               (nKeySpecType == CERT_NCRYPT_KEY_SPEC ? OUString("a CNG key, whatever that is") :
+                OUString("unknown type (") + OUString::number(nKeySpecType, 16) + ")"))));
+
     DWORD nSigLen(0);
-    if (!CryptSignHash(hHash, AT_SIGNATURE, NULL, 0, NULL, &nSigLen))
+    if (!CryptSignHash(hHash, nKeySpecType, NULL, CRYPT_NOHASHOID, NULL, &nSigLen))
     {
         SAL_WARN("vcl.pdfwriter", "CryptSignHash failed: " << WindowsError(GetLastError()));
         CryptDestroyHash(hHash);
@@ -6348,7 +6385,7 @@ bool PDFWriterImpl::finalizeSignature()
     }
 
     boost::scoped_array<BYTE> pSig(new BYTE[nSigLen]);
-    if (!CryptSignHash(hHash, AT_SIGNATURE, NULL, 0, pSig.get(), &nSigLen))
+    if (!CryptSignHash(hHash, nKeySpecType, NULL, CRYPT_NOHASHOID, pSig.get(), &nSigLen))
     {
         SAL_WARN("vcl.pdfwriter", "CryptSignHash failed: " << WindowsError(GetLastError()));
         CryptDestroyHash(hHash);

Ah, the AT_KEYEXCHANGE / AT_SIGNATURE thing is a red herring, see http://microsoft.public.platformsdk.security.narkive.com/dsu6jLuu/at-signature-and-at-keyexchange for instance, "AT_SIGNATURE keys can ONLY be used to sign; AT_KEYEXCHANGE keys can be use both to sign and decrypt"

For what it's worth, I distilled the code for signing down to a minimal freestanding test program, and get the same error, "CryptSignHash failed: Keyset does not exist". (Build with "cl -MD sign.c" in a "VS2013 x64 Native Tools Command Prompt".)

#include <cassert>
#include <iostream>

#include <windows.h>
#include <wincrypt.h>

#define SAL_WARN(tag,stream) std::cerr << stream << std::endl
#define SAL_INFO(tag,stream) std::cerr << stream << std::endl

#define OUString(s) s

static char *WindowsError(DWORD nErrorCode)
{
    LPSTR pMsgBuf;

    if (FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
                       NULL,
                       nErrorCode,
                       MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
                       (LPSTR)&pMsgBuf,
                       0,
                       NULL) == 0)
      return "unknown";

    if (pMsgBuf[strlen(pMsgBuf)-1] == '\n')
        pMsgBuf[strlen(pMsgBuf)-1] = '\0';

    return pMsgBuf;
}

static bool
foo ()
{
    HCERTSTORE hCertStore = CertOpenSystemStore(NULL, "MY");
    if (!hCertStore)
    {
        SAL_WARN("vcl.pdfwriter", "CertOpenSystemStore failed: " << WindowsError(GetLastError()));
        return false;
    }

    PCCERT_CONTEXT pCertContext = CertFindCertificateInStore(hCertStore, X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 0, CERT_FIND_SUBJECT_STR, (const void*) L"collabora-dev-softtoken@wilhelmtux.ch" , NULL);
    if (pCertContext == NULL)
    {
        SAL_WARN("vcl.pdfwriter", "CertFindCertificateInStore failed: " << WindowsError(GetLastError()));
        return false;
    }

    DWORD nProperty(0);
    bool first(true);
    while ((nProperty = CertEnumCertificateContextProperties(pCertContext, nProperty)) != 0)
    {
        if (first)
            SAL_INFO("vcl.pdfwriter", "Certificate context properties:");
        first = false;
#if 0
        DWORD nSize(0);
        if (!CertGetCertificateContextProperty(pCertContext, nProperty, NULL, &nSize))
            SAL_INFO("vcl.pdfwriter", "  " << "(missing?) " << std::hex << nProperty);
        else
        {
            boost::scoped_array<char> aData(new char[nSize]);
            if (!CertGetCertificateContextProperty(pCertContext, nProperty, aData.get(), &nSize))
                SAL_INFO("vcl.pdfwriter", "  " << "(missing?) " << std::hex:: << nProperty);
            else
                SAL_INFO("vcl.pdfwriter", "  " << CertificatePropertyNameAndData(nProperty, aData, nSize));
        }
#else
        SAL_INFO("vcl.pdfwriter", "  " << nProperty);
#endif
    }

    HCRYPTPROV hCryptProvider;
    if (!CryptAcquireContext(&hCryptProvider, NULL, MS_ENH_RSA_AES_PROV, PROV_RSA_AES, CRYPT_VERIFYCONTEXT))
    {
        SAL_WARN("vcl.pdfwriter", "CryptAcquireContext failed: " << WindowsError(GetLastError()));
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    HCRYPTHASH hHash;
    if (!CryptCreateHash(hCryptProvider, CALG_SHA1, 0, 0, &hHash))
    {
        SAL_WARN("vcl.pdfwriter", "CryptCreateHash failed: " << WindowsError(GetLastError()));
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    DWORD nHashSize;
    DWORD nHashSizeLen(sizeof(DWORD));
    if (!CryptGetHashParam(hHash, HP_HASHSIZE, reinterpret_cast<BYTE *>(&nHashSize), &nHashSizeLen, 0))
    {
        SAL_WARN("vcl.pdfwriter", "CryptGetHashParam failed: " << WindowsError(GetLastError()));
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    assert(nHashSizeLen == sizeof(DWORD));
    assert(nHashSize == 20);

    //FIXME: Check if SHA1 is calculated from the correct byterange
    if (!CryptHashData(hHash, reinterpret_cast<const BYTE *>(foo), 2000, 0))
    {
        SAL_WARN("vcl.pdfwriter", "CryptHashData failed: " << WindowsError(GetLastError()));
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

#if 0 // We don't actualy need the hash bytes
    unsigned char aHash[20];
    if (!CryptGetHashParam(hHash, HP_HASHVAL, aHash, &nHashSize, 0))
    {
        SAL_WARN("vcl.pdfwriter", "CryptGetHashParam failed: " << WindowsError(GetLastError()));
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    if (nHashSize != 20)
    {
        SAL_WARN("vcl.pdfwriter", "CryptGetHashParam returned unexpected size hash value: " << nHashSize);
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }
#endif

    HCRYPTPROV_OR_NCRYPT_KEY_HANDLE hCryptProvOrNCryptKey;
    DWORD nKeySpecType;
    BOOL bMustFree;
    if (!CryptAcquireCertificatePrivateKey(pCertContext, 0, NULL, &hCryptProvOrNCryptKey, &nKeySpecType, &bMustFree))
    {
        SAL_WARN("vcl.pdfwriter", "CryptAcquireCertificatePrivateKey failed: " << WindowsError(GetLastError()));
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    SAL_INFO("vcl.pdfwriter", "Certificate has private key that is " <<
             (nKeySpecType == AT_KEYEXCHANGE ? OUString("for key exchange") :
              (nKeySpecType == AT_SIGNATURE ? OUString("for signatures") :
               (nKeySpecType == CERT_NCRYPT_KEY_SPEC ? OUString("a CNG key, whatever that is") :
                OUString("unknown type")))));

    DWORD nSigLen(0);
    if (!CryptSignHash(hHash, nKeySpecType, NULL, CRYPT_NOHASHOID, NULL, &nSigLen))
    {
        SAL_WARN("vcl.pdfwriter", "CryptSignHash failed: " << WindowsError(GetLastError()));
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    auto pSig(new BYTE[nSigLen]);
    if (!CryptSignHash(hHash, nKeySpecType, NULL, CRYPT_NOHASHOID, pSig, &nSigLen))
    {
        SAL_WARN("vcl.pdfwriter", "CryptSignHash failed: " << WindowsError(GetLastError()));
        CryptDestroyHash(hHash);
        CryptReleaseContext(hCryptProvider, 0);
        CertFreeCertificateContext(pCertContext);
        return false;
    }

    // Release resources
    CryptDestroyHash(hHash);
    CryptReleaseContext(hCryptProvider, 0);
    CertFreeCertificateContext(pCertContext);

  return true;
}

int
main (int argc, char **argv)
{
  return foo() ? 0 : 1;
}

There must be some pretty trivial detail I am missing... sigh. If only somebody who has experience of the Windows certificate, hashing and crypt API could have a look.

OK, I figured it out. Just had to stare even longer and harder at the sample code and finally it hit me how it differed from my code. Will commit fix later tonight or tomorrow.

Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=070c93af73df9aa4eb333265c81060d123b530b9

fdo#87030: Prevent PDF signing using Windows API from failing

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.

Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6e91763769a562b88882a4c2a94b1367c6ed4866

fdo#87030: Generate a proper PKCS#7 signature

It will be available in 4.5.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.

Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=be417961eecf6b096bd64fd55dccd4214c48836f&h=libreoffice-4-4

fdo#87030: Make PDF signing using Windows API work

It will be available in 4.4.0.0.beta3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.

Fix confirmed on Windows 7 with:
Version: 4.5.0.0.alpha0+
Build ID: 170616e9f2d30c1302bbb5a7a4b588bc05cd5cc9
TinderBox: Win-x86@39, Branch:master, Time: 2014-12-12_01:58:46

Thanks guys, great work!