Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
731 changes: 731 additions & 0 deletions .github/workflows/windows-cert-store-test.yml

Large diffs are not rendered by default.

23 changes: 19 additions & 4 deletions apps/wolfsshd/auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@
#include <wolfssl/wolfcrypt/error-crypt.h>
#include <wolfssl/wolfcrypt/coding.h>

#ifdef WOLFSSL_FPKI
#if defined(WOLFSSL_FPKI) || defined(_WIN32)
/* Used to bind a client certificate to the requested user name: by UPN
* with FPKI, by subject CN on Windows builds without FPKI. */
#include <wolfssl/wolfcrypt/asn.h>
#endif

Expand Down Expand Up @@ -1538,10 +1540,11 @@ static int RequestAuthentication(WS_UserAuthData* authData,
ret = WOLFSSH_USERAUTH_REJECTED;
}

#ifdef WOLFSSL_FPKI
#if defined(WOLFSSL_FPKI) || defined(_WIN32)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Outer _WIN32 guard vs inner WIN32 guard mismatch in cert auth path · Incorrect bitwise vs logical operators

The outer block is guarded by defined(_WIN32) (line 1543) but the SetupUserTokenWin call inside is guarded by #ifdef WIN32 (line 1642). On a Windows toolchain where _WIN32 is defined but WIN32 is not, the CN check succeeds yet the token-setup branch is skipped, causing cert auth to fall to the unconditional rejection path.

Fix: Change the outer guard to use WIN32 for consistency with the existing inner guard, or change the inner guard to _WIN32.

if (ret == WOLFSSH_USERAUTH_SUCCESS &&
authData->type == WOLFSSH_USERAUTH_PUBLICKEY) {
/* compare user name to UPN in certificate */
/* Bind the certificate to the requested user name via UPN with FPKI or
* CN without FPKI. */
if (authData->sf.publicKey.isCert) {
DecodedCert* dCert;
#ifdef WOLFSSH_SMALL_STACK
Expand All @@ -1566,6 +1569,7 @@ static int RequestAuthentication(WS_UserAuthData* authData,
}
else {
int usrMatch = 0;
#ifdef WOLFSSL_FPKI
DNS_entry* current = dCert->altNames;

while (current != NULL) {
Expand All @@ -1590,6 +1594,15 @@ static int RequestAuthentication(WS_UserAuthData* authData,
}
current = current->next;
}
#else
/* Without FPKI compare subject CN with user name */
if (dCert->subjectCN != NULL &&
(int)XSTRLEN(usr) == dCert->subjectCNLen &&
XSTRNCMP(usr, dCert->subjectCN,
(size_t)dCert->subjectCNLen) == 0) {
usrMatch = 1;
}
#endif

if (usrMatch == 0) {
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] incorrect user cert "
Expand Down Expand Up @@ -1627,7 +1640,9 @@ static int RequestAuthentication(WS_UserAuthData* authData,
}
else {
#ifdef WIN32
/* Still need to get users token on Windows */
/* The UPN/CN-vs-username check above already bound the
* certificate to the requested user. Still need to get
* the users token on Windows. */
wolfSSH_Log(WS_LOG_INFO,
"[SSHD] Relying on CA for public key check");
rc = SetupUserTokenWin(usr, &authData->sf.publicKey,
Expand Down
Loading
Loading