certman: enforce leaf is end-entity in VerifyCerts_buffer#1075
Open
yosuke-wolfssl wants to merge 1 commit into
Open
certman: enforce leaf is end-entity in VerifyCerts_buffer#1075yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1075
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
certman: enforce leaf is an end-entity certificate in VerifyCerts_buffer
Problem
wolfSSH_CERTMAN_VerifyCerts_bufferdid not enforce that the leaf(
certLoc[0]) is an end-entity certificate. The only leafisCAcheck livedinside
CheckProfile, which is gated by#ifndef WOLFSSH_NO_FPKI:In a
WOLFSSH_CERTS && WOLFSSH_NO_FPKIbuild, that block is absent, so anattacker holding any CA certificate in the deployed trust chain (plus its
private key) could present that CA as the leaf. It passes
wolfSSL_CertManagerVerifyBuffersignature validation and the function returnsWS_SUCCESS— a certificate-based authentication bypass (CWE-295).The wolfsshd-level mitigation (fail-closed when no
AuthorizedKeysFileis set)was already in place from earlier commits, but the bypass existed at the
library API, which is reachable by any consumer of
WOLFSSH_CERTMAN.Addressed by f_5899.
Fix
src/certman.c:Decode the leaf once and reject it unconditionally when it asserts CA
basic constraints, regardless of FPKI:
Under FPKI, the same decoded cert additionally must match a profile
(
CheckProfile), via anelse if. The leaf is parsed withcm->cmso thesigner (
ca) is resolved forCheckProfile's issuer-DN match.Removed the now-redundant
!cert->isCAcheck fromCheckProfile(the callerenforces it unconditionally before
CheckProfileruns).Converted both
DecodedCertusages (the leaf block andCertManIntermediateIsCA) to theWOLFSSH_SMALL_STACKheap-allocationpattern, with a distinct log on the allocation-failure (fail-closed) path.
Tests
tests/api.c(test_wolfSSH_CertMan, guarded#ifndef WOLFSSH_NO_ECDSA):WS_CERT_PROFILE_E. Runs in both FPKI-enabled and non-FPKI builds.the CA still verifies with
WS_SUCCESS.certman_make_chain()helper for the length-prefixed chainframing to avoid duplicated byte-shift encoding.
Verification
CA-as-leaf case returns
WS_SUCCESSand the test aborts; with the fix itreturns
WS_CERT_PROFILE_E.api.testpasses under:WOLFSSH_NO_FPKI(default + small-stack), and thedefault FPKI-active build.
src/certman.ccompiles warning-free with-Wall -Wextrain bothsmall-stack and default-stack configurations.