diff --git a/libfreerdp/core/smartcardlogon.c b/libfreerdp/core/smartcardlogon.c index a42114834..b81976220 100644 --- a/libfreerdp/core/smartcardlogon.c +++ b/libfreerdp/core/smartcardlogon.c @@ -537,6 +537,15 @@ static BOOL list_provider_keys(const rdpSettings* settings, NCRYPT_PROV_HANDLE p ret = TRUE; out: + if (count == 0) + { + char cspa[128] = { 0 }; + + ConvertWCharToUtf8(csp, cspa, sizeof(cspa)); + char scopea[128] = { 0 }; + ConvertWCharToUtf8(scope, scopea, sizeof(scopea)); + WLog_WARN(TAG, "%s [%s] no certificates found", cspa, scopea); + } *pcount = count; *pcerts = cert_list; NCryptFreeBuffer(enumState); diff --git a/winpr/libwinpr/ncrypt/ncrypt.c b/winpr/libwinpr/ncrypt/ncrypt.c index cbd953af3..10feba4aa 100644 --- a/winpr/libwinpr/ncrypt/ncrypt.c +++ b/winpr/libwinpr/ncrypt/ncrypt.c @@ -33,13 +33,24 @@ const static char NCRYPT_MAGIC[6] = { 'N', 'C', 'R', 'Y', 'P', 'T' }; SECURITY_STATUS checkNCryptHandle(NCRYPT_HANDLE handle, NCryptHandleType matchType) { - NCryptBaseHandle* base; if (!handle) + { + WLog_VRB(TAG, "invalid handle '%p'", handle); return ERROR_INVALID_PARAMETER; + } - base = (NCryptBaseHandle*)handle; - if (memcmp(base->magic, NCRYPT_MAGIC, 6) != 0) + const NCryptBaseHandle* base = (NCryptBaseHandle*)handle; + if (memcmp(base->magic, NCRYPT_MAGIC, ARRAYSIZE(NCRYPT_MAGIC)) != 0) + { + char magic1[ARRAYSIZE(NCRYPT_MAGIC) + 1] = { 0 }; + char magic2[ARRAYSIZE(NCRYPT_MAGIC) + 1] = { 0 }; + + memcpy(magic1, base->magic, ARRAYSIZE(NCRYPT_MAGIC)); + memcpy(magic2, NCRYPT_MAGIC, ARRAYSIZE(NCRYPT_MAGIC)); + + WLog_VRB(TAG, "handle '%p' invalid magic '%s' instead of '%s'", base, magic1, magic2); return ERROR_INVALID_PARAMETER; + } switch (base->type) { @@ -47,11 +58,15 @@ SECURITY_STATUS checkNCryptHandle(NCRYPT_HANDLE handle, NCryptHandleType matchTy case WINPR_NCRYPT_KEY: break; default: + WLog_VRB(TAG, "handle '%p' invalid type %d", base, base->type); return ERROR_INVALID_PARAMETER; } - if (matchType != WINPR_NCRYPT_INVALID && base->type != matchType) + if ((matchType != WINPR_NCRYPT_INVALID) && (base->type != matchType)) + { + WLog_VRB(TAG, "handle '%p' invalid type %d, expected %d", base, base->type, matchType); return ERROR_INVALID_PARAMETER; + } return ERROR_SUCCESS; } @@ -129,30 +144,26 @@ SECURITY_STATUS NCryptEnumStorageProviders(DWORD* wProviderCount, SECURITY_STATUS NCryptOpenStorageProvider(NCRYPT_PROV_HANDLE* phProvider, LPCWSTR pszProviderName, DWORD dwFlags) { - -#ifdef WITH_PKCS11 - if (pszProviderName && (_wcscmp(pszProviderName, MS_SMART_CARD_KEY_STORAGE_PROVIDER) == 0 || - _wcscmp(pszProviderName, MS_SCARD_PROV) == 0)) - { - return winpr_NCryptOpenStorageProviderEx(phProvider, pszProviderName, dwFlags, NULL); - } -#endif - - return ERROR_NOT_SUPPORTED; + return winpr_NCryptOpenStorageProviderEx(phProvider, pszProviderName, dwFlags, NULL); } SECURITY_STATUS winpr_NCryptOpenStorageProviderEx(NCRYPT_PROV_HANDLE* phProvider, LPCWSTR pszProviderName, DWORD dwFlags, LPCSTR* modulePaths) { -#ifdef WITH_PKCS11 - if (pszProviderName && (_wcscmp(pszProviderName, MS_SMART_CARD_KEY_STORAGE_PROVIDER) == 0 || - _wcscmp(pszProviderName, MS_SCARD_PROV) == 0)) - { +#if defined(WITH_PKCS11) + if (pszProviderName && ((_wcscmp(pszProviderName, MS_SMART_CARD_KEY_STORAGE_PROVIDER) == 0) || + (_wcscmp(pszProviderName, MS_SCARD_PROV) == 0))) return NCryptOpenP11StorageProviderEx(phProvider, pszProviderName, dwFlags, modulePaths); - } -#endif + + char buffer[128] = { 0 }; + ConvertWCharToUtf8(pszProviderName, buffer, sizeof(buffer)); + WLog_WARN(TAG, "provider '%s' not supported", buffer); return ERROR_NOT_SUPPORTED; +#else + WLog_WARN(TAG, "rebuild with -DWITH_PKCS11=ON to enable smartcard logon support"); + return ERROR_NOT_SUPPORTED; +#endif } SECURITY_STATUS NCryptEnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR pszScope, diff --git a/winpr/libwinpr/ncrypt/ncrypt_pkcs11.c b/winpr/libwinpr/ncrypt/ncrypt_pkcs11.c index e137938ed..ca7fd1250 100644 --- a/winpr/libwinpr/ncrypt/ncrypt_pkcs11.c +++ b/winpr/libwinpr/ncrypt/ncrypt_pkcs11.c @@ -334,26 +334,24 @@ static const char* CK_RV_error_string(CK_RV rv) static SECURITY_STATUS collect_keys(NCryptP11ProviderHandle* provider, P11EnumKeysState* state) { - CK_RV rv; - CK_ULONG i, j, nslotObjects; CK_OBJECT_HANDLE slotObjects[MAX_KEYS_PER_SLOT] = { 0 }; const char* step = NULL; - CK_FUNCTION_LIST_PTR p11; WINPR_ASSERT(provider); - p11 = provider->p11; + CK_FUNCTION_LIST_PTR p11 = provider->p11; WINPR_ASSERT(p11); + WLog_DBG(TAG, "checking %" PRIu32 " slots for valid keys...", state->nslots); state->nKeys = 0; - for (i = 0; i < state->nslots; i++) + for (CK_ULONG i = 0; i < state->nslots; i++) { CK_SESSION_HANDLE session = (CK_SESSION_HANDLE)NULL; - CK_SLOT_INFO slotInfo; - CK_TOKEN_INFO tokenInfo; + CK_SLOT_INFO slotInfo = { 0 }; + CK_TOKEN_INFO tokenInfo = { 0 }; WINPR_ASSERT(p11->C_GetSlotInfo); - rv = p11->C_GetSlotInfo(state->slots[i], &slotInfo); + CK_RV rv = p11->C_GetSlotInfo(state->slots[i], &slotInfo); if (rv != CKR_OK) { WLog_ERR(TAG, "unable to retrieve information for slot #%d(%d)", i, state->slots[i]); @@ -404,6 +402,7 @@ static SECURITY_STATUS collect_keys(NCryptP11ProviderHandle* provider, P11EnumKe goto cleanup_FindObjectsInit; } + CK_ULONG nslotObjects = 0; WINPR_ASSERT(p11->C_FindObjects); rv = p11->C_FindObjects(session, &slotObjects[0], ARRAYSIZE(slotObjects), &nslotObjects); if (rv != CKR_OK) @@ -415,7 +414,7 @@ static SECURITY_STATUS collect_keys(NCryptP11ProviderHandle* provider, P11EnumKe } WLog_DBG(TAG, "slot has %d objects", nslotObjects); - for (j = 0; j < nslotObjects; j++) + for (CK_ULONG j = 0; j < nslotObjects; j++) { NCryptKeyEnum* key = &state->keys[state->nKeys]; CK_OBJECT_CLASS dataClass = CKO_PUBLIC_KEY; @@ -638,7 +637,6 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p NCryptKeyName** ppKeyName, PVOID* ppEnumState, DWORD dwFlags) { - SECURITY_STATUS ret; NCryptP11ProviderHandle* provider = (NCryptP11ProviderHandle*)hProvider; P11EnumKeysState* state = (P11EnumKeysState*)*ppEnumState; CK_RV rv = { 0 }; @@ -648,7 +646,7 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p char* slotFilter = NULL; size_t slotFilterLen = 0; - ret = checkNCryptHandle((NCRYPT_HANDLE)hProvider, WINPR_NCRYPT_PROVIDER); + SECURITY_STATUS ret = checkNCryptHandle((NCRYPT_HANDLE)hProvider, WINPR_NCRYPT_PROVIDER); if (ret != ERROR_SUCCESS) return ret; @@ -658,18 +656,27 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p * check whether pszScope is of the form \\.\\ for filtering by * card reader */ - char asciiScope[128 + 6] = { 0 }; + char asciiScope[128 + 6 + 1] = { 0 }; size_t asciiScopeLen; - if (ConvertWCharToUtf8(pszScope, asciiScope, ARRAYSIZE(asciiScope)) < 0) + if (ConvertWCharToUtf8(pszScope, asciiScope, ARRAYSIZE(asciiScope) - 1) < 0) + { + WLog_WARN(TAG, "Invalid scope"); return NTE_INVALID_PARAMETER; + } if (strstr(asciiScope, "\\\\.\\") != asciiScope) + { + WLog_WARN(TAG, "Invalid scope '%s'", asciiScope); return NTE_INVALID_PARAMETER; + } - asciiScopeLen = strnlen(asciiScope, sizeof(asciiScope)); - if (asciiScope[asciiScopeLen - 1] != '\\') + asciiScopeLen = strnlen(asciiScope, ARRAYSIZE(asciiScope)); + if ((asciiScopeLen < 1) || (asciiScope[asciiScopeLen - 1] != '\\')) + { + WLog_WARN(TAG, "Invalid scope '%s'", asciiScope); return NTE_INVALID_PARAMETER; + } asciiScope[asciiScopeLen - 1] = 0; @@ -689,6 +696,7 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p if (rv != CKR_OK) { /* TODO: perhaps convert rv to NTE_*** errors */ + WLog_WARN(TAG, "C_GetSlotList failed with %u", rv); return NTE_FAIL; } @@ -700,6 +708,7 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p { free(state); /* TODO: perhaps convert rv to NTE_*** errors */ + WLog_WARN(TAG, "C_GetSlotList failed with %u", rv); return NTE_FAIL; }