From 6c55a8e5aa91d0824b82ef4071bae2b19f96fa52 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Fri, 2 Oct 2026 16:13:33 +0900 Subject: [PATCH] certman: require keyCertSign on self-issued peer intermediates - CertManIntermediateIsCA() demotes a CA intermediate whose KeyUsage extension lacks keyCertSign whether or not it is self-issued. - certmanForgeCert() takes the subject name from issuerCert when cn is NULL. - certmanIsSelfIssued() reports whether a cert's issuer and subject name hashes match. - certmanCheckIntermediate() gains selfIssued, which gives the intermediate the root's subject name and fails with -935 when the forged cert is not self-issued, and appendRoot, which also sends the trusted root at the end of the chain. - test_CertMan_PromoteValidCaIntermediate() covers a self-issued intermediate with and without keyCertSign, and a chain that ends in the trusted root. --- src/certman.c | 7 +++-- tests/unit.c | 76 ++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/src/certman.c b/src/certman.c index 11c711138..62fcf821f 100644 --- a/src/certman.c +++ b/src/certman.c @@ -313,8 +313,9 @@ enum { #define MAX_CHAIN_DEPTH 9 #endif -/* Returns 1 if der is a CA: isCA set and, unless self-signed, keyCertSign - * set. Already signature-verified by the caller, so parse NO_VERIFY. */ +/* Returns 1 if der is a CA: isCA set and, when a KeyUsage extension is + * present, keyCertSign set. Already signature-verified by the caller, so + * parse NO_VERIFY. */ static int CertManIntermediateIsCA(WOLFSSH_CERTMAN* cm, const unsigned char* der, word32 derSz) { @@ -336,7 +337,7 @@ static int CertManIntermediateIsCA(WOLFSSH_CERTMAN* cm, if (wc_ParseCert(decoded, WOLFSSL_FILETYPE_ASN1, NO_VERIFY, NULL) == 0) { isCA = decoded->isCA; #ifndef ALLOW_INVALID_CERTSIGN - if (isCA && !decoded->selfSigned && decoded->extKeyUsageSet && + if (isCA && decoded->extKeyUsageSet && (decoded->extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0) { /* If a KeyUsage extension is present, an intermediate CA must * assert the keyCertSign bit. */ diff --git a/tests/unit.c b/tests/unit.c index 8dfa18669..d12ca5698 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -17766,7 +17766,8 @@ static int certmanForgeChild(const byte* issuerCert, word32 issuerCertSz, return ret; } -/* Forge a cert with the given subject CN. When isCA is set the cert asserts +/* Forge a cert with the given subject CN, or with issuerCert's subject name + * when cn is NULL (a self-issued cert). 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. extKeyUsage does @@ -17795,12 +17796,17 @@ static int certmanForgeCert(const char* cn, int isCA, const char* keyUsage, /* wc_InitCert zeroed the struct, so leaving the final byte untouched * keeps the name NUL-terminated and avoids a strncpy truncation * warning on the runtime cn pointer. */ - WSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE - 1); - WSTRNCPY(cert.subject.commonName, cn, CTC_NAME_SIZE - 1); + if (cn != NULL) { + WSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE - 1); + WSTRNCPY(cert.subject.commonName, cn, CTC_NAME_SIZE - 1); + } + else { + ret = wc_SetSubjectBuffer(&cert, issuerCert, (int)issuerCertSz); + } cert.sigType = CTC_SHA256wECDSA; cert.daysValid = 365; cert.isCA = isCA; - if (keyUsage != NULL) { + if (ret == 0 && keyUsage != NULL) { #ifdef WOLFSSL_CERT_EXT ret = wc_SetKeyUsage(&cert, keyUsage); #else @@ -17985,6 +17991,25 @@ static int test_CertMan_NoPromoteNonCaIntermediate(void) return result; } +/* Returns 1 when der's issuer name equals its subject name. Compares the name + * hashes, since DecodedCert.selfSigned also requires a self-signature in + * some wolfSSL versions. */ +static int certmanIsSelfIssued(const byte* der, word32 derSz) +{ + DecodedCert decoded; + int ret = 0; + + wc_InitDecodedCert(&decoded, der, derSz, NULL); + if (wc_ParseCert(&decoded, CERT_TYPE, NO_VERIFY, NULL) == 0 && + WMEMCMP(decoded.issuerHash, decoded.subjectHash, + sizeof(decoded.subjectHash)) == 0) { + ret = 1; + } + wc_FreeDecodedCert(&decoded); + + return ret; +} + /* Drives CertManIntermediateIsCA through a forged [leaf <- intermediate] chain * with only the root trusted, so the intermediate CA must be promoted for the * leaf to find a signer. @@ -17997,6 +18022,12 @@ static int test_CertMan_NoPromoteNonCaIntermediate(void) * "digitalSignature" has KeyUsage but lacks keyCertSign: intermediate * CA must be demoted (keyCertSign-rejection branch). * + * selfIssued gives the intermediate the root's subject name while it keeps its + * own key and the root's signature, the shape of a CA key rollover cert. + * + * appendRoot also sends the trusted root at the end of the chain, so it goes + * through CertManIntermediateIsCA too and must be promoted as well. + * * expectPromote==1 asserts the intermediate was promoted (verify returns * anything but WS_CERT_NO_SIGNER_E); ==0 asserts it was not (verify returns * WS_CERT_NO_SIGNER_E). Promotion is the unit under test, not full chain @@ -18004,8 +18035,8 @@ static int test_CertMan_NoPromoteNonCaIntermediate(void) * synthetic leaf is rejected later with WS_CERT_PROFILE_E, which is * orthogonal -- hence the "anything but WS_CERT_NO_SIGNER_E" success criterion * mirroring the negative test's sanity check. */ -static int certmanCheckIntermediate(const char* interKeyUsage, - int expectPromote) +static int certmanCheckIntermediate(const char* interKeyUsage, int selfIssued, + int appendRoot, int expectPromote) { int result = 0; int ret; @@ -18066,12 +18097,17 @@ static int certmanCheckIntermediate(const char* interKeyUsage, } /* Intermediate CA signed by the root. */ - ret = certmanForgeCert("IntermediateCA", 1, interKeyUsage, NULL, - root, rootSz, &rootKey, &interKey, inter, &interSz); + ret = certmanForgeCert(selfIssued ? NULL : "IntermediateCA", 1, + interKeyUsage, NULL, root, rootSz, &rootKey, &interKey, inter, + &interSz); if (ret != 0) { printf("CertMan: forge intermediate failed, ret=%d\n", ret); result = -929; goto done; } + if (selfIssued && !certmanIsSelfIssued(inter, interSz)) { + printf("CertMan: forged intermediate is not self-issued\n"); + result = -935; goto done; + } /* Leaf signed by the intermediate. */ ret = certmanForgeCert("ValidLeaf", 0, NULL, NULL, inter, interSz, @@ -18095,7 +18131,12 @@ static int certmanCheckIntermediate(const char* interKeyUsage, leaf, leafSz); chainSz = certmanAppendCert(chain, (word32)sizeof(chain), chainSz, inter, interSz); - ret = wolfSSH_CERTMAN_VerifyCerts_buffer(cm, chain, chainSz, 2); + if (appendRoot) { + chainSz = certmanAppendCert(chain, (word32)sizeof(chain), chainSz, + root, rootSz); + } + ret = wolfSSH_CERTMAN_VerifyCerts_buffer(cm, chain, chainSz, + appendRoot ? 3 : 2); if (expectPromote && ret == WS_CERT_NO_SIGNER_E) { printf("CertMan: valid intermediate CA not promoted, ret=%d\n", ret); result = -933; goto done; @@ -18127,16 +18168,25 @@ static int test_CertMan_PromoteValidCaIntermediate(void) int result; /* A CA intermediate with no KeyUsage extension must still be promoted. */ - result = certmanCheckIntermediate(NULL, 1); + result = certmanCheckIntermediate(NULL, 0, 0, 1); + /* A peer may send the trusted root too; it asserts keyCertSign, so it is + * promoted like any other CA. */ + if (result == 0) + result = certmanCheckIntermediate(NULL, 0, 1, 1); #ifdef WOLFSSL_CERT_EXT /* As must a CA intermediate that explicitly asserts keyCertSign. */ if (result == 0) - result = certmanCheckIntermediate("keyCertSign", 1); + result = certmanCheckIntermediate("keyCertSign", 0, 0, 1); + if (result == 0) + result = certmanCheckIntermediate("keyCertSign", 1, 0, 1); #ifndef ALLOW_INVALID_CERTSIGN /* But a CA intermediate carrying a KeyUsage extension that omits - * keyCertSign must be rejected (the keyCertSign-rejection branch). */ + * keyCertSign must be rejected (the keyCertSign-rejection branch), + * self-issued or not. */ + if (result == 0) + result = certmanCheckIntermediate("digitalSignature", 0, 0, 0); if (result == 0) - result = certmanCheckIntermediate("digitalSignature", 0); + result = certmanCheckIntermediate("digitalSignature", 1, 0, 0); #endif #endif return result;