[Libreoffice-commits] core.git: include/svl svl/source xmlsecurity/Library_xsec_xmlsec.mk xmlsecurity/qa xmlsecurity/source
Miklos Vajna
vmiklos at collabora.co.uk
Wed May 30 07:04:55 UTC 2018
include/svl/cryptosign.hxx | 3
svl/source/crypto/cryptosign.cxx | 25 ++++-
xmlsecurity/Library_xsec_xmlsec.mk | 1
xmlsecurity/qa/unit/signing/signing.cxx | 45 +++++++++-
xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx | 27 +++++-
xmlsecurity/source/xmlsec/mscrypt/x509certificate_mscryptimpl.cxx | 12 ++
6 files changed, 103 insertions(+), 10 deletions(-)
New commits:
commit 93e33ba279e837356e157745177d7f6061d442b7
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date: Tue May 29 20:54:52 2018 +0200
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 at libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
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()
More information about the Libreoffice-commits
mailing list