From 161c5ae24f257c998cc2b5181202acf0d5dacd9a Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 2 Sep 2026 23:13:15 -0600 Subject: [PATCH] expand CI test for non-fpki build, fix check for error state with CertEnumCertificatesInStore, add extended key usage type check if present, fix for EKU checks with X509 use and add test case --- .github/workflows/windows-cert-store-test.yml | 138 +++++++++- README.md | 11 +- apps/wolfsshd/wolfsshd.c | 37 ++- examples/echoserver/echoserver.c | 26 +- src/certman.c | 239 +++++++++++++++- src/internal.c | 6 + tests/unit.c | 254 +++++++++++++++++- wolfssh/certman.h | 9 + wolfssh/error.h | 4 +- wolfssh/internal.h | 7 + 10 files changed, 674 insertions(+), 57 deletions(-) diff --git a/.github/workflows/windows-cert-store-test.yml b/.github/workflows/windows-cert-store-test.yml index d9e9aa91..ec6971ba 100644 --- a/.github/workflows/windows-cert-store-test.yml +++ b/.github/workflows/windows-cert-store-test.yml @@ -10,6 +10,9 @@ name: Windows Certificate Store Test # 2. If the server key comes from the store: run echoserver with -W and # connect with the SFTP client. # 3. Run wolfsshd as a Windows service and connect with the SFTP client. +# +# Two builds feed the matrix: one with OPENSSL_ALL (wolfSSL FPKI, UPN +# identity binding) and one without (subject CN binding). on: push: @@ -151,6 +154,104 @@ jobs: path: | wolfssh/ide/winvs/**/Release/** + # The same build against a wolfSSL without OPENSSL_ALL, and so without + # WOLFSSL_FPKI to test CN match instead of UPN. + build-no-fpki: + runs-on: windows-latest + timeout-minutes: 30 + + steps: + - uses: actions/checkout@v4 + with: + repository: wolfssl/wolfssl + path: wolfssl + + - uses: actions/checkout@v4 + with: + path: wolfssh + + - name: Add MSBuild to PATH + uses: microsoft/setup-msbuild@v1 + + - name: Restore wolfSSL NuGet packages + working-directory: ${{ github.workspace }}\wolfssl + run: nuget restore ${{env.WOLFSSL_SOLUTION_FILE_PATH}} + + - name: user_settings.h for sshd, x509 and cert store without OPENSSL_ALL + working-directory: ${{ github.workspace }} + shell: bash + run: | + sed -i 's/#if 0/#if 1/g' ${{env.USER_SETTINGS_H_NEW}} + # Drop OPENSSL_ALL so wolfSSL is built without WOLFSSL_FPKI. + sed -i '/OPENSSL_ALL/d' ${{env.USER_SETTINGS_H_NEW}} + if grep -q 'OPENSSL_ALL' ${{env.USER_SETTINGS_H_NEW}}; then + echo "ERROR: OPENSSL_ALL still present" + exit 1 + fi + sed -i '/#endif \/\* _WIN_USER_SETTINGS_H_ \*\//i\ + /* Inserted by windows-cert-store-test CI */\ + #define WOLFSSH_WINDOWS_CERT_STORE\ + #define WOLFSSH_NO_SHA1_SOFT_DISABLE\ + #define WC_SIG_MIN_HASH_TYPE WC_HASH_TYPE_SHA' ${{env.USER_SETTINGS_H_NEW}} + grep -q '^#define WOLFSSH_WINDOWS_CERT_STORE' ${{env.USER_SETTINGS_H_NEW}} + cp ${{env.USER_SETTINGS_H_NEW}} ${{env.USER_SETTINGS_H}} + + - name: Build wolfssl library + working-directory: ${{ github.workspace }}\wolfssl + run: msbuild /m /p:PlatformToolset=v142 /p:Platform=${{env.BUILD_PLATFORM}} /p:Configuration=${{env.WOLFSSL_BUILD_CONFIGURATION}} /t:wolfssl ${{env.WOLFSSL_SOLUTION_FILE_PATH}} + + - name: Upload wolfSSL build artifacts + uses: actions/upload-artifact@v4 + with: + name: wolfssl-windows-build-no-fpki + if-no-files-found: warn + retention-days: 1 + path: | + wolfssl/IDE/WIN/${{env.WOLFSSL_BUILD_CONFIGURATION}}/${{env.BUILD_PLATFORM}}/** + wolfssl/IDE/WIN/${{env.WOLFSSL_BUILD_CONFIGURATION}}/** + wolfssl/${{env.WOLFSSL_BUILD_CONFIGURATION}}/${{env.BUILD_PLATFORM}}/** + wolfssl/${{env.WOLFSSL_BUILD_CONFIGURATION}}/** + + - name: Restore NuGet packages + working-directory: ${{ github.workspace }}\wolfssh\ide\winvs + run: nuget restore ${{env.SOLUTION_FILE_PATH}} + + # Fails the build unless auth.c really compiles the subject-CN branch: + # WOLFSSL_FPKI must be absent and the cert store defines present. + - name: Guard that the CN binding branch is the one compiled + working-directory: ${{ github.workspace }}\wolfssh + shell: bash + run: | + printf '\n#if defined(WOLFSSL_FPKI) || !defined(WOLFSSH_NO_FPKI)\n#error "CI: expected a non-FPKI wolfSSL for the CN binding build"\n#endif\n' >> apps/wolfsshd/auth.c + printf '\n#if !defined(WOLFSSH_WINDOWS_CERT_STORE) || !defined(WOLFSSH_SSHD)\n#error "CI: expected defines did not reach wolfsshd.c"\n#endif\n' >> apps/wolfsshd/wolfsshd.c + + - name: Build wolfssh + working-directory: ${{ github.workspace }}\wolfssh\ide\winvs + run: msbuild /m /p:PlatformToolset=v142 /p:Platform=${{env.BUILD_PLATFORM}} /p:WindowsTargetPlatformVersion=${{env.TARGET_PLATFORM}} /p:Configuration=${{env.WOLFSSH_BUILD_CONFIGURATION}} ${{env.SOLUTION_FILE_PATH}} + + - name: Run api-test and unit-test + working-directory: ${{ github.workspace }}\wolfssh + shell: pwsh + run: | + $PSNativeCommandUseErrorActionPreference = $false + $dir = "ide\winvs\${{env.WOLFSSH_BUILD_CONFIGURATION}}\${{env.BUILD_PLATFORM}}" + $dll = Get-ChildItem -Path "${{ github.workspace }}\wolfssl" -Recurse -Filter "wolfssl.dll" -ErrorAction SilentlyContinue | Select-Object -First 1 + if ($dll) { Copy-Item $dll.FullName $dir -Force } + foreach ($t in @("api-test", "unit-test")) { + $exe = Join-Path $dir "$t.exe" + if (-not (Test-Path $exe)) { throw "$exe not found" } + & $exe + if ($LASTEXITCODE -ne 0) { throw "$t failed (exit $LASTEXITCODE)" } + } + + - name: Upload wolfSSH build artifacts + uses: actions/upload-artifact@v4 + with: + name: wolfssh-windows-build-no-fpki + if-no-files-found: error + path: | + wolfssh/ide/winvs/**/Release/** + # Compile-only check of the WOLFSSL_SYS_CA_CERTS paths in wolfsshd, which # the functional matrix never defines and so never builds. build-sys-ca-certs: @@ -281,12 +382,14 @@ jobs: fi test: - needs: build + needs: [build, build-no-fpki] runs-on: windows-latest timeout-minutes: 30 strategy: fail-fast: false matrix: + # build_flavor selects the artifact pair: unset for the FPKI build, + # "-no-fpki" for the build without OPENSSL_ALL. include: # user_ca_source: store replaces the file-based TrustedUserCAKeys # with wolfSSH_TrustedUserCAStore, so the store is the only trust @@ -326,6 +429,15 @@ jobs: key_algorithm: rsa client_key_algorithm: rsa test_name: "Server-File-Client-Store-RSA" + # The first entry again on the build without WOLFSSL_FPKI, so the + # identity binding step below exercises the subject-CN branch of + # auth.c: matching CN, case-differing account name, mismatched CN. + - server_key_source: file + client_key_source: x509 + key_algorithm: rsa + user_ca_source: store + build_flavor: -no-fpki + test_name: "Server-File-Client-X509-UserCAStore-NoFPKI" steps: - uses: actions/checkout@v4 @@ -335,13 +447,13 @@ jobs: - name: Download wolfSSH build artifacts uses: actions/download-artifact@v4 with: - name: wolfssh-windows-build + name: wolfssh-windows-build${{ matrix.build_flavor }} path: . - name: Download wolfSSL build artifacts uses: actions/download-artifact@v4 with: - name: wolfssl-windows-build + name: wolfssl-windows-build${{ matrix.build_flavor }} path: . - name: Create testuser client certificate - ${{ matrix.test_name }} @@ -1374,17 +1486,15 @@ jobs: # The certificate identity binding: a client certificate whose identity # does not match the requested account must be rejected, and the match is - # case-insensitive like Windows account names. This build defines - # OPENSSL_ALL, which turns on WOLFSSL_ASN_ALL and with it WOLFSSL_FPKI in - # wolfSSL's settings.h, so the binding evaluated here is the certificate - # UPN (renewcerts.cnf gives the test certs an msUPN altname); the - # subject-CN branch only compiles on non-FPKI wolfSSL builds and so has - # no runtime coverage here -- exercising it needs a matrix entry whose - # wolfSSL user_settings.h omits OPENSSL_ALL. Runs - # against the already-running service on the user_ca_source: store - # entry, whose config sets no AuthorizedKeysFile so the identity binding - # is what decides. - - name: Client certificate UPN binding is enforced (FPKI build) + # case-insensitive like Windows account names. On the FPKI build + # (OPENSSL_ALL turns on WOLFSSL_ASN_ALL and with it WOLFSSL_FPKI) the + # binding is the certificate UPN, which renewcerts.cnf sets to + # @example alongside the CN; on the -no-fpki build it is the + # subject CN. The wronguser cert differs in both, so the same three + # connections cover either branch. Runs against the already-running + # service on the user_ca_source: store entries, whose config sets no + # AuthorizedKeysFile so the identity binding is what decides. + - name: Client certificate identity binding is enforced (UPN with FPKI, CN without) if: matrix.user_ca_source == 'store' working-directory: ${{ github.workspace }}\wolfssh shell: pwsh diff --git a/README.md b/README.md index 0003d235..15b3f293 100644 --- a/README.md +++ b/README.md @@ -523,6 +523,14 @@ For this example, we are disabling the FPKI checking as the included certificate for "fred" does not have the required FPKI extensions. If the flag WOLFSSH_NO_FPKI is removed, you can see the certificate get rejected. +With or without FPKI, a peer certificate is held to RFC 6187 section 2.2: a +KeyUsage extension must assert digitalSignature, and an ExtendedKeyUsage +extension must name anyExtendedKeyUsage or a purpose for the role being +verified (id-kp-secureShellClient or clientAuth for a user certificate, +id-kp-secureShellServer or serverAuth for a host certificate). A certificate +without those extensions is accepted. A mismatch fails with +`WS_CERT_KEY_USAGE_E`. + To provide a CA root certificate to validate a user's certificate, give the echoserver the command line option `-a`. @@ -600,7 +608,8 @@ with `WOLFSSH_IGNORE_UNKNOWN_CONFIG` instead log a warning and ignore the directive, preserving the old behavior as a migration path. Without FPKI, a client certificate is bound to the requested account by a -case-insensitive subject CN match only; keep the trusted CA set narrow. Note +case-insensitive subject CN match only, after the RFC 6187 key usage check +described under CERTIFICATE SUPPORT; keep the trusted CA set narrow. Note also that the config parser requires whitespace between an option name and its value; the OpenSSH `Keyword=value` form is rejected. diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index c0881855..e6c2ae97 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -619,6 +619,7 @@ static int LoadUserCACertsFromStore(const WOLFSSHD_CONFIG* conf, word32 rejected = 0; word32 notX509 = 0; int isCA; + DWORD enumErr; storeNameStr = wolfSSHD_ConfigGetWinUserPvPara(conf); dwFlagsStr = wolfSSHD_ConfigGetWinUserDwFlags(conf); @@ -695,11 +696,11 @@ static int LoadUserCACertsFromStore(const WOLFSSHD_CONFIG* conf, #ifdef WOLFSSH_NO_FPKI wolfSSH_Log(WS_LOG_WARN, - "[SSHD] WARNING: built without FPKI profile checking, so peer certs " - "need not carry a client authentication EKU."); + "[SSHD] WARNING: built without FPKI profile checking; peer certs are " + "held only to RFC 6187 key usage."); wolfSSH_Log(WS_LOG_WARN, - "[SSHD] A TLS server, S/MIME or code signing certificate with a " - "matching subject is accepted for login."); + "[SSHD] A cert with no EKU, or one naming clientAuth or " + "secureShellClient, whose subject matches is accepted for login."); #endif /* MB_ERR_INVALID_CHARS, as wolfSSH_ParseCertStoreSpec() uses, so a @@ -736,10 +737,21 @@ static int LoadUserCACertsFromStore(const WOLFSSHD_CONFIG* conf, return WS_FATAL_ERROR; } - /* Passing the previous context frees it and advances the enumeration. */ + /* Passing the previous context frees it and advances the enumeration. + * NULL means either end of store or failure; only the documented end + * codes count as completion, anything else fails the load so a partial + * trust anchor set is never reported as complete. */ for (;;) { pCertContext = CertEnumCertificatesInStore(hStore, pCertContext); if (pCertContext == NULL) { + enumErr = GetLastError(); + if (enumErr != CRYPT_E_NOT_FOUND && + enumErr != ERROR_NO_MORE_FILES) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Enumerating user CA cert store '%.48s' failed, " + "error 0x%08lx", storeNameStr, (unsigned long)enumErr); + ret = WS_FATAL_ERROR; + } break; } if (pCertContext->pbCertEncoded == NULL || @@ -790,7 +802,12 @@ static int LoadUserCACertsFromStore(const WOLFSSHD_CONFIG* conf, /* Counts and location go on their own lines: wolfSSH_Log formats into * a 120 byte buffer, and one line carrying the %.48s store name plus * the counts would be cut short right where the numbers are. */ - if (loaded == 0) { + if (ret != WS_SUCCESS) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] %u CA certificate(s) loaded before the failure; " + "refusing to start on a partial trust anchor set", loaded); + } + else if (loaded == 0) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] No usable CA certificates found in store '%.48s'", storeNameStr); @@ -1222,11 +1239,11 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, "organization's CA."); #ifdef WOLFSSH_NO_FPKI wolfSSH_Log(WS_LOG_WARN, - "[SSHD] WARNING: built without FPKI profile checking, so peer " - "certs need not carry a client authentication EKU."); + "[SSHD] WARNING: built without FPKI profile checking; peer certs " + "are held only to RFC 6187 key usage."); wolfSSH_Log(WS_LOG_WARN, - "[SSHD] A TLS server, S/MIME or code signing certificate with a " - "matching subject is accepted for login."); + "[SSHD] A cert with no EKU, or one naming clientAuth or " + "secureShellClient, whose subject matches is accepted for login."); #endif sslCtx = wolfSSL_CTX_new(wolfSSLv23_server_method()); if (sslCtx == NULL) { diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 5297a4d8..435cec47 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -3192,21 +3192,32 @@ static void ShowUsage(void) } +#define ECHOSERVER_OPTLIST "?1a:d:DefEp:R:Ni:j:i:I:J:K:P:k:b:x:m:c:s:G:HW:" + #ifdef WOLFSSH_WINDOWS_CERT_STORE /* Detects whether argv or the environment requests a host key from the - * Windows certificate store, without doing the full option parse that + * Windows certificate store, before the full option parse that * echoserver_test() does later. Used to decide whether the root directory - * search for PEM key files should be skipped. */ + * search for PEM key files should be skipped. Parses with the same option + * list rather than matching on argv: an option value could start with "-W", + * and "-W" may sit in a cluster such as "-NW". */ static int EchoserverUsingCertStore(int argc, char** argv) { - int i; + int ch; + int found = 0; const char* spec; - for (i = 1; i < argc; i++) { - if (WSTRNCMP(argv[i], "-W", 2) == 0) { - return 1; + myoptind = 0; + while ((ch = mygetopt(argc, argv, ECHOSERVER_OPTLIST)) != -1) { + if (ch == 'W') { + found = 1; + break; } } + myoptind = 0; + if (found) { + return 1; + } spec = getenv("WOLFSSH_CERT_STORE"); return (spec != NULL && spec[0] != '\0'); @@ -3337,9 +3348,8 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) #endif if (argc > 0) { - const char* optlist = "?1a:d:DefEp:R:Ni:j:i:I:J:K:P:k:b:x:m:c:s:G:HW:"; myoptind = 0; - while ((ch = mygetopt(argc, argv, optlist)) != -1) { + while ((ch = mygetopt(argc, argv, ECHOSERVER_OPTLIST)) != -1) { switch (ch) { case '?' : ShowUsage(); diff --git a/src/certman.c b/src/certman.c index 51a8f45a..11c71113 100644 --- a/src/certman.c +++ b/src/certman.c @@ -115,9 +115,14 @@ #endif +/* side is CERTMAN_SIDE_ANY until wolfSSH_CERTMAN_SetSide() names the local + * endpoint; a standalone manager then accepts either SSH key purpose. */ +#define CERTMAN_SIDE_ANY (-1) + struct WOLFSSH_CERTMAN { void* heap; WOLFSSL_CERT_MANAGER* cm; + int side; }; @@ -187,6 +192,7 @@ static WOLFSSH_CERTMAN* _CertMan_init(WOLFSSH_CERTMAN* cm, void* heap) if (ret != NULL) { WMEMSET(ret, 0, sizeof(WOLFSSH_CERTMAN)); ret->heap = heap; + ret->side = CERTMAN_SIDE_ANY; ret->cm = wolfSSL_CertManagerNew_ex(heap); if (ret->cm == NULL) { ret = NULL; @@ -249,6 +255,14 @@ WOLFSSH_CERTMAN* wolfSSH_CERTMAN_new(void* heap) } +void wolfSSH_CERTMAN_SetSide(WOLFSSH_CERTMAN* cm, int side) +{ + if (cm != NULL) { + cm->side = side; + } +} + + void wolfSSH_CERTMAN_free(WOLFSSH_CERTMAN* cm) { WLOG_ENTER(); @@ -344,6 +358,208 @@ static int CertManIntermediateIsCA(WOLFSSH_CERTMAN* cm, return isCA; } +/* Reads a DER tag and definite length at *idx. On success *idx is advanced + * to the content, which is *len bytes and lies within sz. */ +static int DerGetHeader(const byte* in, word32 sz, word32* idx, byte* tag, + word32* len) +{ + word32 i = *idx; + word32 l; + byte b; + int n; + + if (i >= sz || sz - i < 2) { + return -1; + } + *tag = in[i++]; + b = in[i++]; + if (b < 0x80) { + l = b; + } + else { + n = b & 0x7F; + if (n == 0 || n > 4 || sz - i < (word32)n) { + return -1; + } + l = 0; + while (n-- > 0) { + l = (l << 8) | in[i++]; + } + } + if (l > sz - i) { + return -1; + } + *idx = i; + *len = l; + return 0; +} + + +/* id-kp-secureShellClient 1.3.6.1.5.5.7.3.21 and id-kp-secureShellServer + * 1.3.6.1.5.5.7.3.22 (RFC 6187 section 2.2.2). wolfSSL 5.9.2 and earlier do + * not set DecodedCert.extExtKeyUsageSsh bits for these in the default ASN + * template build, so the EKU extension is walked here. Later wolfSSL versions + * can replace this with an extExtKeyUsageSsh bit comparison. */ +static const byte kpSecureShellClientOid[] = + { 0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x03, 0x15 }; +static const byte kpSecureShellServerOid[] = + { 0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x03, 0x16 }; +static const byte extKeyUsageExtOid[] = { 0x55, 0x1D, 0x25 }; + +/* Returns 1 when the cert's ExtendedKeyUsage names oid, 0 when it does not + * (or there is no such extension), negative when the extensions do not + * parse. cert->extensions spans the raw Extensions SEQUENCE, in some wolfSSL + * versions still inside its [3] wrapper. */ +static int CertManExtKeyUsageHasOid(const DecodedCert* cert, const byte* oid, + word32 oidSz) +{ + const byte* in = cert->extensions; + word32 sz, idx = 0, len, end, extEnd; + byte tag; + + if (in == NULL || cert->extensionsSz <= 0) { + return 0; + } + sz = (word32)cert->extensionsSz; + + if (DerGetHeader(in, sz, &idx, &tag, &len) != 0) { + return -1; + } + if (tag == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | 3)) { + if (DerGetHeader(in, sz, &idx, &tag, &len) != 0) { + return -1; + } + } + if (tag != (ASN_SEQUENCE | ASN_CONSTRUCTED)) { + return -1; + } + end = idx + len; + + /* Extension ::= SEQUENCE { extnID OID, critical BOOLEAN DEFAULT FALSE, + * extnValue OCTET STRING } */ + while (idx < end) { + if (DerGetHeader(in, end, &idx, &tag, &len) != 0 || + tag != (ASN_SEQUENCE | ASN_CONSTRUCTED)) { + return -1; + } + extEnd = idx + len; + if (DerGetHeader(in, extEnd, &idx, &tag, &len) != 0 || + tag != ASN_OBJECT_ID) { + return -1; + } + if (len == sizeof(extKeyUsageExtOid) && + WMEMCMP(in + idx, extKeyUsageExtOid, len) == 0) { + idx += len; + if (DerGetHeader(in, extEnd, &idx, &tag, &len) != 0) { + return -1; + } + if (tag == ASN_BOOLEAN) { + idx += len; + if (DerGetHeader(in, extEnd, &idx, &tag, &len) != 0) { + return -1; + } + } + if (tag != ASN_OCTET_STRING) { + return -1; + } + /* ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId */ + extEnd = idx + len; + if (DerGetHeader(in, extEnd, &idx, &tag, &len) != 0 || + tag != (ASN_SEQUENCE | ASN_CONSTRUCTED)) { + return -1; + } + extEnd = idx + len; + while (idx < extEnd) { + if (DerGetHeader(in, extEnd, &idx, &tag, &len) != 0 || + tag != ASN_OBJECT_ID) { + return -1; + } + if (len == oidSz && WMEMCMP(in + idx, oid, oidSz) == 0) { + return 1; + } + idx += len; + } + return 0; + } + idx = extEnd; + } + + return 0; +} + + +/* RFC 6187 section 2.2 leaf checks, applied in every build. A KeyUsage + * extension must assert digitalSignature. An ExtendedKeyUsage extension must + * name anyExtendedKeyUsage or a purpose for the role being verified: + * id-kp-secureShellClient or TLS clientAuth for a user cert (verified by a + * server), id-kp-secureShellServer or TLS serverAuth for a host cert + * (verified by a client). The TLS purposes are what the FPKI profiles + * already require, so both build flavors accept the same certificates. */ +static int CertManHasUserAuthEku(const DecodedCert* cert) +{ + if ((cert->extExtKeyUsage & EXTKEYUSE_CLIENT_AUTH) != 0) { + return 1; + } + return CertManExtKeyUsageHasOid(cert, kpSecureShellClientOid, + sizeof(kpSecureShellClientOid)); +} + + +static int CertManHasHostAuthEku(const DecodedCert* cert) +{ + if ((cert->extExtKeyUsage & EXTKEYUSE_SERVER_AUTH) != 0) { + return 1; + } + return CertManExtKeyUsageHasOid(cert, kpSecureShellServerOid, + sizeof(kpSecureShellServerOid)); +} + + +static int CertManCheckLeafUsage(const DecodedCert* cert, int side) +{ + if (cert->extKeyUsageSet && + (cert->extKeyUsage & KEYUSE_DIGITAL_SIG) == 0) { + WLOG(WS_LOG_CERTMAN, "leaf KeyUsage lacks digitalSignature"); + return WS_CERT_KEY_USAGE_E; + } + + if (!cert->extExtKeyUsageSet) { + return WS_SUCCESS; + } + + if ((cert->extExtKeyUsage & EXTKEYUSE_ANY) != 0) { + return WS_SUCCESS; + } + + /* A server verifies user certs, a client verifies host certs. A + * standalone manager has no side and accepts either. */ + if (side == WOLFSSH_ENDPOINT_SERVER) { + if (CertManHasUserAuthEku(cert) == 1) { + return WS_SUCCESS; + } + WLOG(WS_LOG_CERTMAN, "leaf ExtendedKeyUsage has no purpose usable " + "for SSH user authentication"); + return WS_CERT_KEY_USAGE_E; + } + + if (side == WOLFSSH_ENDPOINT_CLIENT) { + if (CertManHasHostAuthEku(cert) == 1) { + return WS_SUCCESS; + } + WLOG(WS_LOG_CERTMAN, "leaf ExtendedKeyUsage has no purpose usable " + "for SSH host authentication"); + return WS_CERT_KEY_USAGE_E; + } + + if (CertManHasUserAuthEku(cert) == 1 || CertManHasHostAuthEku(cert) == 1) { + return WS_SUCCESS; + } + WLOG(WS_LOG_CERTMAN, "leaf ExtendedKeyUsage has no purpose usable " + "for SSH authentication"); + return WS_CERT_KEY_USAGE_E; +} + + /* if handling a chain it is expected to be the leaf cert first followed by * intermediates and CA last (CA may be omitted) */ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm, @@ -480,9 +696,10 @@ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm, } } - /* Leaf (index 0) must be an end-entity cert; reject a CA leaf even without - * FPKI, and match a profile when FPKI is on. cm->cm resolves the signer - * (ca) that CheckProfile needs for the issuer-DN match. */ + /* Leaf (index 0) must be an end-entity cert whose key usage permits SSH + * for the role being verified; reject a CA leaf even without FPKI, and + * match a profile when FPKI is on. cm->cm resolves the signer (ca) that + * CheckProfile needs for the issuer-DN match. */ if (ret == WS_SUCCESS) { DecodedCert* decoded = NULL; #ifndef WOLFSSH_SMALL_STACK @@ -509,14 +726,18 @@ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm, WLOG(WS_LOG_CERTMAN, "leaf certificate is a CA; rejecting"); ret = WS_CERT_PROFILE_E; } + else { + ret = CertManCheckLeafUsage(decoded, cm->side); #ifndef WOLFSSH_NO_FPKI - else if (!(CheckProfile(decoded, PROFILE_FPKI_WORKSHEET_6) || - CheckProfile(decoded, PROFILE_FPKI_WORKSHEET_10) || - CheckProfile(decoded, PROFILE_FPKI_WORKSHEET_16))) { - WLOG(WS_LOG_CERTMAN, "certificate didn't match profile"); - ret = WS_CERT_PROFILE_E; - } + if (ret == WS_SUCCESS && + !(CheckProfile(decoded, PROFILE_FPKI_WORKSHEET_6) || + CheckProfile(decoded, PROFILE_FPKI_WORKSHEET_10) || + CheckProfile(decoded, PROFILE_FPKI_WORKSHEET_16))) { + WLOG(WS_LOG_CERTMAN, "certificate didn't match profile"); + ret = WS_CERT_PROFILE_E; + } #endif /* WOLFSSH_NO_FPKI */ + } wc_FreeDecodedCert(decoded); } diff --git a/src/internal.c b/src/internal.c index 1db1c34f..d12d8f38 100644 --- a/src/internal.c +++ b/src/internal.c @@ -578,6 +578,9 @@ const char* GetErrorString(int err) case WS_ED448_E: return "Ed448 failure"; + case WS_CERT_KEY_USAGE_E: + return "certificate key usage does not permit SSH use"; + case WS_AUTH_PENDING: return "userauth is still pending (callback would block)"; @@ -1383,6 +1386,9 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap) ctx->certMan = wolfSSH_CERTMAN_new(ctx->heap); if (ctx->certMan == NULL) return NULL; + /* A server verifies user certs and a client host certs; the side picks + * which RFC 6187 key purpose the leaf must allow. */ + wolfSSH_CERTMAN_SetSide(ctx->certMan, side); #endif /* WOLFSSH_CERTS */ ctx->windowSz = DEFAULT_WINDOW_SZ; ctx->maxPacketSz = DEFAULT_MAX_PACKET_SZ; diff --git a/tests/unit.c b/tests/unit.c index 1189ec34..f67c8408 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -15782,10 +15782,14 @@ static int certmanForgeChild(const byte* issuerCert, word32 issuerCertSz, /* Forge a cert with the given subject CN. When isCA is set the cert asserts * basicConstraints CA=TRUE. When keyUsage is non-NULL the cert carries a * KeyUsage extension with the named usage(s) (e.g. "keyCertSign" or - * "digitalSignature"); NULL omits the extension entirely. The issuer name is - * taken from issuerCert, the subject public key from subjectKey, and the cert - * is signed with issuerKey. Fills der/derSz on success. */ + * "digitalSignature"); NULL omits the extension entirely. extKeyUsage does + * the same for ExtendedKeyUsage, taking wc_SetExtKeyUsage() names + * ("serverAuth", "clientAuth", "any", ...) or, with WOLFSSL_EKU_OID, a + * dotted OID. The issuer name is taken from issuerCert, the subject public + * key from subjectKey, and the cert is signed with issuerKey. Fills + * der/derSz on success. */ static int certmanForgeCert(const char* cn, int isCA, const char* keyUsage, + const char* extKeyUsage, const byte* issuerCert, word32 issuerCertSz, ecc_key* issuerKey, ecc_key* subjectKey, byte* der, word32* derSz) { @@ -15817,6 +15821,23 @@ static int certmanForgeCert(const char* cn, int isCA, const char* keyUsage, #endif } } + if (ret == 0 && extKeyUsage != NULL) { + #ifdef WOLFSSL_CERT_EXT + if (WSTRCHR(extKeyUsage, '.') == NULL) { + ret = wc_SetExtKeyUsage(&cert, extKeyUsage); + } + else { + #ifdef WOLFSSL_EKU_OID + ret = wc_SetExtKeyUsageOID(&cert, extKeyUsage, + (word32)WSTRLEN(extKeyUsage), 0, NULL); + #else + ret = BAD_FUNC_ARG; + #endif + } + #else + ret = BAD_FUNC_ARG; + #endif + } if (ret == 0) ret = wc_SetIssuerBuffer(&cert, issuerCert, (int)issuerCertSz); if (ret == 0) { @@ -16058,7 +16079,7 @@ static int certmanCheckIntermediate(const char* interKeyUsage, } /* Intermediate CA signed by the root. */ - ret = certmanForgeCert("IntermediateCA", 1, interKeyUsage, + ret = certmanForgeCert("IntermediateCA", 1, interKeyUsage, NULL, root, rootSz, &rootKey, &interKey, inter, &interSz); if (ret != 0) { printf("CertMan: forge intermediate failed, ret=%d\n", ret); @@ -16066,7 +16087,7 @@ static int certmanCheckIntermediate(const char* interKeyUsage, } /* Leaf signed by the intermediate. */ - ret = certmanForgeCert("ValidLeaf", 0, NULL, inter, interSz, + ret = certmanForgeCert("ValidLeaf", 0, NULL, NULL, inter, interSz, &interKey, &leafKey, leaf, &leafSz); if (ret != 0) { printf("CertMan: forge leaf failed, ret=%d\n", ret); @@ -16134,6 +16155,197 @@ static int test_CertMan_PromoteValidCaIntermediate(void) return result; } +#ifdef WOLFSSL_CERT_EXT + +/* The FPKI profile check runs after the RFC 6187 usage check and never + * matches a forged leaf, so in an FPKI build a leaf that passes the usage + * check surfaces as WS_CERT_PROFILE_E rather than WS_SUCCESS. Either way it + * is distinct from WS_CERT_KEY_USAGE_E. */ +#ifdef WOLFSSH_NO_FPKI + #define CERTMAN_LEAF_USAGE_OK WS_SUCCESS +#else + #define CERTMAN_LEAF_USAGE_OK WS_CERT_PROFILE_E +#endif + +/* Forges a leaf signed by the test root carrying the given KeyUsage and + * ExtendedKeyUsage (NULL omits the extension), verifies it through a manager + * owned by a CTX of the given endpoint, or a standalone manager when side is + * -1, and compares the result with expected. */ +static int certmanCheckLeafUsage(const char* keyUsage, const char* extKeyUsage, + int side, int expected) +{ + int result = 0; + int ret; + byte* root = NULL; + byte* rootKeyBuf = NULL; + word32 rootSz = 0, rootKeySz = 0; + byte leaf[2048]; + word32 leafSz = sizeof(leaf); + byte chain[4096]; + word32 chainSz; + word32 idx; + ecc_key rootKey, leafKey; + int haveRootKey = 0, haveLeafKey = 0; + WC_RNG rng; + int haveRng = 0; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH_CERTMAN* cm = NULL; + + if (certmanLoadFile("./keys/ca-cert-ecc.der", &root, &rootSz) != 0) { + printf("CertMan: can't load root cert\n"); + result = -940; goto done; + } + if (certmanLoadFile("./keys/ca-key-ecc.der", + &rootKeyBuf, &rootKeySz) != 0) { + printf("CertMan: can't load root key\n"); + result = -941; goto done; + } + if (wc_InitRng(&rng) != 0) { + result = -942; goto done; + } + haveRng = 1; + if (wc_ecc_init(&rootKey) != 0) { + result = -943; goto done; + } + haveRootKey = 1; + idx = 0; + if (wc_EccPrivateKeyDecode(rootKeyBuf, &idx, &rootKey, rootKeySz) != 0) { + result = -944; goto done; + } + if (wc_ecc_init(&leafKey) != 0) { + result = -945; goto done; + } + haveLeafKey = 1; + if (wc_ecc_make_key(&rng, 32, &leafKey) != 0) { + result = -946; goto done; + } + + ret = certmanForgeCert("UsageLeaf", 0, keyUsage, extKeyUsage, + root, rootSz, &rootKey, &leafKey, leaf, &leafSz); + if (ret != 0) { + printf("CertMan: forge leaf (KU %s, EKU %s) failed, ret=%d\n", + keyUsage ? keyUsage : "none", + extKeyUsage ? extKeyUsage : "none", ret); + result = -947; goto done; + } + + if (side == WOLFSSH_ENDPOINT_SERVER || side == WOLFSSH_ENDPOINT_CLIENT) { + ctx = wolfSSH_CTX_new(side, NULL); + if (ctx == NULL) { + result = -948; goto done; + } + if (wolfSSH_CTX_AddRootCert_buffer(ctx, root, rootSz, + WOLFSSH_FORMAT_ASN1) != WS_SUCCESS) { + result = -949; goto done; + } + cm = ctx->certMan; + } + else { + cm = wolfSSH_CERTMAN_new(NULL); + if (cm == NULL) { + result = -950; goto done; + } + if (wolfSSH_CERTMAN_LoadRootCA_buffer(cm, root, rootSz) + != WS_SUCCESS) { + result = -951; goto done; + } + } + + chainSz = certmanAppendCert(chain, (word32)sizeof(chain), 0, + leaf, leafSz); + ret = wolfSSH_CERTMAN_VerifyCerts_buffer(cm, chain, chainSz, 1); + if (ret != expected) { + printf("CertMan: leaf KU %s, EKU %s, side %d: got %d, expected %d\n", + keyUsage ? keyUsage : "none", + extKeyUsage ? extKeyUsage : "none", side, ret, expected); + result = -952; goto done; + } + +done: + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + else if (cm != NULL) + wolfSSH_CERTMAN_free(cm); + if (haveRootKey) + wc_ecc_free(&rootKey); + if (haveLeafKey) + wc_ecc_free(&leafKey); + if (haveRng) + wc_FreeRng(&rng); + free(root); + free(rootKeyBuf); + return result; +} + +/* RFC 6187 section 2.2: a leaf KeyUsage must assert digitalSignature and a + * leaf ExtendedKeyUsage must name a purpose for the role being verified. + * The server side verifies user certs (client purpose), the client side host + * certs (server purpose), and a standalone manager accepts either. */ +static int test_CertMan_LeafKeyUsage(void) +{ + static const struct { + const char* keyUsage; + const char* extKeyUsage; + int side; + int expected; + } tv[] = { + { NULL, NULL, WOLFSSH_ENDPOINT_SERVER, CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", NULL, WOLFSSH_ENDPOINT_SERVER, + CERTMAN_LEAF_USAGE_OK }, + { "keyEncipherment", NULL, WOLFSSH_ENDPOINT_SERVER, + WS_CERT_KEY_USAGE_E }, + { "keyEncipherment", NULL, WOLFSSH_ENDPOINT_CLIENT, + WS_CERT_KEY_USAGE_E }, + { "keyEncipherment", "clientAuth", -1, WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "any", WOLFSSH_ENDPOINT_SERVER, + CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "any", WOLFSSH_ENDPOINT_CLIENT, + CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "clientAuth", WOLFSSH_ENDPOINT_SERVER, + CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "clientAuth", WOLFSSH_ENDPOINT_CLIENT, + WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "serverAuth", WOLFSSH_ENDPOINT_SERVER, + WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "serverAuth", WOLFSSH_ENDPOINT_CLIENT, + CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "serverAuth", -1, CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "serverAuth,clientAuth", + WOLFSSH_ENDPOINT_SERVER, CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "codeSigning", WOLFSSH_ENDPOINT_SERVER, + WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "codeSigning", WOLFSSH_ENDPOINT_CLIENT, + WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "codeSigning", -1, WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "emailProtection", WOLFSSH_ENDPOINT_SERVER, + WS_CERT_KEY_USAGE_E }, + { NULL, "emailProtection", WOLFSSH_ENDPOINT_SERVER, + WS_CERT_KEY_USAGE_E }, + #ifdef WOLFSSL_EKU_OID + /* id-kp-secureShellClient and id-kp-secureShellServer */ + { "digitalSignature", "1.3.6.1.5.5.7.3.21", WOLFSSH_ENDPOINT_SERVER, + CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "1.3.6.1.5.5.7.3.21", WOLFSSH_ENDPOINT_CLIENT, + WS_CERT_KEY_USAGE_E }, + { "digitalSignature", "1.3.6.1.5.5.7.3.22", WOLFSSH_ENDPOINT_CLIENT, + CERTMAN_LEAF_USAGE_OK }, + { "digitalSignature", "1.3.6.1.5.5.7.3.22", WOLFSSH_ENDPOINT_SERVER, + WS_CERT_KEY_USAGE_E }, + #endif + }; + int result = 0; + word32 i; + + for (i = 0; result == 0 && i < sizeof(tv) / sizeof(tv[0]); i++) { + result = certmanCheckLeafUsage(tv[i].keyUsage, tv[i].extKeyUsage, + tv[i].side, tv[i].expected); + } + + return result; +} + +#endif /* WOLFSSL_CERT_EXT */ + #endif /* WOLFSSH_TEST_CERTMAN_PROMOTE */ #ifdef WOLFSSH_TEST_SET_CERTMAN @@ -18332,9 +18544,8 @@ static void certChainPut32(word32 v, byte* c) * negotiated x509v3-ecdsa-sha2-nistp384, and an id wcPrimeForId() cannot * map must be rejected up front with WS_INVALID_PRIME_CURVE, matching * ParseECCPubKey() (checked first: it needs no chain verification). - * Skipped (returns 1) when the key files are - * not readable or the chain does not verify in this build's profile (e.g. - * WOLFSSL_FPKI, whose leaf checks these test certs do not meet). */ + * Skipped (returns 1) when the key files are not readable or, in an FPKI + * build, when the FPKI profile rejects the test leaf. */ static int test_ParseECCPubKeyCert(void) { WOLFSSH_CTX* ctx = NULL; @@ -18402,17 +18613,25 @@ static int test_ParseECCPubKeyCert(void) } } - /* matching curve accepted; a failure here means the chain itself did - * not verify under this build's profile, so skip the curve vectors - * rather than fail on unrelated policy */ + /* matching curve accepted. The only tolerated failure is the FPKI + * profile rejecting keys/server-cert.der, which carries none of the + * FPKI extensions; anything else is a parser or verification + * regression and fails the test */ if (result == 0) { ssh->handshake->pubKeyId = ID_X509V3_ECDSA_SHA2_NISTP256; ret = wolfSSH_TestParseECCPubKeyCert(ssh, blob, blobSz); - if (ret != WS_SUCCESS) { - printf("ParseECCPubKeyCert: control chain did not verify " - "(ret %d), skipping curve vectors\n", ret); + #ifndef WOLFSSH_NO_FPKI + if (ret == WS_CERT_PROFILE_E) { + printf("ParseECCPubKeyCert: SKIP, test cert does not meet the " + "FPKI profile\n"); result = 1; } + else + #endif + if (ret != WS_SUCCESS) { + printf("ParseECCPubKeyCert: control chain ret %d\n", ret); + result = -5; + } } #ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP384 /* P-256 leaf offered for a negotiated P-384 algorithm is rejected */ @@ -21139,6 +21358,13 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("CertMan_PromoteValidCaIntermediate: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + +#ifdef WOLFSSL_CERT_EXT + unitResult = test_CertMan_LeafKeyUsage(); + printf("CertMan_LeafKeyUsage: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif #endif #ifdef WOLFSSH_KEYGEN diff --git a/wolfssh/certman.h b/wolfssh/certman.h index 07169cdd..53d036ee 100644 --- a/wolfssh/certman.h +++ b/wolfssh/certman.h @@ -78,6 +78,15 @@ WOLFSSH_API int wolfSSH_CERTMAN_LoadRootCA_buffer(WOLFSSH_CERTMAN* cm, const unsigned char* rootCa, word32 rootCaSz); +/* Verifies a leaf-first chain against the loaded roots and then applies the + * RFC 6187 section 2.2 leaf checks in every build: a KeyUsage extension + * must assert digitalSignature, and an ExtendedKeyUsage extension must name + * anyExtendedKeyUsage or a purpose usable for the SSH role being verified, + * id-kp-secureShellClient or TLS clientAuth for a user certificate and + * id-kp-secureShellServer or TLS serverAuth for a host certificate. Fails + * with WS_CERT_KEY_USAGE_E otherwise. The role comes from the CTX that owns + * the manager; a standalone manager accepts either. Builds with FPKI + * profile matching apply that on top. */ WOLFSSH_API int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm, const unsigned char* cert, word32 certSz, word32 certCount); diff --git a/wolfssh/error.h b/wolfssh/error.h index 531c4a81..ad1c6d2c 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -139,8 +139,10 @@ enum WS_ErrorCodes { WS_DISCONNECT = -1098, /* peer sent disconnect */ WS_MLDSA_E = -1099, /* MLDSA failure */ WS_ED448_E = -1100, /* Ed448 failure */ + WS_CERT_KEY_USAGE_E = -1101, /* Cert (ext)KeyUsage not for SSH */ - WS_LAST_E = WS_ED448_E /* Update to indicate last error */ + WS_LAST_E = WS_CERT_KEY_USAGE_E /* Update to indicate + * last error */ }; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index e8804b79..3618d267 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -834,6 +834,13 @@ typedef struct WOLFSSH_PVT_KEY { WOLFSSH_LOCAL int wolfSSH_CertStoreLocationValid(word32 dwFlags); #endif +#ifdef WOLFSSH_CERTS +/* Records the local endpoint (WOLFSSH_ENDPOINT_SERVER or _CLIENT) so the + * RFC 6187 key purpose check knows whether a user or host certificate is + * being verified. Unset accepts either. Defined in certman.c. */ +WOLFSSH_LOCAL void wolfSSH_CERTMAN_SetSide(WOLFSSH_CERTMAN* cm, int side); +#endif + /* our wolfSSH Context */ struct WOLFSSH_CTX {