Skip to content

certman: enforce leaf is end-entity in VerifyCerts_buffer #1075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

certman: enforce leaf is end-entity in VerifyCerts_buffer #1075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
102 changes: 69 additions & 33 deletions src/certman.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,22 +208,42 @@ enum {
static int CertManIntermediateIsCA(WOLFSSH_CERTMAN* cm,
const unsigned char* der, word32 derSz)
{
DecodedCert decoded;
DecodedCert* decoded = NULL;
#ifndef WOLFSSH_SMALL_STACK
DecodedCert decoded_s;
#endif
int isCA = 0;

wc_InitDecodedCert(&decoded, der, derSz, cm->heap);
if (wc_ParseCert(&decoded, WOLFSSL_FILETYPE_ASN1, NO_VERIFY, NULL) == 0) {
isCA = decoded.isCA;
#ifndef ALLOW_INVALID_CERTSIGN
if (isCA && !decoded.selfSigned && decoded.extKeyUsageSet &&
(decoded.extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) {
/* If a KeyUsage extension is present, an intermediate CA must
* assert the keyCertSign bit. */
isCA = 0;
#ifndef WOLFSSH_SMALL_STACK
decoded = &decoded_s;
#else
decoded = (DecodedCert*)WMALLOC(sizeof(DecodedCert), cm->heap,
DYNTYPE_CERT);
#endif

if (decoded != NULL) {
wc_InitDecodedCert(decoded, der, derSz, cm->heap);
if (wc_ParseCert(decoded, WOLFSSL_FILETYPE_ASN1, NO_VERIFY, NULL) == 0) {
isCA = decoded->isCA;
#ifndef ALLOW_INVALID_CERTSIGN
if (isCA && !decoded->selfSigned && decoded->extKeyUsageSet &&
(decoded->extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) {
/* If a KeyUsage extension is present, an intermediate CA must
* assert the keyCertSign bit. */
isCA = 0;
}
#endif
}
wc_FreeDecodedCert(decoded);
#ifdef WOLFSSH_SMALL_STACK
WFREE(decoded, cm->heap, DYNTYPE_CERT);
#endif
}
wc_FreeDecodedCert(&decoded);
else {
/* allocation failed; fail closed (not a CA) but log the real cause so
* it is not mistaken for a genuine non-CA intermediate */
WLOG(WS_LOG_CERTMAN, "could not allocate cert to check intermediate CA");
}

return isCA;
}
Expand Down Expand Up @@ -364,32 +384,51 @@ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm,
}
}

#ifndef WOLFSSH_NO_FPKI
/* FPKI checking on the leaf certificate */
/* 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. */
if (ret == WS_SUCCESS) {
DecodedCert decoded;
DecodedCert* decoded = NULL;
#ifndef WOLFSSH_SMALL_STACK
DecodedCert decoded_s;

wc_InitDecodedCert(&decoded, certLoc[0], certLen[0], cm->cm);
ret = wc_ParseCert(&decoded, WOLFSSL_FILETYPE_ASN1, 0, cm->cm);

if (ret == 0) {
ret =
CheckProfile(&decoded, PROFILE_FPKI_WORKSHEET_6) ||
CheckProfile(&decoded, PROFILE_FPKI_WORKSHEET_10) ||
CheckProfile(&decoded, PROFILE_FPKI_WORKSHEET_16);
decoded = &decoded_s;
#else
decoded = (DecodedCert*)WMALLOC(sizeof(DecodedCert), cm->heap,
DYNTYPE_CERT);
if (decoded == NULL) {
ret = WS_MEMORY_E;
}
#endif

if (ret == 0) {
WLOG(WS_LOG_CERTMAN, "certificate didn't match profile");
if (ret == WS_SUCCESS) {

@aidangarske aidangarske Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔵 [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.

wc_InitDecodedCert(decoded, certLoc[0], certLen[0], cm->heap);
if (wc_ParseCert(decoded, WOLFSSL_FILETYPE_ASN1, NO_VERIFY, cm->cm)
!= 0) {
WLOG(WS_LOG_CERTMAN, "unable to parse leaf certificate");
ret = WS_CERT_OTHER_E;
}
else if (decoded->isCA) {
WLOG(WS_LOG_CERTMAN, "leaf certificate is a CA; rejecting");
ret = WS_CERT_PROFILE_E;
}
else {
ret = WS_SUCCESS;
#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;
}
#endif /* WOLFSSH_NO_FPKI */
wc_FreeDecodedCert(decoded);
}

FreeDecodedCert(&decoded);
#ifdef WOLFSSH_SMALL_STACK
if (decoded != NULL) {
WFREE(decoded, cm->heap, DYNTYPE_CERT);
}
#endif
}
#endif /* WOLFSSH_NO_FPKI */

if (certLoc != NULL)
WFREE(certLoc, cm->heap, DYNTYPE_CERT);
Expand Down Expand Up @@ -438,11 +477,8 @@ static int CheckProfile(DecodedCert* cert, int profile)
WLOG(WS_LOG_CERTMAN, "cert country of citizenship invalid");
}

if (valid) {
valid = !cert->isCA;
if (valid != 1)
WLOG(WS_LOG_CERTMAN, "cert basic constraint invalid");
}
/* leaf isCA (basic constraint) is enforced unconditionally by the caller
* before CheckProfile runs, so it is not re-checked here */

if (valid) {
valid =
Expand Down
89 changes: 89 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,34 @@ static void test_wolfSSH_CTX_SetWindowPacketSize(void)
}


#if defined(WOLFSSH_CERTS) && !defined(WOLFSSH_NO_ECDSA)
/* Build the length-prefixed single-cert chain buffer that
* wolfSSH_CERTMAN_VerifyCerts_buffer expects. Caller frees *chain. */
static int certman_make_chain(const byte* cert, word32 certSz,
byte** chain, word32* chainSz)
{
int ret = 0;
byte* buf;

buf = (byte*)malloc(UINT32_SZ + certSz);
if (buf != NULL) {
buf[0] = (byte)(certSz >> 24);
buf[1] = (byte)(certSz >> 16);
buf[2] = (byte)(certSz >> 8);
buf[3] = (byte)(certSz);
memcpy(buf + UINT32_SZ, cert, certSz);
*chain = buf;
*chainSz = UINT32_SZ + certSz;
}
else {
ret = -1;
}

return ret;
}
#endif /* WOLFSSH_CERTS && !WOLFSSH_NO_ECDSA */


static void test_wolfSSH_CertMan(void)
{
#ifdef WOLFSSH_CERTMAN
Expand Down Expand Up @@ -751,6 +779,67 @@ static void test_wolfSSH_CertMan(void)

wolfSSH_CERTMAN_free(cm);
}
/* ECC trust anchor and leaf, so guard on ECDSA like the sibling tests. */
#ifndef WOLFSSH_NO_ECDSA
{
/* Negative control: a trusted CA presented as the leaf (index 0) must
* be rejected as an end-entity cert in both FPKI and non-FPKI builds;
* otherwise a trusted CA could be used to bypass authentication. */
WOLFSSH_CERTMAN* cm;
byte* caCert = NULL;
byte* chain = NULL;
word32 caCertSz = 0;
word32 chainSz;

cm = wolfSSH_CERTMAN_new(NULL);
AssertNotNull(cm);

AssertIntEQ(0, load_file("./keys/ca-cert-ecc.der", &caCert, &caCertSz));
AssertIntEQ(WS_SUCCESS,
wolfSSH_CERTMAN_LoadRootCA_buffer(cm, caCert, caCertSz));

AssertIntEQ(0, certman_make_chain(caCert, caCertSz, &chain, &chainSz));
AssertIntEQ(WS_CERT_PROFILE_E,
wolfSSH_CERTMAN_VerifyCerts_buffer(cm, chain, chainSz, 1));

free(chain);
free(caCert);
wolfSSH_CERTMAN_free(cm);
}
#ifdef WOLFSSH_NO_FPKI
{
/* Positive control: a genuine end-entity leaf signed by the CA still
* verifies. The leaf is not an FPKI cert, so only assert this when
* FPKI profile checking is compiled out. */
WOLFSSH_CERTMAN* cm;
byte* caCert = NULL;
byte* leafCert = NULL;
byte* chain = NULL;
word32 caCertSz = 0;
word32 leafCertSz = 0;
word32 chainSz;

cm = wolfSSH_CERTMAN_new(NULL);
AssertNotNull(cm);

AssertIntEQ(0, load_file("./keys/ca-cert-ecc.der", &caCert, &caCertSz));
AssertIntEQ(WS_SUCCESS,
wolfSSH_CERTMAN_LoadRootCA_buffer(cm, caCert, caCertSz));

AssertIntEQ(0,
load_file("./keys/fred-cert.der", &leafCert, &leafCertSz));
AssertIntEQ(0,
certman_make_chain(leafCert, leafCertSz, &chain, &chainSz));
AssertIntEQ(WS_SUCCESS,
wolfSSH_CERTMAN_VerifyCerts_buffer(cm, chain, chainSz, 1));

free(chain);
free(caCert);
free(leafCert);
wolfSSH_CERTMAN_free(cm);
}
#endif /* WOLFSSH_NO_FPKI */
#endif /* WOLFSSH_NO_ECDSA */
#endif /* WOLFSSH_CERTS */
}

Expand Down
Loading