Warning fixes and assert

This commit is contained in:
Armin Novak 2022-02-14 13:05:59 +01:00 committed by David Fort
parent aac28aaeab
commit a229c1672d
3 changed files with 99 additions and 57 deletions

View File

@ -69,8 +69,11 @@ void* ncrypt_new_handle(NCryptHandleType kind, size_t len, NCryptGetPropertyFn g
return ret;
}
SECURITY_STATUS winpr_NCryptDefault_dtor(NCryptBaseHandle* h)
SECURITY_STATUS winpr_NCryptDefault_dtor(NCRYPT_HANDLE handle)
{
NCryptBaseHandle* h = (NCryptBaseHandle*)handle;
WINPR_ASSERT(h);
memset(h->magic, 0, sizeof(h->magic));
h->type = WINPR_NCRYPT_INVALID;
h->releaseFn = NULL;
@ -136,19 +139,20 @@ SECURITY_STATUS NCryptOpenStorageProvider(NCRYPT_PROV_HANDLE* phProvider, LPCWST
_wcscmp(pszProviderName, MS_SCARD_PROV) == 0)
{
static LPCSTR openscPaths[] = {
"opensc-pkcs11.so", /* In case winpr is installed in system paths */
#ifdef __APPLE__
"/usr/local/lib/pkcs11/opensc-pkcs11.so",
"/usr/local/lib/pkcs11/opensc-pkcs11.so",
#else
/* linux and UNIXes */
#ifdef LIBS64
"/usr/lib/x86_64-linux-gnu/pkcs11/opensc-pkcs11.so", /* Ubuntu/debian */
"/lib64/pkcs11/opensc-pkcs11.so", /* Fedora */
"/usr/lib/x86_64-linux-gnu/pkcs11/opensc-pkcs11.so", /* Ubuntu/debian */
"/lib64/pkcs11/opensc-pkcs11.so", /* Fedora */
#else
"/usr/lib/i386-linux-gnu/opensc-pkcs11.so", /* debian */
"/lib32/pkcs11/opensc-pkcs11.so", /* Fedora */
"/usr/lib/i386-linux-gnu/opensc-pkcs11.so", /* debian */
"/lib32/pkcs11/opensc-pkcs11.so", /* Fedora */
#endif
#endif
NULL
NULL
};
return winpr_NCryptOpenStorageProviderEx(phProvider, pszProviderName, dwFlags, openscPaths);

View File

@ -81,7 +81,7 @@ typedef struct
SECURITY_STATUS checkNCryptHandle(NCRYPT_HANDLE handle, NCryptHandleType matchType);
void* ncrypt_new_handle(NCryptHandleType kind, size_t len, NCryptGetPropertyFn getProp,
NCryptReleaseFn dtor);
SECURITY_STATUS winpr_NCryptDefault_dtor(NCryptBaseHandle* h);
SECURITY_STATUS winpr_NCryptDefault_dtor(NCRYPT_HANDLE handle);
#if defined(WITH_PKCS11)
SECURITY_STATUS NCryptOpenP11StorageProviderEx(NCRYPT_PROV_HANDLE* phProvider,

View File

@ -21,13 +21,17 @@
#include <pkcs11-helper-1.0/pkcs11.h>
#include <winpr/library.h>
#include <winpr/assert.h>
#include <winpr/spec.h>
#include "../log.h"
#include "ncrypt.h"
#define TAG WINPR_TAG("ncryptp11")
#define ARRAY_LENGTH(a) (sizeof(a) / sizeof(a)[0])
#define MAX_SLOTS 64
#define MAX_PRIVATE_KEYS 64
#define MAX_KEYS_PER_SLOT 64
/** @brief ncrypt provider handle */
typedef struct
@ -38,22 +42,15 @@ typedef struct
CK_FUNCTION_LIST_PTR p11;
} NCryptP11ProviderHandle;
static SECURITY_STATUS NCryptP11StorageProvider_dtor(NCryptP11ProviderHandle* provider)
/** @brief a handle returned by NCryptOpenKey */
typedef struct
{
CK_RV rv = provider->p11->C_Finalize(NULL);
if (rv != CKR_OK)
{
}
if (provider->library)
FreeLibrary(provider->library);
return winpr_NCryptDefault_dtor(&provider->baseProvider.baseHandle);
}
#define MAX_SLOTS 64
#define MAX_PRIVATE_KEYS 64
#define MAX_KEYS_PER_SLOT 64
NCryptBaseHandle base;
NCryptP11ProviderHandle* provider;
CK_SLOT_ID slotId;
CK_BYTE keyCertId[64];
CK_ULONG keyCertIdLen;
} NCryptP11KeyHandle;
/** @brief */
typedef struct
@ -86,6 +83,22 @@ static CK_ATTRIBUTE private_key_filter[] = {
{ CKA_KEY_TYPE, &object_ktype_rsa, sizeof(object_ktype_rsa) }
};
static SECURITY_STATUS NCryptP11StorageProvider_dtor(NCRYPT_HANDLE handle)
{
NCryptP11ProviderHandle* provider = (NCryptP11ProviderHandle*)handle;
CK_RV rv;
WINPR_ASSERT(provider);
rv = provider->p11->C_Finalize(NULL);
if (rv != CKR_OK)
{
}
if (provider->library)
FreeLibrary(provider->library);
return winpr_NCryptDefault_dtor(&provider->baseProvider.baseHandle);
}
static void fix_padded_string(char *str, size_t maxlen)
{
@ -97,7 +110,6 @@ static void fix_padded_string(char *str, size_t maxlen)
*ptr = 0;
}
static BOOL attributes_have_unallocated_buffers(CK_ATTRIBUTE_PTR attributes, CK_ULONG count)
{
CK_ULONG i;
@ -159,10 +171,17 @@ static BOOL attributes_allocate_buffers(CK_ATTRIBUTE_PTR attributes, CK_ULONG co
return ret;
}
CK_RV object_load_attributes(NCryptP11ProviderHandle* provider, CK_SESSION_HANDLE session,
CK_OBJECT_HANDLE object, CK_ATTRIBUTE_PTR attributes, CK_ULONG count)
static CK_RV object_load_attributes(NCryptP11ProviderHandle* provider, CK_SESSION_HANDLE session,
CK_OBJECT_HANDLE object, CK_ATTRIBUTE_PTR attributes,
CK_ULONG count)
{
CK_RV rv = provider->p11->C_GetAttributeValue(session, object, attributes, count);
CK_RV rv;
WINPR_ASSERT(provider);
WINPR_ASSERT(provider->p11);
WINPR_ASSERT(provider->p11->C_GetAttributeValue);
rv = provider->p11->C_GetAttributeValue(session, object, attributes, count);
switch (rv)
{
@ -200,15 +219,21 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
CK_RV rv;
CK_SESSION_HANDLE session = (CK_SESSION_HANDLE)NULL;
CK_ULONG i, j, nslotObjects;
CK_OBJECT_HANDLE slotObjects[MAX_KEYS_PER_SLOT];
CK_OBJECT_HANDLE slotObjects[MAX_KEYS_PER_SLOT] = { 0 };
const char* step = NULL;
CK_FUNCTION_LIST_PTR p11 = provider->p11;
CK_FUNCTION_LIST_PTR p11;
WINPR_ASSERT(provider);
p11 = provider->p11;
WINPR_ASSERT(p11);
state->nprivateKeys = 0;
for (i = 0; i < state->nslots; i++)
{
CK_SLOT_INFO slotInfo;
WINPR_ASSERT(p11->C_GetSessionInfo);
rv = p11->C_GetSlotInfo(state->slots[i], &slotInfo);
if (rv != CKR_OK)
{
@ -216,6 +241,7 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
continue;
}
WINPR_ASSERT(p11->C_OpenSession);
rv = p11->C_OpenSession(state->slots[i], CKF_SERIAL_SESSION, NULL, NULL, &session);
if (rv != CKR_OK)
{
@ -224,7 +250,8 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
continue;
}
rv = p11->C_FindObjectsInit(session, private_key_filter, ARRAY_LENGTH(private_key_filter));
WINPR_ASSERT(p11->C_FindObjectsInit);
rv = p11->C_FindObjectsInit(session, private_key_filter, ARRAYSIZE(private_key_filter));
if (rv != CKR_OK)
{
// TODO: shall it be fatal ?
@ -233,7 +260,8 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
goto cleanup_FindObjectsInit;
}
rv = p11->C_FindObjects(session, &slotObjects[0], ARRAY_LENGTH(slotObjects), &nslotObjects);
WINPR_ASSERT(p11->C_FindObjects);
rv = p11->C_FindObjects(session, &slotObjects[0], ARRAYSIZE(slotObjects), &nslotObjects);
if (rv != CKR_OK)
{
WLog_ERR(TAG, "unable to findObjects for slot #%d(%d)", i, state->slots[i]);
@ -253,7 +281,7 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
};
rv = object_load_attributes(provider, session, slotObjects[j], key_or_certAttrs,
ARRAY_LENGTH(key_or_certAttrs));
ARRAYSIZE(key_or_certAttrs));
if (rv != CKR_OK)
{
WLog_ERR(TAG, "error getting attributes");
@ -267,6 +295,7 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
}
cleanup_FindObjects:
WINPR_ASSERT(p11->C_FindObjectsFinal);
rv = p11->C_FindObjectsFinal(session);
if (rv != CKR_OK)
{
@ -274,6 +303,7 @@ static SECURITY_STATUS collect_private_keys(NCryptP11ProviderHandle* provider,
state->slots[i], step);
}
cleanup_FindObjectsInit:
WINPR_ASSERT(p11->C_CloseSession);
rv = p11->C_CloseSession(session);
if (rv != CKR_OK)
{
@ -423,16 +453,6 @@ static SECURITY_STATUS parseKeyName(LPCWSTR pszKeyName, CK_SLOT_ID* slotId, CK_B
return ERROR_SUCCESS;
}
/** @brief a handle returned by NCryptOpenKey */
typedef struct
{
NCryptBaseHandle base;
NCryptP11ProviderHandle* provider;
CK_SLOT_ID slotId;
CK_BYTE keyCertId[64];
CK_ULONG keyCertIdLen;
} NCryptP11KeyHandle;
static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR pszScope,
NCryptKeyName** ppKeyName, PVOID* ppEnumState,
DWORD dwFlags)
@ -440,10 +460,9 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
SECURITY_STATUS ret;
NCryptP11ProviderHandle* provider = (NCryptP11ProviderHandle*)hProvider;
P11EnumKeysState* state = (P11EnumKeysState*)*ppEnumState;
CK_RV rv;
CK_SLOT_ID currentSlot;
CK_RV rv = { 0 };
CK_SLOT_ID currentSlot = { 0 };
CK_SESSION_HANDLE currentSession = (CK_SESSION_HANDLE)NULL;
NCryptKeyName* keyName = NULL;
char slotFilterBuffer[65] = { 0 };
char* slotFilter = NULL;
size_t slotFilterLen = 0;
@ -485,6 +504,7 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
if (!state)
return NTE_NO_MEMORY;
WINPR_ASSERT(provider->p11->C_GetSlotList);
rv = provider->p11->C_GetSlotList(CK_TRUE, NULL, &state->nslots);
if (rv != CKR_OK)
{
@ -515,6 +535,7 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
for (; state->privateKeyIndex < state->nprivateKeys; state->privateKeyIndex++)
{
NCryptKeyName* keyName = NULL;
NCryptPrivateKeyEnum* privKey = &state->privateKeys[state->privateKeyIndex];
CK_OBJECT_CLASS oclass = CKO_CERTIFICATE;
CK_CERTIFICATE_TYPE ctype = CKC_X_509;
@ -534,10 +555,12 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
*/
if (currentSession)
{
WINPR_ASSERT(provider->p11->C_CloseSession);
rv = provider->p11->C_CloseSession(currentSession);
currentSession = (CK_SESSION_HANDLE)NULL;
}
WINPR_ASSERT(provider->p11->C_OpenSession);
rv = provider->p11->C_OpenSession(privKey->slotId, CKF_SERIAL_SESSION, NULL, NULL,
&currentSession);
if (rv != CKR_OK)
@ -549,14 +572,16 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
}
/* look if we can find a certificate that matches the private key's id */
WINPR_ASSERT(provider->p11->C_FindObjectsInit);
rv = provider->p11->C_FindObjectsInit(currentSession, certificateFilter,
ARRAY_LENGTH(certificateFilter));
ARRAYSIZE(certificateFilter));
if (rv != CKR_OK)
{
WLog_ERR(TAG, "unable to initiate search for slot %d", privKey->slotId);
continue;
}
WINPR_ASSERT(provider->p11->C_FindObjects);
rv = provider->p11->C_FindObjects(currentSession, &certObject, 1, &ncertObjects);
if (rv != CKR_OK)
{
@ -566,8 +591,8 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
if (ncertObjects)
{
/* sizeof keyName struct + "\<slotId>\<certId>" */
#define KEYNAME_SZ (1 + 8 /*slotId*/ + 1 + (privKey->idLen * 2) + 1) * 2
/* sizeof keyName struct + "\<slotId>\<certId>" */
const size_t KEYNAME_SZ = (1 + 8 /*slotId*/ + 1 + (privKey->idLen * 2) + 1) * 2;
keyName = calloc(1, sizeof(*keyName) + KEYNAME_SZ);
if (!keyName)
{
@ -582,6 +607,7 @@ static SECURITY_STATUS NCryptP11EnumKeys(NCRYPT_PROV_HANDLE hProvider, LPCWSTR p
}
cleanup_FindObjects:
WINPR_ASSERT(provider->p11->C_FindObjectsFinal);
rv = provider->p11->C_FindObjectsFinal(currentSession);
if (keyName)
@ -604,7 +630,7 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
CK_SESSION_HANDLE session;
CK_OBJECT_HANDLE objectHandle;
CK_ULONG objectCount;
NCryptP11ProviderHandle* provider = keyHandle->provider;
NCryptP11ProviderHandle* provider;
CK_OBJECT_CLASS oclass = CKO_CERTIFICATE;
CK_CERTIFICATE_TYPE ctype = CKC_X_509;
CK_ATTRIBUTE certificateFilter[] = { { CKA_CLASS, &oclass, sizeof(oclass) },
@ -612,7 +638,11 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
{ CKA_ID, keyHandle->keyCertId,
keyHandle->keyCertIdLen } };
CK_ATTRIBUTE* objectFilter = certificateFilter;
CK_ULONG objectFilterLen = ARRAY_LENGTH(certificateFilter);
CK_ULONG objectFilterLen = ARRAYSIZE(certificateFilter);
WINPR_ASSERT(keyHandle);
provider = keyHandle->provider;
WINPR_ASSERT(provider);
switch (property)
{
@ -621,6 +651,7 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
case NCRYPT_PROPERTY_READER: {
CK_SLOT_INFO slotInfo;
WINPR_ASSERT(provider->p11->C_GetSlotInfo);
rv = provider->p11->C_GetSlotInfo(keyHandle->slotId, &slotInfo);
if (rv != CKR_OK)
return NTE_BAD_KEY;
@ -643,6 +674,7 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
return NTE_NOT_SUPPORTED;
}
WINPR_ASSERT(provider->p11->C_OpenSession);
rv = provider->p11->C_OpenSession(keyHandle->slotId, CKF_SERIAL_SESSION, NULL, NULL, &session);
if (rv != CKR_OK)
{
@ -650,6 +682,7 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
return NTE_FAIL;
}
WINPR_ASSERT(provider->p11->C_FindObjectsInit);
rv = provider->p11->C_FindObjectsInit(session, objectFilter, objectFilterLen);
if (rv != CKR_OK)
{
@ -657,6 +690,7 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
goto out;
}
WINPR_ASSERT(provider->p11->C_FindObjects);
rv = provider->p11->C_FindObjects(session, &objectHandle, 1, &objectCount);
if (rv != CKR_OK)
{
@ -675,6 +709,7 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
{
CK_ATTRIBUTE certValue = { CKA_VALUE, pbOutput, cbOutput };
WINPR_ASSERT(provider->p11->C_GetAttributeValue);
rv = provider->p11->C_GetAttributeValue(session, objectHandle, &certValue, 1);
if (rv != CKR_OK)
{
@ -691,12 +726,14 @@ static SECURITY_STATUS NCryptP11KeyGetProperties(NCryptP11KeyHandle* keyHandle,
}
out_final:
WINPR_ASSERT(provider->p11->C_FindObjectsFinal);
rv = provider->p11->C_FindObjectsFinal(session);
if (rv != CKR_OK)
{
WLog_ERR(TAG, "error in C_FindObjectsFinal() for slot %d", keyHandle->slotId);
}
out:
WINPR_ASSERT(provider->p11->C_CloseSession);
rv = provider->p11->C_CloseSession(session);
if (rv != CKR_OK)
{
@ -711,6 +748,7 @@ static SECURITY_STATUS NCryptP11GetProperty(NCRYPT_HANDLE hObject, NCryptKeyGetP
{
NCryptBaseHandle* base = (NCryptBaseHandle*)hObject;
WINPR_ASSERT(base);
switch (base->type)
{
case WINPR_NCRYPT_PROVIDER:
@ -729,7 +767,7 @@ static SECURITY_STATUS NCryptP11OpenKey(NCRYPT_PROV_HANDLE hProvider, NCRYPT_KEY
{
SECURITY_STATUS ret;
CK_SLOT_ID slotId;
CK_BYTE keyCertId[64];
CK_BYTE keyCertId[64] = { 0 };
CK_ULONG keyCertIdLen;
NCryptP11KeyHandle* keyHandle;
@ -737,9 +775,8 @@ static SECURITY_STATUS NCryptP11OpenKey(NCRYPT_PROV_HANDLE hProvider, NCRYPT_KEY
if (ret != ERROR_SUCCESS)
return ret;
keyHandle = (NCryptP11KeyHandle*)ncrypt_new_handle(WINPR_NCRYPT_KEY, sizeof(*keyHandle),
(NCryptGetPropertyFn)NCryptP11GetProperty,
(NCryptReleaseFn)winpr_NCryptDefault_dtor);
keyHandle = (NCryptP11KeyHandle*)ncrypt_new_handle(
WINPR_NCRYPT_KEY, sizeof(*keyHandle), NCryptP11GetProperty, winpr_NCryptDefault_dtor);
if (!keyHandle)
return NTE_NO_MEMORY;
@ -760,12 +797,13 @@ SECURITY_STATUS NCryptOpenP11StorageProviderEx(NCRYPT_PROV_HANDLE* phProvider,
CK_RV rv;
CK_RV (*c_get_function_list)(CK_FUNCTION_LIST_PTR_PTR);
WINPR_ASSERT(modulePaths);
if (!phProvider)
return ERROR_INVALID_PARAMETER;
ret = (NCryptP11ProviderHandle*)ncrypt_new_handle(
WINPR_NCRYPT_PROVIDER, sizeof(*ret), (NCryptGetPropertyFn)NCryptP11GetProperty,
(NCryptReleaseFn)NCryptP11StorageProvider_dtor);
WINPR_NCRYPT_PROVIDER, sizeof(*ret), NCryptP11GetProperty, NCryptP11StorageProvider_dtor);
if (!ret)
return NTE_NO_MEMORY;