Skip to content

Commit 6fdd0de

Browse files
committed
Fix handling of otherName in ConfirmNameConstraints
1 parent 1c9555c commit 6fdd0de

3 files changed

Lines changed: 399 additions & 13 deletions

File tree

tests/api.c

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22193,6 +22193,262 @@ static int test_MakeCertWith0Ser(void)
2219322193
return EXPECT_RESULT();
2219422194
}
2219522195

22196+
#if defined(WOLFSSL_CERT_REQ) && !defined(NO_ASN_TIME) && \
22197+
defined(WOLFSSL_CERT_GEN) && defined(HAVE_ECC) && \
22198+
defined(WOLFSSL_CERT_EXT) && !defined(NO_CERTS) && \
22199+
defined(WOLFSSL_ALT_NAMES) && defined(WOLFSSL_CUSTOM_OID) && \
22200+
defined(HAVE_OID_ENCODING) && !defined(IGNORE_NAME_CONSTRAINTS)
22201+
22202+
/* Build a SubjectAltName extension value (a SEQUENCE wrapping a single
22203+
* otherName GeneralName) for the Microsoft UPN OID 1.3.6.1.4.1.311.20.2.3
22204+
* with the given 7-byte UTF8String value. */
22205+
static word32 build_otherName_san(byte* out, word32 outSz, const char* val7)
22206+
{
22207+
static const byte prefix[] = {
22208+
0x30, 0x19, /* SEQUENCE, 25 */
22209+
0xA0, 0x17, /* [0] CONSTRUCTED, 23 */
22210+
0x06, 0x0A, /* OBJECT ID, 10 */
22211+
0x2B, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37,
22212+
0x14, 0x02, 0x03, /* UPN OID */
22213+
0xA0, 0x09, /* [0] EXPLICIT, 9 */
22214+
0x0C, 0x07 /* UTF8String, 7 */
22215+
};
22216+
if (outSz < sizeof(prefix) + 7)
22217+
return 0;
22218+
XMEMCPY(out, prefix, sizeof(prefix));
22219+
XMEMCPY(out + sizeof(prefix), val7, 7);
22220+
return (word32)(sizeof(prefix) + 7);
22221+
}
22222+
22223+
/* Build a NameConstraints extension value carrying a single subtree of
22224+
* the given list type ([0] permitted or [1] excluded) for an otherName
22225+
* UPN whose UTF8 value is the given 7-byte string. */
22226+
static word32 build_otherName_nameConstraints(byte* out, word32 outSz,
22227+
int excluded, const char* val7)
22228+
{
22229+
static const byte common[] = {
22230+
0x30, 0x1D, /* SEQUENCE, 29 */
22231+
0x00, 0x1B, /* listTag, 27 (patched) */
22232+
0x30, 0x19, /* GeneralSubtree, 25 */
22233+
0xA0, 0x17, /* [0] CONSTRUCTED, 23 */
22234+
0x06, 0x0A,
22235+
0x2B, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37,
22236+
0x14, 0x02, 0x03,
22237+
0xA0, 0x09,
22238+
0x0C, 0x07
22239+
};
22240+
if (outSz < sizeof(common) + 7)
22241+
return 0;
22242+
XMEMCPY(out, common, sizeof(common));
22243+
out[2] = excluded ? 0xA1 : 0xA0; /* listTag */
22244+
XMEMCPY(out + sizeof(common), val7, 7);
22245+
return (word32)(sizeof(common) + 7);
22246+
}
22247+
22248+
/* Build a chain (root -> intermediate -> leaf) where the intermediate
22249+
* carries `nameConstraintsDer` as a (possibly critical) nameConstraints
22250+
* extension and the leaf carries `sanDer` as its SAN. Loads root and
22251+
* intermediate as trusted CAs into a fresh CertManager, parses the leaf
22252+
* with VERIFY, and returns the result code from wc_ParseCert(). */
22253+
static int verify_with_otherName_chain(const byte* nameConstraintsDer,
22254+
word32 nameConstraintsDerSz, int critical,
22255+
const byte* sanDer, word32 sanDerSz)
22256+
{
22257+
Cert cert;
22258+
DecodedCert decodedCert;
22259+
byte rootDer[FOURK_BUF];
22260+
byte icaDer[FOURK_BUF];
22261+
byte leafDer[FOURK_BUF];
22262+
int rootDerSz = 0, icaDerSz = 0, leafDerSz = 0;
22263+
int parseRet = -1;
22264+
WC_RNG rng;
22265+
ecc_key rootKey, icaKey, leafKey;
22266+
WOLFSSL_CERT_MANAGER* cm = NULL;
22267+
22268+
XMEMSET(&rng, 0, sizeof(rng));
22269+
XMEMSET(&rootKey, 0, sizeof(rootKey));
22270+
XMEMSET(&icaKey, 0, sizeof(icaKey));
22271+
XMEMSET(&leafKey, 0, sizeof(leafKey));
22272+
22273+
if (wc_InitRng(&rng) != 0) goto done;
22274+
if (wc_ecc_init(&rootKey) != 0) goto done;
22275+
if (wc_ecc_init(&icaKey) != 0) goto done;
22276+
if (wc_ecc_init(&leafKey) != 0) goto done;
22277+
if (wc_ecc_make_key(&rng, 32, &rootKey) != 0) goto done;
22278+
if (wc_ecc_make_key(&rng, 32, &icaKey) != 0) goto done;
22279+
if (wc_ecc_make_key(&rng, 32, &leafKey) != 0) goto done;
22280+
22281+
/* Self-signed root. */
22282+
if (wc_InitCert(&cert) != 0) goto done;
22283+
(void)XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE);
22284+
(void)XSTRNCPY(cert.subject.org, "OtherNCRoot", CTC_NAME_SIZE);
22285+
(void)XSTRNCPY(cert.subject.commonName, "OtherNCRoot", CTC_NAME_SIZE);
22286+
cert.selfSigned = 1;
22287+
cert.isCA = 1;
22288+
cert.sigType = CTC_SHA256wECDSA;
22289+
cert.keyUsage = KEYUSE_KEY_CERT_SIGN | KEYUSE_CRL_SIGN;
22290+
if (wc_SetSubjectKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &rootKey) != 0)
22291+
goto done;
22292+
if (wc_MakeCert(&cert, rootDer, FOURK_BUF, NULL, &rootKey, &rng) < 0)
22293+
goto done;
22294+
rootDerSz = wc_SignCert(cert.bodySz, cert.sigType, rootDer, FOURK_BUF,
22295+
NULL, &rootKey, &rng);
22296+
if (rootDerSz < 0) goto done;
22297+
22298+
/* Intermediate, signed by root, carrying nameConstraints. */
22299+
if (wc_InitCert(&cert) != 0) goto done;
22300+
cert.selfSigned = 0;
22301+
cert.isCA = 1;
22302+
cert.sigType = CTC_SHA256wECDSA;
22303+
cert.keyUsage = KEYUSE_KEY_CERT_SIGN | KEYUSE_CRL_SIGN;
22304+
(void)XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE);
22305+
(void)XSTRNCPY(cert.subject.org, "OtherNCICA", CTC_NAME_SIZE);
22306+
(void)XSTRNCPY(cert.subject.commonName, "OtherNCICA", CTC_NAME_SIZE);
22307+
if (wc_SetIssuerBuffer(&cert, rootDer, rootDerSz) != 0) goto done;
22308+
if (wc_SetAuthKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &rootKey) != 0)
22309+
goto done;
22310+
if (wc_SetSubjectKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &icaKey) != 0)
22311+
goto done;
22312+
if (nameConstraintsDer != NULL) {
22313+
/* nameConstraints OID = 2.5.29.30 */
22314+
if (wc_SetCustomExtension(&cert, critical ? 1 : 0, "2.5.29.30",
22315+
nameConstraintsDer, nameConstraintsDerSz) != 0)
22316+
goto done;
22317+
}
22318+
if (wc_MakeCert(&cert, icaDer, FOURK_BUF, NULL, &icaKey, &rng) < 0)
22319+
goto done;
22320+
icaDerSz = wc_SignCert(cert.bodySz, cert.sigType, icaDer, FOURK_BUF,
22321+
NULL, &rootKey, &rng);
22322+
if (icaDerSz < 0) goto done;
22323+
22324+
/* Leaf, signed by intermediate, carrying the otherName SAN. */
22325+
if (wc_InitCert(&cert) != 0) goto done;
22326+
cert.selfSigned = 0;
22327+
cert.isCA = 0;
22328+
cert.sigType = CTC_SHA256wECDSA;
22329+
(void)XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE);
22330+
(void)XSTRNCPY(cert.subject.org, "OtherNCLeaf", CTC_NAME_SIZE);
22331+
(void)XSTRNCPY(cert.subject.commonName, "OtherNCLeaf", CTC_NAME_SIZE);
22332+
if (wc_SetIssuerBuffer(&cert, icaDer, icaDerSz) != 0) goto done;
22333+
if (wc_SetAuthKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &icaKey) != 0)
22334+
goto done;
22335+
if (wc_SetSubjectKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &leafKey) != 0)
22336+
goto done;
22337+
if (sanDerSz > sizeof(cert.altNames)) goto done;
22338+
XMEMCPY(cert.altNames, sanDer, sanDerSz);
22339+
cert.altNamesSz = (int)sanDerSz;
22340+
if (wc_MakeCert(&cert, leafDer, FOURK_BUF, NULL, &leafKey, &rng) < 0)
22341+
goto done;
22342+
leafDerSz = wc_SignCert(cert.bodySz, cert.sigType, leafDer, FOURK_BUF,
22343+
NULL, &icaKey, &rng);
22344+
if (leafDerSz < 0) goto done;
22345+
22346+
cm = wolfSSL_CertManagerNew();
22347+
if (cm == NULL) goto done;
22348+
if (wolfSSL_CertManagerLoadCABuffer(cm, rootDer, rootDerSz,
22349+
WOLFSSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS) goto done;
22350+
if (wolfSSL_CertManagerLoadCABuffer(cm, icaDer, icaDerSz,
22351+
WOLFSSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS) goto done;
22352+
22353+
wc_InitDecodedCert(&decodedCert, leafDer, (word32)leafDerSz, NULL);
22354+
parseRet = wc_ParseCert(&decodedCert, CERT_TYPE, VERIFY, cm);
22355+
wc_FreeDecodedCert(&decodedCert);
22356+
22357+
done:
22358+
if (cm != NULL) wolfSSL_CertManagerFree(cm);
22359+
wc_ecc_free(&leafKey);
22360+
wc_ecc_free(&icaKey);
22361+
wc_ecc_free(&rootKey);
22362+
wc_FreeRng(&rng);
22363+
return parseRet;
22364+
}
22365+
#endif
22366+
22367+
/* Verifies wolfSSL enforces an issuing CA's nameConstraints extension on a
22368+
* leaf certificate's otherName SAN (RFC 5280 4.2.1.10). The vulnerability
22369+
* was that ConfirmNameConstraints() ignored ASN_OTHER_TYPE entirely, so a
22370+
* malicious intermediate could issue leaves whose otherName SAN violated
22371+
* its own subtree.
22372+
*
22373+
* Coverage:
22374+
* 1. Critical excluded subtree, leaf SAN matches -> reject
22375+
* 2. Critical excluded subtree, leaf SAN does NOT match -> accept
22376+
* (positive control: distinguishes 'right rule fired' from
22377+
* 'broke everything with otherName')
22378+
* 3. Non-critical excluded subtree, leaf SAN matches -> reject
22379+
* (excluded is enforced regardless of criticality)
22380+
* 4. Critical permitted subtree, leaf SAN matches -> accept
22381+
* 5. Critical permitted subtree, leaf SAN does NOT match -> reject
22382+
*/
22383+
static int test_NameConstraints_OtherName(void)
22384+
{
22385+
EXPECT_DECLS;
22386+
#if defined(WOLFSSL_CERT_REQ) && !defined(NO_ASN_TIME) && \
22387+
defined(WOLFSSL_CERT_GEN) && defined(HAVE_ECC) && \
22388+
defined(WOLFSSL_CERT_EXT) && !defined(NO_CERTS) && \
22389+
defined(WOLFSSL_ALT_NAMES) && defined(WOLFSSL_CUSTOM_OID) && \
22390+
defined(HAVE_OID_ENCODING) && !defined(IGNORE_NAME_CONSTRAINTS)
22391+
byte sanBlocked[64];
22392+
byte sanAllowed[64];
22393+
byte ncExcludedBlocked[64];
22394+
byte ncPermittedAllowed[64];
22395+
word32 sanBlockedSz, sanAllowedSz;
22396+
word32 ncExcludedBlockedSz, ncPermittedAllowedSz;
22397+
22398+
sanBlockedSz =
22399+
build_otherName_san(sanBlocked, sizeof(sanBlocked), "blocked");
22400+
sanAllowedSz =
22401+
build_otherName_san(sanAllowed, sizeof(sanAllowed), "allowed");
22402+
ncExcludedBlockedSz = build_otherName_nameConstraints(
22403+
ncExcludedBlocked, sizeof(ncExcludedBlocked), 1, "blocked");
22404+
ncPermittedAllowedSz = build_otherName_nameConstraints(
22405+
ncPermittedAllowed, sizeof(ncPermittedAllowed), 0, "allowed");
22406+
ExpectIntGT((int)sanBlockedSz, 0);
22407+
ExpectIntGT((int)sanAllowedSz, 0);
22408+
ExpectIntGT((int)ncExcludedBlockedSz, 0);
22409+
ExpectIntGT((int)ncPermittedAllowedSz, 0);
22410+
22411+
/* (1) Original bypass scenario: critical excluded otherName matches
22412+
* the leaf's otherName SAN. Must be rejected. */
22413+
ExpectIntEQ(verify_with_otherName_chain(
22414+
ncExcludedBlocked, ncExcludedBlockedSz, 1,
22415+
sanBlocked, sanBlockedSz),
22416+
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
22417+
22418+
/* (2) Positive control: same critical excluded subtree, but the leaf
22419+
* carries a DIFFERENT otherName value, so byte-comparison says no
22420+
* match and the chain MUST verify. This pins the rejection in (1)
22421+
* to the matching path rather than to a blanket 'reject any
22422+
* otherName under critical'. */
22423+
ExpectIntEQ(verify_with_otherName_chain(
22424+
ncExcludedBlocked, ncExcludedBlockedSz, 1,
22425+
sanAllowed, sanAllowedSz),
22426+
0);
22427+
22428+
/* (3) Non-critical excluded subtree, leaf SAN matches: exclusion is
22429+
* enforced regardless of criticality. */
22430+
ExpectIntEQ(verify_with_otherName_chain(
22431+
ncExcludedBlocked, ncExcludedBlockedSz, 0,
22432+
sanBlocked, sanBlockedSz),
22433+
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
22434+
22435+
/* (4) Critical permitted subtree, leaf SAN inside the permitted set:
22436+
* verification succeeds. */
22437+
ExpectIntEQ(verify_with_otherName_chain(
22438+
ncPermittedAllowed, ncPermittedAllowedSz, 1,
22439+
sanAllowed, sanAllowedSz),
22440+
0);
22441+
22442+
/* (5) Critical permitted subtree, leaf SAN outside the permitted set:
22443+
* verification rejects. */
22444+
ExpectIntEQ(verify_with_otherName_chain(
22445+
ncPermittedAllowed, ncPermittedAllowedSz, 1,
22446+
sanBlocked, sanBlockedSz),
22447+
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
22448+
#endif
22449+
return EXPECT_RESULT();
22450+
}
22451+
2219622452
static int test_MakeCertWithCaFalse(void)
2219722453
{
2219822454
EXPECT_DECLS;
@@ -37012,6 +37268,7 @@ TEST_CASE testCases[] = {
3701237268
TEST_DECL(test_PathLenSelfIssued),
3701337269
TEST_DECL(test_PathLenSelfIssuedAllowed),
3701437270
TEST_DECL(test_PathLenNoKeyUsage),
37271+
TEST_DECL(test_NameConstraints_OtherName),
3701537272
TEST_DECL(test_MakeCertWith0Ser),
3701637273
TEST_DECL(test_MakeCertWithCaFalse),
3701737274
#ifdef WOLFSSL_CERT_SIGN_CB

0 commit comments

Comments
 (0)