Skip to content

Commit 7b4b61f

Browse files
JacobBarthelmehJacobBarthelmeh
committed
use word32 type instead of DWORD and fix for memory management
1 parent b700995 commit 7b4b61f

13 files changed

Lines changed: 123 additions & 60 deletions

File tree

‎apps/wolfsshd/configuration.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ enum {
435435
#endif /* USE_WINDOWS_API */
436436
};
437437
enum {
438-
NUM_OPTIONS = 31
438+
NUM_OPTIONS = 28
439439
#ifdef USE_WINDOWS_API
440440
+ 3
441441
#endif /* USE_WINDOWS_API */
@@ -1809,6 +1809,8 @@ int wolfSSHD_ConfigSetWinUserStores(WOLFSSHD_CONFIG* conf, const char* value)
18091809
}
18101810

18111811
if (ret == WS_SUCCESS) {
1812+
/* free any previously set value before replacing it */
1813+
FreeString(&conf->winUserStores, conf->heap);
18121814
ret = CreateString(&conf->winUserStores, value,
18131815
(int)WSTRLEN(value), conf->heap);
18141816
}
@@ -1843,6 +1845,8 @@ int wolfSSHD_ConfigSetWinUserDwFlags(WOLFSSHD_CONFIG* conf, const char* value)
18431845
}
18441846

18451847
if (ret == WS_SUCCESS) {
1848+
/* free any previously set value before replacing it */
1849+
FreeString(&conf->winUserDwFlags, conf->heap);
18461850
ret = CreateString(&conf->winUserDwFlags, value,
18471851
(int)WSTRLEN(value), conf->heap);
18481852
}
@@ -1874,6 +1878,8 @@ int wolfSSHD_ConfigSetWinUserPvPara(WOLFSSHD_CONFIG* conf, const char* value)
18741878
}
18751879

18761880
if (ret == WS_SUCCESS) {
1881+
/* free any previously set value before replacing it */
1882+
FreeString(&conf->winUserPvPara, conf->heap);
18771883
ret = CreateString(&conf->winUserPvPara, value,
18781884
(int)WSTRLEN(value), conf->heap);
18791885
}

‎apps/wolfsshd/wolfsshd.c‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -400,41 +400,51 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx,
400400
/* Use cert store host key */
401401
wchar_t* wStoreName = NULL;
402402
wchar_t* wSubjectName = NULL;
403-
DWORD dwFlags = CERT_SYSTEM_STORE_CURRENT_USER;
403+
word32 dwFlags = CERT_SYSTEM_STORE_CURRENT_USER;
404404
int storeNameLen, subjectNameLen;
405-
405+
406406
/* Parse flags if provided */
407407
if (hostKeyStoreFlags != NULL) {
408408
if (WSTRCMP(hostKeyStoreFlags, "CURRENT_USER") == 0) {
409409
dwFlags = CERT_SYSTEM_STORE_CURRENT_USER;
410410
} else if (WSTRCMP(hostKeyStoreFlags, "LOCAL_MACHINE") == 0) {
411411
dwFlags = CERT_SYSTEM_STORE_LOCAL_MACHINE;
412412
} else {
413-
dwFlags = (DWORD)atoi(hostKeyStoreFlags);
413+
dwFlags = (word32)atoi(hostKeyStoreFlags);
414414
}
415415
}
416-
416+
417417
/* Convert to wide strings */
418418
storeNameLen = MultiByteToWideChar(CP_UTF8, 0, hostKeyStore, -1, NULL, 0);
419419
subjectNameLen = MultiByteToWideChar(CP_UTF8, 0, hostKeyStoreSubject, -1, NULL, 0);
420-
421-
wStoreName = (wchar_t*)WMALLOC(storeNameLen * sizeof(wchar_t), heap, DYNTYPE_SSHD);
422-
wSubjectName = (wchar_t*)WMALLOC(subjectNameLen * sizeof(wchar_t), heap, DYNTYPE_SSHD);
423-
424-
if (wStoreName == NULL || wSubjectName == NULL) {
425-
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Memory allocation failed for cert store strings");
426-
ret = WS_MEMORY_E;
420+
421+
if (storeNameLen == 0 || subjectNameLen == 0) {
422+
wolfSSH_Log(WS_LOG_ERROR,
423+
"[SSHD] Failed to convert cert store strings to wide characters");
424+
ret = WS_BAD_ARGUMENT;
427425
} else {
428-
MultiByteToWideChar(CP_UTF8, 0, hostKeyStore, -1, wStoreName, storeNameLen);
429-
MultiByteToWideChar(CP_UTF8, 0, hostKeyStoreSubject, -1, wSubjectName, subjectNameLen);
430-
431-
ret = wolfSSH_CTX_UsePrivateKey_fromStore(*ctx, wStoreName, dwFlags, wSubjectName);
432-
if (ret != WS_SUCCESS) {
433-
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to load host key from certificate store");
426+
wStoreName = (wchar_t*)WMALLOC(storeNameLen * sizeof(wchar_t), heap, DYNTYPE_SSHD);
427+
wSubjectName = (wchar_t*)WMALLOC(subjectNameLen * sizeof(wchar_t), heap, DYNTYPE_SSHD);
428+
429+
if (wStoreName == NULL || wSubjectName == NULL) {
430+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Memory allocation failed for cert store strings");
431+
ret = WS_MEMORY_E;
432+
} else {
433+
MultiByteToWideChar(CP_UTF8, 0, hostKeyStore, -1, wStoreName, storeNameLen);
434+
MultiByteToWideChar(CP_UTF8, 0, hostKeyStoreSubject, -1, wSubjectName, subjectNameLen);
435+
436+
ret = wolfSSH_CTX_UsePrivateKey_fromStore(*ctx, wStoreName, dwFlags, wSubjectName);
437+
if (ret != WS_SUCCESS) {
438+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to load host key from certificate store");
439+
}
440+
}
441+
442+
if (wStoreName != NULL) {
443+
WFREE(wStoreName, heap, DYNTYPE_SSHD);
444+
}
445+
if (wSubjectName != NULL) {
446+
WFREE(wSubjectName, heap, DYNTYPE_SSHD);
434447
}
435-
436-
WFREE(wStoreName, heap, DYNTYPE_SSHD);
437-
WFREE(wSubjectName, heap, DYNTYPE_SSHD);
438448
}
439449
} else
440450
#elif defined(WOLFSSH_CERTS)

‎configure.ac‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,9 @@ AS_IF([test "x$ENABLED_CERTS" = "xyes"],
285285
AS_IF([test "x$ENABLED_WINDOWS_CERT_STORE" = "xyes"],
286286
[AS_IF([test "x$ENABLED_CERTS" != "xyes"],
287287
[AC_MSG_ERROR([--enable-windows-cert-store requires X.509 cert support (--enable-certs)])])
288-
AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_WINDOWS_CERT_STORE"])
288+
AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_WINDOWS_CERT_STORE"
289+
AS_CASE([$host],
290+
[*mingw*|*msys*|*cygwin*],[LIBS="$LIBS -lcrypt32 -lncrypt"])])
289291
AS_IF([test "x$ENABLED_SMALLSTACK" = "xyes"],
290292
[AM_CPPFLAGS="$AM_CPPFLAGS -DWOLFSSH_SMALL_STACK"])
291293
AS_IF([test "x$ENABLED_SSHCLIENT" = "xyes"],

‎examples/client/common.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1170,7 +1170,7 @@ void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName,
11701170

11711171
#ifdef WOLFSSH_WINDOWS_CERT_STORE
11721172
int ClientSetPrivateKeyFromStore(WOLFSSH_CTX* ctx,
1173-
const wchar_t* storeName, DWORD dwFlags, const wchar_t* subjectName)
1173+
const wchar_t* storeName, word32 dwFlags, const wchar_t* subjectName)
11741174
{
11751175
int ret = WS_SUCCESS;
11761176

‎examples/client/common.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ int ClientSetTpm(WOLFSSH* ssh);
3737
#endif
3838
#ifdef WOLFSSH_WINDOWS_CERT_STORE
3939
int ClientSetPrivateKeyFromStore(WOLFSSH_CTX* ctx,
40-
const wchar_t* storeName, DWORD dwFlags, const wchar_t* subjectName);
40+
const wchar_t* storeName, word32 dwFlags, const wchar_t* subjectName);
4141
int ClientSetupCertStoreAuth(WOLFSSH_CTX* ctx);
4242
#endif /* WOLFSSH_WINDOWS_CERT_STORE */
4343

‎examples/echoserver/echoserver.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3293,7 +3293,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args)
32933293
/* Load host key from Windows certificate store */
32943294
wchar_t* wStoreName = NULL;
32953295
wchar_t* wSubjectName = NULL;
3296-
DWORD dwFlags = 0;
3296+
word32 dwFlags = 0;
32973297
int ret;
32983298

32993299
ret = wolfSSH_ParseCertStoreSpec(certStoreSpec, &wStoreName,

‎examples/sftpclient/sftpclient.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1710,7 +1710,7 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args)
17101710
if (certStoreSpec != NULL) {
17111711
wchar_t* wStoreName = NULL;
17121712
wchar_t* wSubjectName = NULL;
1713-
DWORD dwFlags = 0;
1713+
word32 dwFlags = 0;
17141714

17151715
ret = wolfSSH_ParseCertStoreSpec(certStoreSpec, &wStoreName,
17161716
&wSubjectName, &dwFlags, NULL);

‎src/certman.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ static int CheckProfile(DecodedCert* cert, int profile)
629629
* Returns WS_SUCCESS on success. */
630630
int wolfSSH_ParseCertStoreSpec(const char* spec,
631631
wchar_t** wStoreName, wchar_t** wSubjectName,
632-
DWORD* dwFlags, void* heap)
632+
word32* dwFlags, void* heap)
633633
{
634634
char* specCopy = NULL;
635635
char* storeName = NULL;
@@ -668,7 +668,7 @@ int wolfSSH_ParseCertStoreSpec(const char* spec,
668668
*dwFlags = CERT_SYSTEM_STORE_LOCAL_MACHINE;
669669
}
670670
else {
671-
*dwFlags = (DWORD)atoi(flagsStr);
671+
*dwFlags = (word32)atoi(flagsStr);
672672
}
673673
}
674674
}

‎src/internal.c‎

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14032,13 +14032,19 @@ static int SignWithCertStoreKey(WOLFSSH* ssh,
1403214032

1403314033
pCertContext = (PCCERT_CONTEXT)pvtKey->certStoreContext;
1403414034

14035-
/* Get the private key handle from the certificate */
14035+
/* Get the private key handle from the certificate. Prefer CNG/NCRYPT,
14036+
* but fall back to legacy CryptoAPI providers so the non-NCRYPT signing
14037+
* path below remains reachable for CSP-backed keys. */
1403614038
if (!CryptAcquireCertificatePrivateKey(pCertContext,
1403714039
CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG | CRYPT_ACQUIRE_SILENT_FLAG,
1403814040
NULL, &hCryptProv, &dwKeySpec, &fCallerFreeProv)) {
14039-
DWORD dwErr = GetLastError();
14040-
WLOG(WS_LOG_DEBUG, "SignWithCertStoreKey: Failed to acquire private key, error: %lu", dwErr);
14041-
return WS_CRYPTO_FAILED;
14041+
if (!CryptAcquireCertificatePrivateKey(pCertContext,
14042+
CRYPT_ACQUIRE_SILENT_FLAG,
14043+
NULL, &hCryptProv, &dwKeySpec, &fCallerFreeProv)) {
14044+
DWORD dwErr = GetLastError();
14045+
WLOG(WS_LOG_DEBUG, "SignWithCertStoreKey: Failed to acquire private key, error: %lu", dwErr);
14046+
return WS_CRYPTO_FAILED;
14047+
}
1404214048
}
1404314049

1404414050
/* Sign using CNG (Next Generation Crypto API) */
@@ -17299,23 +17305,38 @@ static int BuildUserAuthRequestEccCert(WOLFSSH* ssh,
1729917305
if (ret == WS_SUCCESS) {
1730017306
/* NCryptSignHash ECDSA output is raw r||s, each
1730117307
* component is half the total signature size. */
17302-
word32 halfSz = sigSz / 2;
17308+
word32 halfSz;
1730317309
word32 rOff = 0, sOff = 0;
17304-
r = rs;
17305-
s = rs + halfSz;
17306-
WMEMCPY(r, sig, halfSz);
17307-
WMEMCPY(s, sig + halfSz, halfSz);
17308-
/* Trim leading zeroes */
17309-
while (rOff < halfSz - 1 && r[rOff] == 0) rOff++;
17310-
while (sOff < halfSz - 1 && s[sOff] == 0) sOff++;
17311-
if (rOff > 0) {
17312-
WMEMMOVE(r, r + rOff, halfSz - rOff);
17310+
17311+
if (sigSz < 2 || (sigSz & 1) != 0) {
17312+
WLOG(WS_LOG_DEBUG,
17313+
"SUAR: Invalid cert store ECC signature size");
17314+
ret = WS_ECC_E;
1731317315
}
17314-
rSz = halfSz - rOff;
17315-
if (sOff > 0) {
17316-
WMEMMOVE(s, s + sOff, halfSz - sOff);
17316+
halfSz = sigSz / 2;
17317+
if (ret == WS_SUCCESS &&
17318+
halfSz > (word32)sizeof(rs) / 2) {
17319+
WLOG(WS_LOG_DEBUG,
17320+
"SUAR: Cert store ECC signature too large");
17321+
ret = WS_ECC_E;
17322+
}
17323+
if (ret == WS_SUCCESS) {
17324+
r = rs;
17325+
s = rs + halfSz;
17326+
WMEMCPY(r, sig, halfSz);
17327+
WMEMCPY(s, sig + halfSz, halfSz);
17328+
/* Trim leading zeroes */
17329+
while (rOff < halfSz - 1 && r[rOff] == 0) rOff++;
17330+
while (sOff < halfSz - 1 && s[sOff] == 0) sOff++;
17331+
if (rOff > 0) {
17332+
WMEMMOVE(r, r + rOff, halfSz - rOff);
17333+
}
17334+
rSz = halfSz - rOff;
17335+
if (sOff > 0) {
17336+
WMEMMOVE(s, s + sOff, halfSz - sOff);
17337+
}
17338+
sSz = halfSz - sOff;
1731717339
}
17318-
sSz = halfSz - sOff;
1731917340
} else {
1732017341
WLOG(WS_LOG_DEBUG, "SUAR: Cert store ECC sign failed");
1732117342
ret = WS_ECC_E;

‎src/ssh.c‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include <wincrypt.h>
4141
#include <ncrypt.h>
4242
#include <string.h>
43+
#include <wchar.h>
4344
#ifndef CERT_NCRYPT_KEY_SPEC
4445
#define CERT_NCRYPT_KEY_SPEC 0x00000003
4546
#endif
@@ -2694,7 +2695,7 @@ int wolfSSH_CTX_AddRootCert_buffer(WOLFSSH_CTX* ctx,
26942695
* returns WS_SUCCESS on success
26952696
*/
26962697
int wolfSSH_CTX_UsePrivateKey_fromStore(WOLFSSH_CTX* ctx,
2697-
const wchar_t* storeName, DWORD dwFlags,
2698+
const wchar_t* storeName, word32 dwFlags,
26982699
const wchar_t* subjectName)
26992700
{
27002701
int ret = WS_SUCCESS;
@@ -2716,7 +2717,7 @@ int wolfSSH_CTX_UsePrivateKey_fromStore(WOLFSSH_CTX* ctx,
27162717

27172718
/* Open the certificate store */
27182719
hStore = CertOpenStore(CERT_STORE_PROV_SYSTEM_W, 0, (HCRYPTPROV_LEGACY)0,
2719-
dwFlags | CERT_STORE_OPEN_EXISTING_FLAG, storeName);
2720+
(DWORD)dwFlags | CERT_STORE_OPEN_EXISTING_FLAG, storeName);
27202721
if (hStore == NULL) {
27212722
DWORD dwErr = GetLastError();
27222723
WLOG(WS_LOG_DEBUG, "wolfSSH_CTX_UsePrivateKey_fromStore: Failed to open store, error: %lu", dwErr);
@@ -2746,8 +2747,6 @@ int wolfSSH_CTX_UsePrivateKey_fromStore(WOLFSSH_CTX* ctx,
27462747
}
27472748

27482749
if (pCertContext == NULL) {
2749-
/* Try finding by thumbprint if subject name didn't work */
2750-
/* Note: subjectName could be a thumbprint in format "XX XX XX ..." */
27512750
CertCloseStore(hStore, 0);
27522751
WLOG(WS_LOG_ERROR, "wolfSSH_CTX_UsePrivateKey_fromStore: Certificate "
27532752
"not found with subject '%ls'", subjectName);
@@ -2837,17 +2836,34 @@ int wolfSSH_CTX_UsePrivateKey_fromStore(WOLFSSH_CTX* ctx,
28372836
return WS_MEMORY_E;
28382837
}
28392838

2840-
/* Free existing resources if replacing an existing slot */
2839+
/* Free existing resources if replacing an existing slot. The slot may
2840+
* previously have held either a cert-store key or a file-based
2841+
* key/cert, so clear both kinds of resources. */
28412842
if (ctx->privateKey[keyIdx].useCertStore) {
2842-
if (ctx->privateKey[keyIdx].certStoreContext != NULL)
2843+
if (ctx->privateKey[keyIdx].certStoreContext != NULL) {
28432844
CertFreeCertificateContext(
28442845
(PCCERT_CONTEXT)ctx->privateKey[keyIdx].certStoreContext);
2845-
if (ctx->privateKey[keyIdx].storeName != NULL)
2846+
ctx->privateKey[keyIdx].certStoreContext = NULL;
2847+
}
2848+
if (ctx->privateKey[keyIdx].storeName != NULL) {
28462849
WFREE(ctx->privateKey[keyIdx].storeName, heap, DYNTYPE_STRING);
2847-
if (ctx->privateKey[keyIdx].subjectName != NULL)
2850+
ctx->privateKey[keyIdx].storeName = NULL;
2851+
}
2852+
if (ctx->privateKey[keyIdx].subjectName != NULL) {
28482853
WFREE(ctx->privateKey[keyIdx].subjectName, heap, DYNTYPE_STRING);
2849-
if (ctx->privateKey[keyIdx].cert != NULL)
2850-
WFREE(ctx->privateKey[keyIdx].cert, heap, DYNTYPE_CERT);
2854+
ctx->privateKey[keyIdx].subjectName = NULL;
2855+
}
2856+
}
2857+
if (ctx->privateKey[keyIdx].key != NULL) {
2858+
ForceZero(ctx->privateKey[keyIdx].key, ctx->privateKey[keyIdx].keySz);
2859+
WFREE(ctx->privateKey[keyIdx].key, heap, DYNTYPE_PRIVKEY);
2860+
ctx->privateKey[keyIdx].key = NULL;
2861+
ctx->privateKey[keyIdx].keySz = 0;
2862+
}
2863+
if (ctx->privateKey[keyIdx].cert != NULL) {
2864+
WFREE(ctx->privateKey[keyIdx].cert, heap, DYNTYPE_CERT);
2865+
ctx->privateKey[keyIdx].cert = NULL;
2866+
ctx->privateKey[keyIdx].certSz = 0;
28512867
}
28522868

28532869
/* Set up the private key structure */

0 commit comments

Comments
 (0)