certman: enforce leaf is end-entity in VerifyCerts_buffer

[yosuke-wolfssl]

certman: enforce leaf is an end-entity certificate in VerifyCerts_buffer

Problem

wolfSSH_CERTMAN_VerifyCerts_buffer did not enforce that the leaf
(certLoc[0]) is an end-entity certificate. The only leaf isCA check lived
inside CheckProfile, which is gated by #ifndef WOLFSSH_NO_FPKI:

/* CheckProfile, only compiled when FPKI is enabled */
valid = !cert->isCA;

In a WOLFSSH_CERTS && WOLFSSH_NO_FPKI build, that block is absent, so an
attacker holding any CA certificate in the deployed trust chain (plus its
private key) could present that CA as the leaf. It passes
wolfSSL_CertManagerVerifyBuffer signature validation and the function returns
WS_SUCCESS — a certificate-based authentication bypass (CWE-295).

The wolfsshd-level mitigation (fail-closed when no AuthorizedKeysFile is 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:

  else if (decoded->isCA) {
      WLOG(WS_LOG_CERTMAN, "leaf certificate is a CA; rejecting");
      ret = WS_CERT_PROFILE_E;
  }

• Under FPKI, the same decoded cert additionally must match a profile
  (CheckProfile), via an else if. The leaf is parsed with cm->cm so the
  signer (ca) is resolved for CheckProfile's issuer-DN match.

• Removed the now-redundant !cert->isCA check from CheckProfile (the caller
  enforces it unconditionally before CheckProfile runs).

• Converted both DecodedCert usages (the leaf block and
  CertManIntermediateIsCA) to the WOLFSSH_SMALL_STACK heap-allocation
  pattern, with a distinct log on the allocation-failure (fail-closed) path.

Tests

tests/api.c (test_wolfSSH_CertMan, guarded #ifndef WOLFSSH_NO_ECDSA):

• Negative control — a trusted CA presented as the leaf is rejected with
  WS_CERT_PROFILE_E. Runs in both FPKI-enabled and non-FPKI builds.
• Positive control (non-FPKI builds) — a genuine end-entity leaf signed by
  the CA still verifies with WS_SUCCESS.
• Added a local certman_make_chain() helper for the length-prefixed chain
  framing to avoid duplicated byte-shift encoding.

Verification

• Negative control proven to catch the bug: with the fix reverted, the
  CA-as-leaf case returns WS_SUCCESS and the test aborts; with the fix it
  returns WS_CERT_PROFILE_E.
• api.test passes under: WOLFSSH_NO_FPKI (default + small-stack), and the
  default FPKI-active build.
• src/certman.c compiles warning-free with -Wall -Wextra in both
  small-stack and default-stack configurations.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1075

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: COMMENT
Findings: 2 total — 2 posted, 1 skipped

Posted findings

• [Low] Leaf certificate is now fully re-parsed on every verification, including non-FPKI builds — src/certman.c:404-424
• [Info] Added multi-line explanatory comment blocks — src/certman.c:387-389,242-244,480-481

Skipped findings

• [Info] Added multi-line explanatory comment blocks

Review generated by Skoll.

[aidangarske]

🔵 [Low] Leaf certificate is now fully re-parsed on every verification, including non-FPKI builds
💡 SUGGEST question

Previously the leaf decode (wc_InitDecodedCert/wc_ParseCert) only happened inside #ifndef WOLFSSH_NO_FPKI. The fix correctly makes the CA-leaf rejection unconditional, but as a side effect every successful verification now performs an additional full wc_ParseCert of certLoc[0] (on top of the parse already done inside wolfSSL_CertManagerVerifyBuffer). This is the intended security tradeoff and the cost is small, but it is worth confirming it is acceptable on constrained targets. If wc_ParseCert were ever to fail for a leaf that already passed CertManagerVerifyBuffer, a previously-successful (non-FPKI) verification would now return WS_CERT_OTHER_E. In practice CertManagerVerifyBuffer already parses the same buffer, so this is very unlikely, but it is a behavioral change for the non-FPKI path.

Recommendation: Confirm the extra leaf parse per verification is acceptable for the target platforms. No code change required — the security benefit (unconditional CA-leaf rejection) justifies the cost.