Use of Windows certificate store for authentication by JacobBarthelmeh · Pull Request #900 · wolfSSL/wolfssh

JacobBarthelmeh · 2026-03-25T12:38:01Z

No description provided.

…t store.

fix for memory leak and windows build issues sanity check on input and refactor parse cert store spec function

Copilot

Pull request overview

This PR adds Windows Certificate Store integration to wolfSSH so host keys and client authentication keys can be sourced from the Windows cert store (including CI coverage on Windows).

Changes:

Add a Windows-only API to load a private key by locating a certificate in the Windows Certificate Store, and use CNG to sign during SSH handshakes/auth.
Extend cert manager plumbing and wolfsshd configuration to support system/user CA loading and cert-store-based host keys.
Update Windows build projects and add a GitHub Actions workflow to exercise file-vs-store interop permutations.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
wolfssh/test.h	Prefer wolfCrypt Base16 when available; otherwise keep local Base16 decode helper.
wolfssh/ssh.h	Add `wolfSSH_CTX_UsePrivateKey_fromStore()` Windows-only public API.
wolfssh/internal.h	Add CTX private-key metadata for cert-store backed keys and internal helper prototypes.
wolfssh/certman.h	Expose cert-manager setter and Windows cert-store spec parser API.
src/ssh.c	Implement loading a CTX private key from the Windows Certificate Store.
src/internal.c	Add cert-store signing path (CNG) and cert-derived RSA public-key extraction for KEX/auth flows.
src/certman.c	Implement `wolfSSH_SetCertManager()` and `wolfSSH_ParseCertStoreSpec()`.
ide/winvs/wolfsshd/wolfsshd.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features.
ide/winvs/wolfssh/wolfssh.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features.
ide/winvs/wolfsftp-client/wolfsftp-client.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features.
ide/winvs/unit-test/unit-test.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features; normalize XML header.
ide/winvs/echoserver/echoserver.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features.
ide/winvs/client/client.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features.
ide/winvs/api-test/api-test.vcxproj	Link against `crypt32`/`ncrypt` for cert-store features; normalize XML header.
examples/sftpclient/sftpclient.c	Add `-W store:subject:flags` support for client key from Windows cert store.
examples/echoserver/echoserver.c	Add `-W` support for server host key from Windows cert store; skip key-file root search when using store.
examples/client/common.h	Declare helper functions for cert-store key loading/auth setup.
examples/client/common.c	Implement cert-store key loading wrapper + auth globals setup for x509v3 publickey auth.
apps/wolfsshd/wolfsshd.c	Add host-key-from-store support and optional system/user CA store loading into wolfSSH cert manager.
apps/wolfsshd/configuration.h	Add config getters for host-key store and Windows user-CA store options.
apps/wolfsshd/configuration.c	Add parsing/storage for new config directives and defaults.
.github/workflows/windows-cert-store-test.yml	Add Windows CI workflow to validate store/file combinations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/internal.c

src/certman.c

wolfssh/internal.h

src/ssh.c

src/internal.c

Copilot

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T03:33:29Z

wolfssh/certman.h

+WOLFSSH_API
+int wolfSSH_SetCertManager(WOLFSSH_CTX* ctx, WOLFSSL_CERT_MANAGER* cm);
+


wolfSSH_SetCertManager() uses WOLFSSH_CTX* but this header does not declare/forward-declare WOLFSSH_CTX (and it doesn’t include <wolfssh/ssh.h>). Any translation unit including wolfssh/certman.h without including wolfssh/ssh.h first will fail to compile. Add a forward declaration (typedef struct WOLFSSH_CTX WOLFSSH_CTX;) or include the appropriate header before using the type here.

Copilot · 2026-04-14T03:33:30Z

src/ssh.c

+        if (ctx->privateKey[keyIdx].certStoreContext != NULL)
+            CertFreeCertificateContext(
+                (PCCERT_CONTEXT)ctx->privateKey[keyIdx].certStoreContext);
+        if (ctx->privateKey[keyIdx].storeName != NULL)
+            WFREE(ctx->privateKey[keyIdx].storeName, heap, DYNTYPE_STRING);
+        if (ctx->privateKey[keyIdx].subjectName != NULL)
+            WFREE(ctx->privateKey[keyIdx].subjectName, heap, DYNTYPE_STRING);
+        if (ctx->privateKey[keyIdx].cert != NULL)
+            WFREE(ctx->privateKey[keyIdx].cert, heap, DYNTYPE_CERT);
+    }


When reusing an existing private-key slot (same publicKeyFmt), this cleanup only runs if the existing slot already had useCertStore=1. If the slot previously held a file-based key/cert, its key (and possibly cert) buffers are never freed/zeroed before being overwritten, causing leaks and leaving stale key material attached to the slot. Consider mirroring SetHostPrivateKey() cleanup: free/ForceZero any existing key/keySz, free any existing cert/certSz, and clear related pointers regardless of useCertStore before assigning the cert-store-backed key.

Suggested change

if (ctx->privateKey[keyIdx].certStoreContext != NULL)

CertFreeCertificateContext(

(PCCERT_CONTEXT)ctx->privateKey[keyIdx].certStoreContext);

if (ctx->privateKey[keyIdx].storeName != NULL)

WFREE(ctx->privateKey[keyIdx].storeName, heap, DYNTYPE_STRING);

if (ctx->privateKey[keyIdx].subjectName != NULL)

WFREE(ctx->privateKey[keyIdx].subjectName, heap, DYNTYPE_STRING);

if (ctx->privateKey[keyIdx].cert != NULL)

WFREE(ctx->privateKey[keyIdx].cert, heap, DYNTYPE_CERT);

}

if (ctx->privateKey[keyIdx].certStoreContext != NULL) {

CertFreeCertificateContext(

(PCCERT_CONTEXT)ctx->privateKey[keyIdx].certStoreContext);

ctx->privateKey[keyIdx].certStoreContext = NULL;

}

if (ctx->privateKey[keyIdx].storeName != NULL) {

WFREE(ctx->privateKey[keyIdx].storeName, heap, DYNTYPE_STRING);

ctx->privateKey[keyIdx].storeName = NULL;

}

if (ctx->privateKey[keyIdx].subjectName != NULL) {

WFREE(ctx->privateKey[keyIdx].subjectName, heap, DYNTYPE_STRING);

ctx->privateKey[keyIdx].subjectName = NULL;

}

}

if (ctx->privateKey[keyIdx].key != NULL) {

ForceZero(ctx->privateKey[keyIdx].key, ctx->privateKey[keyIdx].keySz);

WFREE(ctx->privateKey[keyIdx].key, heap, DYNTYPE_PRIVKEY);

ctx->privateKey[keyIdx].key = NULL;

ctx->privateKey[keyIdx].keySz = 0;

}

if (ctx->privateKey[keyIdx].cert != NULL) {

WFREE(ctx->privateKey[keyIdx].cert, heap, DYNTYPE_CERT);

ctx->privateKey[keyIdx].cert = NULL;

ctx->privateKey[keyIdx].certSz = 0;

}

Copilot · 2026-04-14T03:33:30Z

src/ssh.c

+    if (pCertContext == NULL) {
+        /* Try finding by thumbprint if subject name didn't work */
+        /* Note: subjectName could be a thumbprint in format "XX XX XX ..." */
+        CertCloseStore(hStore, 0);
+        WLOG(WS_LOG_ERROR, "wolfSSH_CTX_UsePrivateKey_fromStore: Certificate "
+             "not found with subject '%ls'", subjectName);
+        return WS_FATAL_ERROR;


The code comments say it will "Try finding by thumbprint" when subject lookup fails, but the implementation returns immediately without attempting any thumbprint-based search (e.g., CERT_FIND_HASH). This is misleading for callers and future maintenance; either implement the thumbprint lookup or remove/adjust the comment to match actual behavior.

Copilot · 2026-04-14T03:33:30Z

apps/wolfsshd/wolfsshd.c

+            /* Convert to wide strings */
+            storeNameLen = MultiByteToWideChar(CP_UTF8, 0, hostKeyStore, -1, NULL, 0);
+            subjectNameLen = MultiByteToWideChar(CP_UTF8, 0, hostKeyStoreSubject, -1, NULL, 0);
+
+            wStoreName = (wchar_t*)WMALLOC(storeNameLen * sizeof(wchar_t), heap, DYNTYPE_SSHD);
+            wSubjectName = (wchar_t*)WMALLOC(subjectNameLen * sizeof(wchar_t), heap, DYNTYPE_SSHD);
+
+            if (wStoreName == NULL || wSubjectName == NULL) {
+                wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Memory allocation failed for cert store strings");
+                ret = WS_MEMORY_E;
+            } else {
+                MultiByteToWideChar(CP_UTF8, 0, hostKeyStore, -1, wStoreName, storeNameLen);
+                MultiByteToWideChar(CP_UTF8, 0, hostKeyStoreSubject, -1, wSubjectName, subjectNameLen);
+
+                ret = wolfSSH_CTX_UsePrivateKey_fromStore(*ctx, wStoreName, dwFlags, wSubjectName);
+                if (ret != WS_SUCCESS) {
+                    wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to load host key from certificate store");
+                }
+
+                WFREE(wStoreName, heap, DYNTYPE_SSHD);
+                WFREE(wSubjectName, heap, DYNTYPE_SSHD);
+            }


MultiByteToWideChar() return values are not checked before being used as allocation sizes. If conversion fails, it can return 0, leading to zero-sized allocations and subsequent calls that will fail (or worse, write into undersized buffers). Also, if one of WMALLOC allocations succeeds and the other fails, the successful allocation is leaked. Please validate storeNameLen/subjectNameLen > 0, handle GetLastError()/conversion failures, and free any partially allocated wide strings on error paths.

Copilot · 2026-04-14T03:33:30Z

apps/wolfsshd/configuration.c

+        ret = WS_BAD_ARGUMENT;
+    }
+
+    if (ret == WS_SUCCESS) {


These setters assign a newly allocated string via CreateString() without freeing any previously set value, which leaks memory if the config option is specified more than once (or if config structs are reused). Consider freeing conf->winUserStores first (e.g., FreeString) or using the existing SetFileString() helper which handles replacement safely.

Suggested change

if (ret == WS_SUCCESS) {

if (ret == WS_SUCCESS) {

if (conf->winUserStores != NULL) {

WFREE(conf->winUserStores, conf->heap, DYNTYPE_STRING);

conf->winUserStores = NULL;

}

JacobBarthelmeh and others added 7 commits March 24, 2026 21:32

add TrustedSystemCAKeys sshd option for system CA load

40302ac

add macro guard for system ca certs load

b0cf94f

Add support for loading user CA certs from a configurable Windows cer…

46964da

…t store.

Prefix wolfSSH specific options with wolfSSH_.

6f7b777

add Windows cert store use with signing and add example arguments

6c4188b

add Windows cert store test case

8d1094d

add windows build link with ncrypt for cert store use

4ddac11

JacobBarthelmeh self-assigned this Mar 25, 2026

Copilot AI review requested due to automatic review settings March 25, 2026 12:38

Copilot started reviewing on behalf of JacobBarthelmeh March 25, 2026 12:39 View session

fixes for brace style, null deref, ecc curve detection, unused variables

ca912b0

fix for memory leak and windows build issues sanity check on input and refactor parse cert store spec function

JacobBarthelmeh force-pushed the winSysCerts branch from bd1396d to ca912b0 Compare March 25, 2026 12:43

Copilot AI reviewed Mar 25, 2026

View reviewed changes

JacobBarthelmeh added 2 commits March 25, 2026 11:05

add macro guard for no curve25519 build

fde4606

add ECC test case and review items

0d5c65f

Copilot AI review requested due to automatic review settings April 14, 2026 03:24

Copilot started reviewing on behalf of JacobBarthelmeh April 14, 2026 03:27 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use of Windows certificate store for authentication#900

Use of Windows certificate store for authentication#900
JacobBarthelmeh wants to merge 10 commits intowolfSSL:masterfrom
JacobBarthelmeh:winSysCerts

JacobBarthelmeh commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		WOLFSSH_API
		int wolfSSH_SetCertManager(WOLFSSH_CTX* ctx, WOLFSSL_CERT_MANAGER* cm);

-    if (ret == WS_SUCCESS) {
+    if (ret == WS_SUCCESS) {
+        if (conf->winUserStores != NULL) {
+            WFREE(conf->winUserStores, conf->heap, DYNTYPE_STRING);
+            conf->winUserStores = NULL;
+        }

Conversation

JacobBarthelmeh commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants