Skip to content

Commit 262737d

Browse files
embhornembhorn
committed
Fixes from review
1 parent 6fdd0de commit 262737d

3 files changed

Lines changed: 133 additions & 70 deletions

File tree

‎tests/api.c‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22220,6 +22220,26 @@ static word32 build_otherName_san(byte* out, word32 outSz, const char* val7)
2222022220
return (word32)(sizeof(prefix) + 7);
2222122221
}
2222222222

22223+
/* Build a NameConstraints extension value with a single excludedSubtree
22224+
* carrying a registeredID GeneralName for OID 1.2.3.4. registeredID is a
22225+
* GeneralName form wolfSSL does not enforce, so DecodeSubtree() must
22226+
* record it as 'unsupported' and ConfirmNameConstraints() must fail
22227+
* closed when the extension is critical (RFC 5280 4.2.1.10). */
22228+
static word32 build_registeredID_nameConstraints(byte* out, word32 outSz)
22229+
{
22230+
static const byte ridNc[] = {
22231+
0x30, 0x09, /* SEQUENCE, 9 */
22232+
0xA1, 0x07, /* [1] excluded, 7 */
22233+
0x30, 0x05, /* GeneralSubtree, 5 */
22234+
0x88, 0x03, /* [8] regId, 3 */
22235+
0x2A, 0x03, 0x04 /* OID 1.2.3.4 */
22236+
};
22237+
if (outSz < sizeof(ridNc))
22238+
return 0;
22239+
XMEMCPY(out, ridNc, sizeof(ridNc));
22240+
return (word32)sizeof(ridNc);
22241+
}
22242+
2222322243
/* Build a NameConstraints extension value carrying a single subtree of
2222422244
* the given list type ([0] permitted or [1] excluded) for an otherName
2222522245
* UPN whose UTF8 value is the given 7-byte string. */
@@ -22334,9 +22354,11 @@ static int verify_with_otherName_chain(const byte* nameConstraintsDer,
2233422354
goto done;
2233522355
if (wc_SetSubjectKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &leafKey) != 0)
2233622356
goto done;
22337-
if (sanDerSz > sizeof(cert.altNames)) goto done;
22338-
XMEMCPY(cert.altNames, sanDer, sanDerSz);
22339-
cert.altNamesSz = (int)sanDerSz;
22357+
if (sanDer != NULL && sanDerSz > 0) {
22358+
if (sanDerSz > sizeof(cert.altNames)) goto done;
22359+
XMEMCPY(cert.altNames, sanDer, sanDerSz);
22360+
cert.altNamesSz = (int)sanDerSz;
22361+
}
2234022362
if (wc_MakeCert(&cert, leafDer, FOURK_BUF, NULL, &leafKey, &rng) < 0)
2234122363
goto done;
2234222364
leafDerSz = wc_SignCert(cert.bodySz, cert.sigType, leafDer, FOURK_BUF,
@@ -22379,6 +22401,10 @@ static int verify_with_otherName_chain(const byte* nameConstraintsDer,
2237922401
* (excluded is enforced regardless of criticality)
2238022402
* 4. Critical permitted subtree, leaf SAN matches -> accept
2238122403
* 5. Critical permitted subtree, leaf SAN does NOT match -> reject
22404+
* 6. Critical nameConstraints carrying an unsupported form
22405+
* (registeredID), leaf has no relevant SAN -> reject
22406+
* (RFC 5280 4.2.1.10 fail-closed for unprocessed forms)
22407+
* 7. Same as (6) but non-critical -> accept
2238222408
*/
2238322409
static int test_NameConstraints_OtherName(void)
2238422410
{
@@ -22392,8 +22418,9 @@ static int test_NameConstraints_OtherName(void)
2239222418
byte sanAllowed[64];
2239322419
byte ncExcludedBlocked[64];
2239422420
byte ncPermittedAllowed[64];
22421+
byte ncRegisteredID[16];
2239522422
word32 sanBlockedSz, sanAllowedSz;
22396-
word32 ncExcludedBlockedSz, ncPermittedAllowedSz;
22423+
word32 ncExcludedBlockedSz, ncPermittedAllowedSz, ncRegisteredIDSz;
2239722424

2239822425
sanBlockedSz =
2239922426
build_otherName_san(sanBlocked, sizeof(sanBlocked), "blocked");
@@ -22403,10 +22430,13 @@ static int test_NameConstraints_OtherName(void)
2240322430
ncExcludedBlocked, sizeof(ncExcludedBlocked), 1, "blocked");
2240422431
ncPermittedAllowedSz = build_otherName_nameConstraints(
2240522432
ncPermittedAllowed, sizeof(ncPermittedAllowed), 0, "allowed");
22433+
ncRegisteredIDSz = build_registeredID_nameConstraints(
22434+
ncRegisteredID, sizeof(ncRegisteredID));
2240622435
ExpectIntGT((int)sanBlockedSz, 0);
2240722436
ExpectIntGT((int)sanAllowedSz, 0);
2240822437
ExpectIntGT((int)ncExcludedBlockedSz, 0);
2240922438
ExpectIntGT((int)ncPermittedAllowedSz, 0);
22439+
ExpectIntGT((int)ncRegisteredIDSz, 0);
2241022440

2241122441
/* (1) Original bypass scenario: critical excluded otherName matches
2241222442
* the leaf's otherName SAN. Must be rejected. */
@@ -22445,6 +22475,22 @@ static int test_NameConstraints_OtherName(void)
2244522475
ncPermittedAllowed, ncPermittedAllowedSz, 1,
2244622476
sanBlocked, sanBlockedSz),
2244722477
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
22478+
22479+
/* (6) Critical nameConstraints carrying a GeneralName form wolfSSL
22480+
* does not enforce (registeredID). RFC 5280 4.2.1.10 requires the
22481+
* verifier to either process the constraint or reject; we reject
22482+
* fail-closed. The leaf needs no SAN to exercise this path. */
22483+
ExpectIntEQ(verify_with_otherName_chain(
22484+
ncRegisteredID, ncRegisteredIDSz, 1, NULL, 0),
22485+
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
22486+
22487+
/* (7) Same as (6) but non-critical: RFC 5280 only mandates the
22488+
* fail-closed reject when the extension is critical, so a
22489+
* non-critical unsupported constraint form is silently ignored
22490+
* and verification succeeds. */
22491+
ExpectIntEQ(verify_with_otherName_chain(
22492+
ncRegisteredID, ncRegisteredIDSz, 0, NULL, 0),
22493+
0);
2244822494
#endif
2244922495
return EXPECT_RESULT();
2245022496
}

‎wolfcrypt/src/asn.c‎

Lines changed: 46 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12139,6 +12139,8 @@ void FreeDecodedCert(DecodedCert* cert)
1213912139
FreeAltNames(cert->altEmailNames, cert->heap);
1214012140
if (cert->altDirNames)
1214112141
FreeAltNames(cert->altDirNames, cert->heap);
12142+
if (cert->altOtherNamesRaw)
12143+
FreeAltNames(cert->altOtherNamesRaw, cert->heap);
1214212144
if (cert->permittedNames)
1214312145
FreeNameSubtrees(cert->permittedNames, cert->heap);
1214412146
if (cert->excludedNames)
@@ -17622,6 +17624,19 @@ int wolfssl_local_MatchIpSubnet(const byte* ip, int ipSz,
1762217624
return match;
1762317625
}
1762417626

17627+
/* RFC 5280 4.2.1.10: otherName matching is byte-exact comparison of the
17628+
* full OtherName encoding (OID || [0] EXPLICIT value). Both the leaf SAN
17629+
* (cert->altOtherNamesRaw) and the constraint subtree (Base_entry from
17630+
* DecodeSubtree) store the same form, so a memcmp suffices. */
17631+
static int MatchOtherNameConstraint(DNS_entry* name, Base_entry* current)
17632+
{
17633+
if (name == NULL || current == NULL)
17634+
return 0;
17635+
if (name->len != current->nameSz)
17636+
return 0;
17637+
return XMEMCMP(name->name, current->name, (size_t)current->nameSz) == 0;
17638+
}
17639+
1762517640
/* Search through the list to find if the name is permitted.
1762617641
* name The DNS name to search for
1762717642
* dnsList The list to search through
@@ -17656,21 +17671,7 @@ static int PermittedListOk(DNS_entry* name, Base_entry* dnsList, byte nameType)
1765617671
}
1765717672
}
1765817673
else if (nameType == ASN_OTHER_TYPE) {
17659-
/* RFC 5280 4.2.1.10: otherName matching is byte-exact
17660-
* comparison of the full OtherName encoding. The FPKI/SEP
17661-
* path also stores entries that contain only the parsed
17662-
* UPN/FASCN value and have oidSum != 0; those are not
17663-
* byte-comparable with the OID || [0] EXPLICIT value form
17664-
* stored for the constraint, so we explicitly skip them.
17665-
* Without that guard, a coincidental length match could
17666-
* mis-validate. */
17667-
if (
17668-
#ifdef WOLFSSL_FPKI
17669-
name->oidSum == 0 &&
17670-
#endif
17671-
name->len == current->nameSz &&
17672-
XMEMCMP(name->name, current->name,
17673-
(size_t)current->nameSz) == 0) {
17674+
if (MatchOtherNameConstraint(name, current)) {
1767417675
match = 1;
1767517676
break;
1767617677
}
@@ -17723,14 +17724,7 @@ static int IsInExcludedList(DNS_entry* name, Base_entry* dnsList, byte nameType)
1772317724
}
1772417725
}
1772517726
else if (nameType == ASN_OTHER_TYPE) {
17726-
/* See note in PermittedListOk about byte-exact matching. */
17727-
if (
17728-
#ifdef WOLFSSL_FPKI
17729-
name->oidSum == 0 &&
17730-
#endif
17731-
name->len == current->nameSz &&
17732-
XMEMCMP(name->name, current->name,
17733-
(size_t)current->nameSz) == 0) {
17727+
if (MatchOtherNameConstraint(name, current)) {
1773417728
ret = 1;
1773517729
break;
1773617730
}
@@ -17825,15 +17819,12 @@ static int ConfirmNameConstraints(Signer* signer, DecodedCert* cert)
1782517819
name = cert->altNames;
1782617820
break;
1782717821
case ASN_OTHER_TYPE:
17828-
/* otherName SAN entries are stored on cert->altNames with
17829-
* type ASN_OTHER_TYPE. Match by byte-exact comparison of
17830-
* the OtherName encoding (OID || [0] EXPLICIT value). For
17831-
* FPKI/SEP builds, altNames may also contain entries that
17832-
* hold only the parsed UPN/FASCN value (oidSum != 0); the
17833-
* explicit oidSum guard in IsInExcludedList /
17834-
* PermittedListOk skips those so a coincidental length
17835-
* match cannot mis-validate. */
17836-
name = cert->altNames;
17822+
/* otherName SAN entries are stored on cert->altOtherNamesRaw
17823+
* (kept separate from altNames so the public altNames view
17824+
* is unaffected). Each entry holds the raw OtherName
17825+
* encoding (OID || [0] EXPLICIT value) and is byte-matched
17826+
* against the issuing CA's subtree. */
17827+
name = cert->altOtherNamesRaw;
1783717828
break;
1783817829
default:
1783917830
return 0;
@@ -18192,37 +18183,37 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
1819218183
}
1819318184
#endif /* WOLFSSL_RID_ALT_NAME */
1819418185
#endif /* IGNORE_NAME_CONSTRAINTS */
18195-
#if defined(WOLFSSL_SEP) || defined(WOLFSSL_FPKI)
18196-
/* GeneralName choice: otherName */
18186+
#ifndef IGNORE_NAME_CONSTRAINTS
18187+
/* GeneralName choice: otherName.
18188+
* Store the raw OtherName encoding (OID || [0] EXPLICIT value) on a
18189+
* dedicated internal list so ConfirmNameConstraints() can byte-match
18190+
* it against the issuing CA's nameConstraints subtree (RFC 5280
18191+
* 4.2.1.10). The raw form is kept separate from cert->altNames so
18192+
* the public altNames view (used by OpenSSL-compat APIs) reflects
18193+
* exactly what the SAN extension carries. */
1819718194
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | ASN_OTHER_TYPE)) {
18198-
/* TODO: test data for code path */
18199-
#ifndef IGNORE_NAME_CONSTRAINTS
18200-
/* Store the raw OtherName encoding so ConfirmNameConstraints() can
18201-
* byte-match it against the issuing CA's subtree (RFC 5280
18202-
* 4.2.1.10). DecodeOtherName() may also add an entry that holds
18203-
* only the parsed UPN/FASCN value with oidSum != 0; the explicit
18204-
* oidSum guard in IsInExcludedList()/PermittedListOk() ensures
18205-
* those parsed entries are skipped during byte-comparison. */
1820618195
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
18207-
ASN_OTHER_TYPE, &cert->altNames);
18196+
ASN_OTHER_TYPE, &cert->altOtherNamesRaw);
1820818197
if (ret != 0) {
1820918198
return ret;
1821018199
}
18211-
#endif
18200+
#if defined(WOLFSSL_SEP) || defined(WOLFSSL_FPKI)
18201+
/* FPKI/SEP also OID-decode the otherName into a separate altNames
18202+
* entry that holds the parsed UPN/FASCN value (with oidSum != 0).
18203+
* That parsed entry is consumed by wc_GetUUIDFromCert /
18204+
* wc_GetFASCNFromCert; ConfirmNameConstraints() does not look at
18205+
* it - it iterates altOtherNamesRaw instead. */
1821218206
ret = DecodeOtherName(cert, input, &idx, len);
18207+
#else
18208+
idx += (word32)len;
18209+
#endif
1821318210
}
18214-
#elif !defined(IGNORE_NAME_CONSTRAINTS)
18215-
/* GeneralName choice: otherName.
18216-
* No OID-specific decoding in this build, but we store the raw
18217-
* OtherName encoding (OID || [0] EXPLICIT value) on altNames so
18218-
* ConfirmNameConstraints() can byte-match it against the issuing CA's
18219-
* nameConstraints subtree (RFC 5280 4.2.1.10). */
18211+
#elif defined(WOLFSSL_SEP) || defined(WOLFSSL_FPKI)
18212+
/* No name constraints support in the build, but FPKI/SEP still need
18213+
* the parsed otherName entry for wc_GetUUIDFromCert /
18214+
* wc_GetFASCNFromCert. */
1822018215
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | ASN_OTHER_TYPE)) {
18221-
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
18222-
ASN_OTHER_TYPE, &cert->altNames);
18223-
if (ret == 0) {
18224-
idx += (word32)len;
18225-
}
18216+
ret = DecodeOtherName(cert, input, &idx, len);
1822618217
}
1822718218
#endif
1822818219
/* GeneralName choice: dNSName, x400Address, ediPartyName */

‎wolfssl/wolfcrypt/asn.h‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,6 +1760,13 @@ struct DecodedCert {
17601760
#ifndef IGNORE_NAME_CONSTRAINTS
17611761
DNS_entry* altEmailNames; /* alt names list of RFC822 entries */
17621762
DNS_entry* altDirNames; /* alt names list of DIR entries */
1763+
/* Raw OtherName GeneralName encodings (OID || [0] EXPLICIT value)
1764+
* for any otherName SAN seen on this certificate. Used internally by
1765+
* ConfirmNameConstraints() for byte-exact matching against the
1766+
* issuing CA's nameConstraints subtrees (RFC 5280 4.2.1.10). Kept
1767+
* separate from altNames so OpenSSL-compat APIs that iterate
1768+
* altNames see exactly the entries the SAN extension carries. */
1769+
DNS_entry* altOtherNamesRaw;
17631770
Base_entry* permittedNames; /* Permitted name bases */
17641771
Base_entry* excludedNames; /* Excluded name bases */
17651772
#endif /* IGNORE_NAME_CONSTRAINTS */
@@ -2062,11 +2069,22 @@ struct DecodedCert {
20622069
WC_BITFIELD extSubjAltNameCrit:1;
20632070
WC_BITFIELD extAuthKeyIdCrit:1;
20642071
#ifndef IGNORE_NAME_CONSTRAINTS
2072+
/*!
2073+
* \brief Set when the certificate's nameConstraints extension was
2074+
* present and marked critical.
2075+
*/
20652076
WC_BITFIELD extNameConstraintCrit:1;
2066-
/* Set when DecodeSubtree encountered a constraint form (e.g.
2067-
* registeredID, x400Address, ediPartyName) we cannot enforce. Used
2068-
* together with extNameConstraintCrit to implement the RFC 5280
2069-
* 4.2.1.10 fail-closed requirement. */
2077+
/*!
2078+
* \brief Set when decoding the nameConstraints extension encountered
2079+
* at least one permittedSubtrees or excludedSubtrees entry whose
2080+
* GeneralName form (e.g. registeredID, x400Address,
2081+
* ediPartyName) wolfSSL does not enforce.
2082+
*
2083+
* During verification, ConfirmNameConstraints() implements the RFC
2084+
* 5280 4.2.1.10 fail-closed requirement: when both this flag and
2085+
* extNameConstraintCrit are set, the chain is rejected rather than
2086+
* the unsupported constraint form being silently ignored.
2087+
*/
20702088
WC_BITFIELD extNameConstraintHasUnsupported:1;
20712089
#endif
20722090
WC_BITFIELD extSubjKeyIdCrit:1;
@@ -2136,13 +2154,21 @@ struct Signer {
21362154
word16 maxPathLen;
21372155
WC_BITFIELD selfSigned:1;
21382156
#ifndef IGNORE_NAME_CONSTRAINTS
2139-
/* Mirror of DecodedCert::extNameConstraintCrit and
2140-
* extNameConstraintHasUnsupported so ConfirmNameConstraints can
2141-
* implement the RFC 5280 4.2.1.10 fail-closed requirement when a
2142-
* critical nameConstraints extension imposes a constraint form we
2143-
* cannot fully enforce. Co-located with selfSigned to share its
2144-
* bitfield storage word and avoid growing sizeof(Signer), which is
2145-
* load-bearing for PERSIST_CERT_CACHE. */
2157+
/*!
2158+
* \brief Mirrors DecodedCert::extNameConstraintCrit and
2159+
* DecodedCert::extNameConstraintHasUnsupported so the
2160+
* nameConstraints state survives onto the CA Signer and is
2161+
* available during chain verification.
2162+
*
2163+
* ConfirmNameConstraints() uses these flags to implement the RFC 5280
2164+
* 4.2.1.10 fail-closed requirement: when extNameConstraintCrit is set
2165+
* and extNameConstraintHasUnsupported is also set, verification fails
2166+
* rather than the unsupported constraint form being silently ignored.
2167+
*
2168+
* Co-located with selfSigned to share its bitfield storage word and
2169+
* avoid growing sizeof(Signer), which is load-bearing for
2170+
* PERSIST_CERT_CACHE.
2171+
*/
21462172
WC_BITFIELD extNameConstraintCrit:1;
21472173
WC_BITFIELD extNameConstraintHasUnsupported:1;
21482174
#endif

0 commit comments

Comments
 (0)