From f76c14c256afc0f07618d14deba71aaca5c5f4e7 Mon Sep 17 00:00:00 2001 From: David Fort Date: Thu, 13 Oct 2022 00:09:52 +0200 Subject: [PATCH] fix smartcard logon with smartcard emulation When smartcard emulation was enabled we were dumping the key and cert to temporary files for PKINIT call, but they were deleted before we have actually done the PKINIT. This patch fixes it. It also add debug statement for the listing of smartcard keys / certs. This also fixes the listing of smartcard on certain windows configurations were we have to force NCRYPT_SILENT when doing a NCryptOpenKey. --- client/common/smartcard_cli.c | 2 +- include/freerdp/utils/smartcardlogon.h | 2 +- libfreerdp/core/nla.c | 19 ++++---- libfreerdp/core/smartcardlogon.c | 34 ++++++++++++--- winpr/include/winpr/ncrypt.h | 8 ++++ winpr/libwinpr/ncrypt/ncrypt.c | 60 +++++++++++++++++++++++++- 6 files changed, 108 insertions(+), 17 deletions(-) diff --git a/client/common/smartcard_cli.c b/client/common/smartcard_cli.c index d4181dc5c..f38abeb25 100644 --- a/client/common/smartcard_cli.c +++ b/client/common/smartcard_cli.c @@ -54,7 +54,7 @@ BOOL freerdp_smartcard_list(const rdpSettings* settings) if (info->upn) printf("\t* UPN: %s\n", info->upn); } - smartcardCerts_Free(certs); + smartcardCerts_Free(&certs); return TRUE; } diff --git a/include/freerdp/utils/smartcardlogon.h b/include/freerdp/utils/smartcardlogon.h index 748bfbb40..2964ed48e 100644 --- a/include/freerdp/utils/smartcardlogon.h +++ b/include/freerdp/utils/smartcardlogon.h @@ -45,6 +45,6 @@ typedef struct FREERDP_API BOOL smartcard_enumerateCerts(const rdpSettings* settings, SmartcardCerts** scCert, DWORD* retCount); FREERDP_API const SmartcardCertInfo* smartcard_getCertInfo(SmartcardCerts* scCerts, DWORD index); -FREERDP_API void smartcardCerts_Free(SmartcardCerts* scCert); +FREERDP_API void smartcardCerts_Free(SmartcardCerts** pscCert); #endif /* LIBFREERDP_CORE_SMARTCARDLOGON_H */ diff --git a/libfreerdp/core/nla.c b/libfreerdp/core/nla.c index 3ada15a64..86ac8de37 100644 --- a/libfreerdp/core/nla.c +++ b/libfreerdp/core/nla.c @@ -124,6 +124,8 @@ struct rdp_nla rdpCredsspAuth* auth; char* pkinitArgs; + SmartcardCerts* smartcardCerts; + DWORD nsmartcardCerts; BYTE certSha1[20]; }; @@ -185,9 +187,7 @@ static const UINT32 NonceLength = 32; static BOOL nla_adjust_settings_from_smartcard(rdpNla* nla) { - SmartcardCerts* certs = NULL; const SmartcardCertInfo* info = NULL; - DWORD count; rdpSettings* settings; BOOL ret = FALSE; @@ -200,22 +200,24 @@ static BOOL nla_adjust_settings_from_smartcard(rdpNla* nla) if (!settings->SmartcardLogon) return TRUE; - if (!smartcard_enumerateCerts(settings, &certs, &count)) + smartcardCerts_Free(&nla->smartcardCerts); + + if (!smartcard_enumerateCerts(settings, &nla->smartcardCerts, &nla->nsmartcardCerts)) { WLog_ERR(TAG, "unable to list smartcard certificates"); return FALSE; } - if (count < 1) + if (nla->nsmartcardCerts < 1) { WLog_ERR(TAG, "no smartcard certificates found"); goto out; } - if (count != 1) + if (nla->nsmartcardCerts != 1) goto setup_pin; - info = smartcard_getCertInfo(certs, 0); + info = smartcard_getCertInfo(nla->smartcardCerts, 0); if (!info) goto out; @@ -291,8 +293,6 @@ setup_pin: ret = TRUE; out: - smartcardCerts_Free(certs); - return ret; } @@ -1694,6 +1694,7 @@ void nla_free(rdpNla* nla) if (!nla) return; + smartcardCerts_Free(&nla->smartcardCerts); sspi_SecBufferFree(&nla->pubKeyAuth); sspi_SecBufferFree(&nla->authInfo); sspi_SecBufferFree(&nla->negoToken); @@ -1703,8 +1704,8 @@ void nla_free(rdpNla* nla) credssp_auth_free(nla->auth); sspi_FreeAuthIdentity(nla->identity); + free(nla->pkinitArgs); free(nla->identity); - free(nla); } diff --git a/libfreerdp/core/smartcardlogon.c b/libfreerdp/core/smartcardlogon.c index 42b3c29be..d599cae32 100644 --- a/libfreerdp/core/smartcardlogon.c +++ b/libfreerdp/core/smartcardlogon.c @@ -151,9 +151,13 @@ static void smartcardCertInfoPrivate_Free(SmartcardCertInfoPrivate* scCert) *scCert = empty; } -void smartcardCerts_Free(SmartcardCerts* scCert) +void smartcardCerts_Free(SmartcardCerts** pscCert) { size_t x; + SmartcardCerts* scCert; + + WINPR_ASSERT(pscCert); + scCert = *pscCert; if (!scCert) return; @@ -161,13 +165,17 @@ void smartcardCerts_Free(SmartcardCerts* scCert) smartcardCertInfoPrivate_Free(&scCert->certs[x]); free(scCert); + *pscCert = NULL; } static BOOL treat_sc_cert(SmartcardCertInfo* scCert) { scCert->upn = crypto_cert_get_upn(scCert->certificate->px509); if (!scCert->upn) + { + WLog_DBG(TAG, "%s has no UPN, trying emailAddress", scCert->containerName); scCert->upn = crypto_cert_get_email(scCert->certificate->px509); + } if (scCert->upn) { @@ -280,10 +288,19 @@ static BOOL list_provider_keys(const rdpSettings* settings, NCRYPT_PROV_HANDLE p NULL) <= 0) goto endofloop; + WLog_DBG(TAG, "opening key %s", cert->info.containerName); + status = NCryptOpenKey(provider, &phKey, keyName->pszName, keyName->dwLegacyKeySpec, - keyName->dwFlags); + NCRYPT_SILENT_FLAG); if (status != ERROR_SUCCESS) + { + WLog_DBG(TAG, + "unable to NCryptOpenKey(dwLegacyKeySpec=0x%" PRIx32 " dwFlags=0x%" PRIx32 + "), status=%s, skipping", + status, keyName->dwLegacyKeySpec, keyName->dwFlags, + winpr_NCryptSecurityStatusError(status)); goto endofloop; + } cert->info.csp = _wcsdup(csp); if (!cert->info.csp) @@ -294,7 +311,8 @@ static BOOL list_provider_keys(const rdpSettings* settings, NCRYPT_PROV_HANDLE p &cbOutput, NCRYPT_SILENT_FLAG); if (status != ERROR_SUCCESS) { - WLog_ERR(TAG, "unable to retrieve slotId for key %s", cert->info.containerName); + WLog_ERR(TAG, "unable to retrieve slotId for key %s, status=%s", + cert->info.containerName, winpr_NCryptSecurityStatusError(status)); goto endofloop; } #endif /* _WIN32 */ @@ -335,6 +353,8 @@ static BOOL list_provider_keys(const rdpSettings* settings, NCRYPT_PROV_HANDLE p if (status != ERROR_SUCCESS) { /* can happen that key don't have certificates */ + WLog_DBG(TAG, "unable to retrieve certificate property len, status=0x%lx, skipping", + status); goto endofloop; } @@ -371,7 +391,10 @@ static BOOL list_provider_keys(const rdpSettings* settings, NCRYPT_PROV_HANDLE p } if (!treat_sc_cert(&cert->info)) + { + WLog_DBG(TAG, "error treating cert"); goto endofloop; + } if (userFilter && cert->info.userHint && strcmp(cert->info.userHint, userFilter) != 0) { @@ -522,7 +545,7 @@ static BOOL smartcard_hw_enumerateCerts(const rdpSettings* settings, LPCWSTR csp out: if (!ret) - smartcardCerts_Free(certs); + smartcardCerts_Free(&certs); free(scope); return ret; } @@ -599,6 +622,7 @@ static BOOL smartcard_sw_enumerateCerts(const rdpSettings* settings, SmartcardCe * We need files for PKINIT to read, so write the certificate to some * temporary location and use that. */ + WLog_DBG(TAG, "writing PKINIT cert/key to %s and %s", keyPath, certPath); if (!write_pem(keyPath, freerdp_settings_get_string(settings, FreeRDP_SmartcardPrivateKey))) goto out_error; if (!write_pem(certPath, freerdp_settings_get_string(settings, FreeRDP_SmartcardCertificate))) @@ -616,7 +640,7 @@ static BOOL smartcard_sw_enumerateCerts(const rdpSettings* settings, SmartcardCe out_error: if (!rc) - smartcardCerts_Free(certs); + smartcardCerts_Free(&certs); return rc; } diff --git a/winpr/include/winpr/ncrypt.h b/winpr/include/winpr/ncrypt.h index 8ea8270c7..4b7e6c015 100644 --- a/winpr/include/winpr/ncrypt.h +++ b/winpr/include/winpr/ncrypt.h @@ -202,6 +202,14 @@ extern "C" LPCWSTR pszProviderName, DWORD dwFlags, LPCSTR* modulePaths); + /** + * Gives a string representation of a SECURITY_STATUS + * + * @param status [in] SECURITY_STATUS that we want as string + * @return the string representation of status + */ + WINPR_API const char* winpr_NCryptSecurityStatusError(SECURITY_STATUS status); + #ifdef __cplusplus } #endif diff --git a/winpr/libwinpr/ncrypt/ncrypt.c b/winpr/libwinpr/ncrypt/ncrypt.c index 5143c5fcb..103787551 100644 --- a/winpr/libwinpr/ncrypt/ncrypt.c +++ b/winpr/libwinpr/ncrypt/ncrypt.c @@ -271,4 +271,62 @@ out_free_lib: FreeLibrary(lib); return ret; } -#endif +#endif /* _WIN32 */ + +const char* winpr_NCryptSecurityStatusError(SECURITY_STATUS status) +{ +#define NTE_CASE(S) \ + case S: \ + return #S + + switch (status) + { + NTE_CASE(ERROR_SUCCESS); + NTE_CASE(ERROR_INVALID_PARAMETER); + NTE_CASE(ERROR_INVALID_HANDLE); + NTE_CASE(ERROR_NOT_SUPPORTED); + + NTE_CASE(NTE_BAD_UID); + NTE_CASE(NTE_BAD_HASH); + NTE_CASE(NTE_BAD_KEY); + NTE_CASE(NTE_BAD_LEN); + NTE_CASE(NTE_BAD_DATA); + NTE_CASE(NTE_BAD_SIGNATURE); + NTE_CASE(NTE_BAD_VER); + NTE_CASE(NTE_BAD_ALGID); + NTE_CASE(NTE_BAD_FLAGS); + NTE_CASE(NTE_BAD_TYPE); + NTE_CASE(NTE_BAD_KEY_STATE); + NTE_CASE(NTE_BAD_HASH_STATE); + NTE_CASE(NTE_NO_KEY); + NTE_CASE(NTE_NO_MEMORY); + NTE_CASE(NTE_EXISTS); + NTE_CASE(NTE_PERM); + NTE_CASE(NTE_NOT_FOUND); + NTE_CASE(NTE_DOUBLE_ENCRYPT); + NTE_CASE(NTE_BAD_PROVIDER); + NTE_CASE(NTE_BAD_PROV_TYPE); + NTE_CASE(NTE_BAD_PUBLIC_KEY); + NTE_CASE(NTE_BAD_KEYSET); + NTE_CASE(NTE_PROV_TYPE_NOT_DEF); + NTE_CASE(NTE_PROV_TYPE_ENTRY_BAD); + NTE_CASE(NTE_KEYSET_NOT_DEF); + NTE_CASE(NTE_KEYSET_ENTRY_BAD); + NTE_CASE(NTE_PROV_TYPE_NO_MATCH); + NTE_CASE(NTE_SIGNATURE_FILE_BAD); + NTE_CASE(NTE_PROVIDER_DLL_FAIL); + NTE_CASE(NTE_PROV_DLL_NOT_FOUND); + NTE_CASE(NTE_BAD_KEYSET_PARAM); + NTE_CASE(NTE_FAIL); + NTE_CASE(NTE_SYS_ERR); + NTE_CASE(NTE_SILENT_CONTEXT); + NTE_CASE(NTE_TOKEN_KEYSET_STORAGE_FULL); + NTE_CASE(NTE_TEMPORARY_PROFILE); + NTE_CASE(NTE_FIXEDPARAMETER); + + default: + return ""; + } + +#undef NTE_CASE +}