[PATCH v5 0/1] MR10561: secur32: Fix handling by SChannel for CNG/NCrypt client certificates for mTLS
Part 3 of my attempt to run the [Niko Home Control programming software](https://appdb.winehq.org/objectManager.php?sClass=application&iId=21635) under Wine. If I understand correctly, the current implementation only supports retrieving via the legacy CryptoAPI (CAPI). The Niko app uses Cryptography Next Generation (CNG/NCrypt) so this PRs adds support for it in acquire_credentials_handle. -- v5: secur32: Fix handling by SChannel for CNG/NCrypt client certificates for mTLS https://gitlab.winehq.org/wine/wine/-/merge_requests/10561
From: Benoît Legat <benoit.legat@gmail.com> --- dlls/secur32/Makefile.in | 2 +- dlls/secur32/schannel.c | 123 +++++++++++++++++++++++++++++++-- dlls/secur32/schannel_gnutls.c | 54 +++++++-------- dlls/secur32/tests/Makefile.in | 2 +- dlls/secur32/tests/schannel.c | 65 +++++++++++++++++ 5 files changed, 209 insertions(+), 37 deletions(-) diff --git a/dlls/secur32/Makefile.in b/dlls/secur32/Makefile.in index 2120a9c0e02..23704ad6935 100644 --- a/dlls/secur32/Makefile.in +++ b/dlls/secur32/Makefile.in @@ -2,7 +2,7 @@ MODULE = secur32.dll IMPORTLIB = secur32 UNIXLIB = secur32.so IMPORTS = advapi32 -DELAYIMPORTS = crypt32 +DELAYIMPORTS = crypt32 ncrypt UNIX_CFLAGS = $(GNUTLS_CFLAGS) SOURCES = \ diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c index 83aea0b4737..261d0606244 100644 --- a/dlls/secur32/schannel.c +++ b/dlls/secur32/schannel.c @@ -32,6 +32,8 @@ #include "sspi.h" #define SCHANNEL_USE_BLACKLISTS #include "schannel.h" +#include "bcrypt.h" +#include "ncrypt.h" #include "wine/unixlib.h" #include "wine/debug.h" @@ -512,6 +514,80 @@ static WCHAR *get_key_container_path(const CERT_CONTEXT *ctx) } #define MAX_LEAD_BYTES 8 + +static void reverse_bytes(BYTE *buf, ULONG len) +{ + BYTE tmp; + ULONG i; + for (i = 0; i < len / 2; i++) + { + tmp = buf[i]; + buf[i] = buf[len - i - 1]; + buf[len - i - 1] = tmp; + } +} + +/* Convert CAPI PRIVATEKEYBLOB (little-endian) to BCRYPT_RSAKEY_BLOB (big-endian) */ +static BYTE *convert_capi_to_bcrypt(const BYTE *capi_blob, DWORD capi_size, DWORD *out_size) +{ + const BLOBHEADER *blob_hdr = (const BLOBHEADER *)capi_blob; + const RSAPUBKEY *rsa_hdr = (const RSAPUBKEY *)(blob_hdr + 1); + BCRYPT_RSAKEY_BLOB *hdr; + DWORD bitlen, modlen, half, bcrypt_size; + const BYTE *src; + BYTE *buf, *dst; + + if (capi_size < sizeof(BLOBHEADER) + sizeof(RSAPUBKEY)) return NULL; + + bitlen = rsa_hdr->bitlen; + modlen = bitlen / 8; + half = bitlen / 16; + + bcrypt_size = sizeof(BCRYPT_RSAKEY_BLOB) + sizeof(rsa_hdr->pubexp) + modlen * 2 + half * 5; + if (!(buf = malloc(bcrypt_size + MAX_LEAD_BYTES))) return NULL; + + hdr = (BCRYPT_RSAKEY_BLOB *)buf; + hdr->Magic = BCRYPT_RSAFULLPRIVATE_MAGIC; + hdr->BitLength = bitlen; + hdr->cbPublicExp = sizeof(rsa_hdr->pubexp); + hdr->cbModulus = modlen; + hdr->cbPrime1 = half; + hdr->cbPrime2 = half; + + dst = buf + sizeof(*hdr); + + /* PublicExp: CAPI stores as DWORD (little-endian), BCRYPT as big-endian bytes */ + reverse_bytes((BYTE *)&rsa_hdr->pubexp, sizeof(rsa_hdr->pubexp)); + memcpy(dst, &rsa_hdr->pubexp, sizeof(rsa_hdr->pubexp)); + dst += sizeof(rsa_hdr->pubexp); + + src = (const BYTE *)(rsa_hdr + 1); + + /* Modulus */ + memcpy(dst, src, modlen); reverse_bytes(dst, modlen); + src += modlen; dst += modlen; + /* Prime1 */ + memcpy(dst, src, half); reverse_bytes(dst, half); + src += half; dst += half; + /* Prime2 */ + memcpy(dst, src, half); reverse_bytes(dst, half); + src += half; dst += half; + /* Exponent1 */ + memcpy(dst, src, half); reverse_bytes(dst, half); + src += half; dst += half; + /* Exponent2 */ + memcpy(dst, src, half); reverse_bytes(dst, half); + src += half; dst += half; + /* Coefficient */ + memcpy(dst, src, half); reverse_bytes(dst, half); + src += half; dst += half; + /* PrivateExponent */ + memcpy(dst, src, modlen); reverse_bytes(dst, modlen); + + *out_size = bcrypt_size + MAX_LEAD_BYTES; + return buf; +} + static BYTE *get_key_blob(const CERT_CONTEXT *ctx, DWORD *size) { BYTE *buf, *ret = NULL; @@ -548,19 +624,53 @@ static BYTE *get_key_blob(const CERT_CONTEXT *ctx, DWORD *size) blob_in.cbData = len; if (CryptUnprotectData(&blob_in, NULL, NULL, NULL, NULL, 0, &blob_out)) { - assert(blob_in.cbData >= blob_out.cbData); - memcpy(buf, blob_out.pbData, blob_out.cbData); + ret = convert_capi_to_bcrypt(blob_out.pbData, blob_out.cbData, size); LocalFree(blob_out.pbData); - *size = blob_out.cbData + MAX_LEAD_BYTES; - ret = buf; } } - else free(buf); + free(buf); RegCloseKey(hkey); return ret; } +static BYTE *get_key_blob_ncrypt(const CERT_CONTEXT *ctx, DWORD *size) +{ + CERT_KEY_CONTEXT keyctx; + DWORD ctx_size = sizeof(keyctx); + NCRYPT_KEY_HANDLE key; + DWORD blob_size; + BYTE *buf; + SECURITY_STATUS status; + + if (!CertGetCertificateContextProperty(ctx, CERT_KEY_CONTEXT_PROP_ID, &keyctx, &ctx_size)) + return NULL; + if (keyctx.dwKeySpec != CERT_NCRYPT_KEY_SPEC) + return NULL; + + key = keyctx.hCryptProv; + + status = NCryptExportKey(key, 0, BCRYPT_RSAFULLPRIVATE_BLOB, NULL, NULL, 0, &blob_size, 0); + if (status) + { + TRACE("NCryptExportKey size query failed: %#lx\n", status); + return NULL; + } + + if (!(buf = malloc(blob_size + MAX_LEAD_BYTES))) return NULL; + + status = NCryptExportKey(key, 0, BCRYPT_RSAFULLPRIVATE_BLOB, NULL, buf, blob_size, &blob_size, 0); + if (status) + { + TRACE("NCryptExportKey failed: %#lx\n", status); + free(buf); + return NULL; + } + + *size = blob_size + MAX_LEAD_BYTES; + return buf; +} + static SECURITY_STATUS acquire_credentials_handle(ULONG fCredentialUse, const SCHANNEL_CRED *schanCred, PCredHandle phCredential, PTimeStamp ptsExpiry) { @@ -614,7 +724,8 @@ static SECURITY_STATUS acquire_credentials_handle(ULONG fCredentialUse, creds->credential_use = fCredentialUse; creds->enabled_protocols = enabled_protocols; - if (cert && !(key_blob = get_key_blob(cert, &key_size))) goto fail; + if (cert && !(key_blob = get_key_blob(cert, &key_size)) + && !(key_blob = get_key_blob_ncrypt(cert, &key_size))) goto fail; params.c = creds; if (cert) { diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c index ca7e82bbbde..b95d8875d65 100644 --- a/dlls/secur32/schannel_gnutls.c +++ b/dlls/secur32/schannel_gnutls.c @@ -42,6 +42,7 @@ #include "windef.h" #include "winbase.h" #include "winternl.h" +#include "bcrypt.h" #include "sspi.h" #include "secur32_priv.h" @@ -1300,24 +1301,11 @@ static NTSTATUS schan_set_dtls_timeouts( void *args ) return SEC_E_OK; } -static inline void reverse_bytes(BYTE *buf, ULONG len) -{ - BYTE tmp; - ULONG i; - for (i = 0; i < len / 2; i++) - { - tmp = buf[i]; - buf[i] = buf[len - i - 1]; - buf[len - i - 1] = tmp; - } -} - static ULONG set_component(gnutls_datum_t *comp, BYTE *data, ULONG len, ULONG *buflen) { comp->data = data; comp->size = len; - reverse_bytes(comp->data, comp->size); - if (comp->data[0] & 0x80) /* add leading 0 byte if most significant bit is set */ + if (comp->size > 0 && comp->data[0] & 0x80) /* add leading 0 byte if most significant bit is set */ { memmove(comp->data + 1, comp->data, *buflen); comp->data[0] = 0; @@ -1327,32 +1315,40 @@ static ULONG set_component(gnutls_datum_t *comp, BYTE *data, ULONG len, ULONG *b return comp->size; } +/* BCRYPT_RSAKEY_BLOB layout: already big-endian, matching GnuTLS expectations. */ static gnutls_x509_privkey_t get_x509_key(ULONG key_size, const BYTE *key_blob) { gnutls_privkey_t key = NULL; gnutls_x509_privkey_t x509key = NULL; gnutls_datum_t m, e, d, p, q, u, e1, e2; + const BCRYPT_RSAKEY_BLOB *hdr = (const BCRYPT_RSAKEY_BLOB *)key_blob; BYTE *ptr; - RSAPUBKEY *rsakey; - DWORD size = key_size; + DWORD size; int ret; - if (size < sizeof(BLOBHEADER)) return NULL; + if (key_size < sizeof(*hdr)) return NULL; + if (hdr->Magic != BCRYPT_RSAFULLPRIVATE_MAGIC) + { + TRACE("unexpected magic %#x\n", (unsigned)hdr->Magic); + return NULL; + } - rsakey = (RSAPUBKEY *)(key_blob + sizeof(BLOBHEADER)); - TRACE("RSA key bitlen %u pubexp %u\n", (unsigned)rsakey->bitlen, (unsigned)rsakey->pubexp); + TRACE("BCRYPT RSA key bitlen %u cbExp %u cbMod %u cbP1 %u cbP2 %u\n", + (unsigned)hdr->BitLength, (unsigned)hdr->cbPublicExp, (unsigned)hdr->cbModulus, + (unsigned)hdr->cbPrime1, (unsigned)hdr->cbPrime2); - size -= sizeof(BLOBHEADER) + FIELD_OFFSET(RSAPUBKEY, pubexp); - set_component(&e, (BYTE *)&rsakey->pubexp, sizeof(rsakey->pubexp), &size); + size = key_size - sizeof(*hdr); + ptr = (BYTE *)(hdr + 1); - ptr = (BYTE *)(rsakey + 1); - ptr += set_component(&m, ptr, rsakey->bitlen / 8, &size); - ptr += set_component(&p, ptr, rsakey->bitlen / 16, &size); - ptr += set_component(&q, ptr, rsakey->bitlen / 16, &size); - ptr += set_component(&e1, ptr, rsakey->bitlen / 16, &size); - ptr += set_component(&e2, ptr, rsakey->bitlen / 16, &size); - ptr += set_component(&u, ptr, rsakey->bitlen / 16, &size); - ptr += set_component(&d, ptr, rsakey->bitlen / 8, &size); + /* BCRYPT blob: PublicExp, Modulus, Prime1, Prime2, Exponent1, Exponent2, Coefficient, PrivateExponent */ + ptr += set_component(&e, ptr, hdr->cbPublicExp, &size); + ptr += set_component(&m, ptr, hdr->cbModulus, &size); + ptr += set_component(&p, ptr, hdr->cbPrime1, &size); + ptr += set_component(&q, ptr, hdr->cbPrime2, &size); + ptr += set_component(&e1, ptr, hdr->cbPrime1, &size); + ptr += set_component(&e2, ptr, hdr->cbPrime2, &size); + ptr += set_component(&u, ptr, hdr->cbPrime1, &size); + ptr += set_component(&d, ptr, hdr->cbModulus, &size); if ((ret = pgnutls_privkey_init(&key)) < 0) { diff --git a/dlls/secur32/tests/Makefile.in b/dlls/secur32/tests/Makefile.in index caeac7e47e1..06518618639 100644 --- a/dlls/secur32/tests/Makefile.in +++ b/dlls/secur32/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = secur32.dll -IMPORTS = secur32 crypt32 advapi32 ws2_32 +IMPORTS = secur32 crypt32 ncrypt advapi32 ws2_32 SOURCES = \ main.c \ diff --git a/dlls/secur32/tests/schannel.c b/dlls/secur32/tests/schannel.c index 04ef9ca6880..58089b451d7 100644 --- a/dlls/secur32/tests/schannel.c +++ b/dlls/secur32/tests/schannel.c @@ -27,6 +27,11 @@ #include <security.h> #define SCHANNEL_USE_BLACKLISTS #include <schannel.h> +#include <ncrypt.h> + +#ifndef CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG +#define CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG 0x00010000 +#endif #include "wine/test.h" @@ -2051,6 +2056,65 @@ static void test_connection_shutdown(void) FreeCredentialsHandle( &cred_handle ); } +static void test_ncrypt_key_credentials(void) +{ + SCHANNEL_CRED schanCred; + CredHandle cred; + SECURITY_STATUS st; + CRYPT_DATA_BLOB pfx; + HCERTSTORE store; + const CERT_CONTEXT *cert; + NCRYPT_KEY_HANDLE ncrypt_key = 0; + DWORD key_spec = 0; + BOOL free_key = FALSE; + BOOL ret; + + pfx.pbData = (BYTE *)pfxdata; + pfx.cbData = sizeof(pfxdata); + store = PFXImportCertStore(&pfx, NULL, CRYPT_EXPORTABLE | PKCS12_ALWAYS_CNG_KSP); + ok(store != NULL, "PFXImportCertStore failed: %lu\n", GetLastError()); + if (!store) return; + + cert = CertFindCertificateInStore(store, X509_ASN_ENCODING, 0, CERT_FIND_ANY, NULL, NULL); + ok(cert != NULL, "CertFindCertificateInStore failed: %lu\n", GetLastError()); + if (!cert) + { + CertCloseStore(store, 0); + return; + } + + /* Verify the key is NCrypt. */ + ret = CryptAcquireCertificatePrivateKey(cert, CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG, NULL, + &ncrypt_key, &key_spec, &free_key); + ok(ret, "CryptAcquireCertificatePrivateKey failed: %lu\n", GetLastError()); + todo_wine + ok(key_spec == CERT_NCRYPT_KEY_SPEC, + "expected CERT_NCRYPT_KEY_SPEC, got %lu\n", key_spec); + + /* AcquireCredentialsHandle should succeed with an NCrypt key. */ + init_cred(&schanCred); + schanCred.cCreds = 1; + schanCred.paCred = &cert; + st = AcquireCredentialsHandleA(NULL, (SEC_CHAR *)UNISP_NAME_A, SECPKG_CRED_OUTBOUND, + NULL, &schanCred, NULL, NULL, &cred, NULL); + ok(st == SEC_E_OK, "AcquireCredentialsHandleA outbound with NCrypt key failed: %08lx\n", st); + if (st == SEC_E_OK) FreeCredentialsHandle(&cred); + + st = AcquireCredentialsHandleA(NULL, (SEC_CHAR *)UNISP_NAME_A, SECPKG_CRED_INBOUND, + NULL, &schanCred, NULL, NULL, &cred, NULL); + ok(st == SEC_E_OK, "AcquireCredentialsHandleA inbound with NCrypt key failed: %08lx\n", st); + if (st == SEC_E_OK) FreeCredentialsHandle(&cred); + + /* Clean up the key handle. */ + if (ret && key_spec == CERT_NCRYPT_KEY_SPEC) + NCryptFreeObject(ncrypt_key); + else if (ret && free_key) + CryptReleaseContext(ncrypt_key, 0); + + CertFreeCertificateContext(cert); + CertCloseStore(store, 0); +} + START_TEST(schannel) { WSADATA wsa_data; @@ -2059,6 +2123,7 @@ START_TEST(schannel) test_cread_attrs(); testAcquireSecurityContext(); + test_ncrypt_key_credentials(); test_InitializeSecurityContext(); test_communication(); test_application_protocol_negotiation(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10561
I just squashed. Do you prefer that I always keep is stashed or leave it as separate commit during peer reviewing and only squash once the MR is accepted ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10561#note_136278
I (and I think most of us) prefer to see fixups squashed into their corresponding commit. Note that gitlab can show the diff compared to the previous version, so that information isn't lost. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10561#note_136287
Hans Leidekker (@hans) commented about dlls/secur32/tests/schannel.c:
#include <security.h> #define SCHANNEL_USE_BLACKLISTS #include <schannel.h> +#include <ncrypt.h> + +#ifndef CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG +#define CRYPT_ACQUIRE_ALLOW_NCRYPT_KEY_FLAG 0x00010000 +#endif This define should go in include/wincrypt.h.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10561#note_136502
participants (3)
-
Benoît Legat -
Benoît Legat (＠blegat)
-
Hans Leidekker (＠hans)