Fixes from review

This commit is contained in:
Eric Blankenhorn
2026-04-28 11:54:46 -05:00
parent 6fdd0de0ca
commit 262737d63f
3 changed files with 133 additions and 70 deletions
+50 -4
View File
@@ -22220,6 +22220,26 @@ static word32 build_otherName_san(byte* out, word32 outSz, const char* val7)
return (word32)(sizeof(prefix) + 7);
}
/* Build a NameConstraints extension value with a single excludedSubtree
* carrying a registeredID GeneralName for OID 1.2.3.4. registeredID is a
* GeneralName form wolfSSL does not enforce, so DecodeSubtree() must
* record it as 'unsupported' and ConfirmNameConstraints() must fail
* closed when the extension is critical (RFC 5280 4.2.1.10). */
static word32 build_registeredID_nameConstraints(byte* out, word32 outSz)
{
static const byte ridNc[] = {
0x30, 0x09, /* SEQUENCE, 9 */
0xA1, 0x07, /* [1] excluded, 7 */
0x30, 0x05, /* GeneralSubtree, 5 */
0x88, 0x03, /* [8] regId, 3 */
0x2A, 0x03, 0x04 /* OID 1.2.3.4 */
};
if (outSz < sizeof(ridNc))
return 0;
XMEMCPY(out, ridNc, sizeof(ridNc));
return (word32)sizeof(ridNc);
}
/* Build a NameConstraints extension value carrying a single subtree of
* the given list type ([0] permitted or [1] excluded) for an otherName
* UPN whose UTF8 value is the given 7-byte string. */
@@ -22334,9 +22354,11 @@ static int verify_with_otherName_chain(const byte* nameConstraintsDer,
goto done;
if (wc_SetSubjectKeyIdFromPublicKey_ex(&cert, ECC_TYPE, &leafKey) != 0)
goto done;
if (sanDerSz > sizeof(cert.altNames)) goto done;
XMEMCPY(cert.altNames, sanDer, sanDerSz);
cert.altNamesSz = (int)sanDerSz;
if (sanDer != NULL && sanDerSz > 0) {
if (sanDerSz > sizeof(cert.altNames)) goto done;
XMEMCPY(cert.altNames, sanDer, sanDerSz);
cert.altNamesSz = (int)sanDerSz;
}
if (wc_MakeCert(&cert, leafDer, FOURK_BUF, NULL, &leafKey, &rng) < 0)
goto done;
leafDerSz = wc_SignCert(cert.bodySz, cert.sigType, leafDer, FOURK_BUF,
@@ -22379,6 +22401,10 @@ done:
* (excluded is enforced regardless of criticality)
* 4. Critical permitted subtree, leaf SAN matches -> accept
* 5. Critical permitted subtree, leaf SAN does NOT match -> reject
* 6. Critical nameConstraints carrying an unsupported form
* (registeredID), leaf has no relevant SAN -> reject
* (RFC 5280 4.2.1.10 fail-closed for unprocessed forms)
* 7. Same as (6) but non-critical -> accept
*/
static int test_NameConstraints_OtherName(void)
{
@@ -22392,8 +22418,9 @@ static int test_NameConstraints_OtherName(void)
byte sanAllowed[64];
byte ncExcludedBlocked[64];
byte ncPermittedAllowed[64];
byte ncRegisteredID[16];
word32 sanBlockedSz, sanAllowedSz;
word32 ncExcludedBlockedSz, ncPermittedAllowedSz;
word32 ncExcludedBlockedSz, ncPermittedAllowedSz, ncRegisteredIDSz;
sanBlockedSz =
build_otherName_san(sanBlocked, sizeof(sanBlocked), "blocked");
@@ -22403,10 +22430,13 @@ static int test_NameConstraints_OtherName(void)
ncExcludedBlocked, sizeof(ncExcludedBlocked), 1, "blocked");
ncPermittedAllowedSz = build_otherName_nameConstraints(
ncPermittedAllowed, sizeof(ncPermittedAllowed), 0, "allowed");
ncRegisteredIDSz = build_registeredID_nameConstraints(
ncRegisteredID, sizeof(ncRegisteredID));
ExpectIntGT((int)sanBlockedSz, 0);
ExpectIntGT((int)sanAllowedSz, 0);
ExpectIntGT((int)ncExcludedBlockedSz, 0);
ExpectIntGT((int)ncPermittedAllowedSz, 0);
ExpectIntGT((int)ncRegisteredIDSz, 0);
/* (1) Original bypass scenario: critical excluded otherName matches
* the leaf's otherName SAN. Must be rejected. */
@@ -22445,6 +22475,22 @@ static int test_NameConstraints_OtherName(void)
ncPermittedAllowed, ncPermittedAllowedSz, 1,
sanBlocked, sanBlockedSz),
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
/* (6) Critical nameConstraints carrying a GeneralName form wolfSSL
* does not enforce (registeredID). RFC 5280 4.2.1.10 requires the
* verifier to either process the constraint or reject; we reject
* fail-closed. The leaf needs no SAN to exercise this path. */
ExpectIntEQ(verify_with_otherName_chain(
ncRegisteredID, ncRegisteredIDSz, 1, NULL, 0),
WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
/* (7) Same as (6) but non-critical: RFC 5280 only mandates the
* fail-closed reject when the extension is critical, so a
* non-critical unsupported constraint form is silently ignored
* and verification succeeds. */
ExpectIntEQ(verify_with_otherName_chain(
ncRegisteredID, ncRegisteredIDSz, 0, NULL, 0),
0);
#endif
return EXPECT_RESULT();
}
+46 -55
View File
@@ -12139,6 +12139,8 @@ void FreeDecodedCert(DecodedCert* cert)
FreeAltNames(cert->altEmailNames, cert->heap);
if (cert->altDirNames)
FreeAltNames(cert->altDirNames, cert->heap);
if (cert->altOtherNamesRaw)
FreeAltNames(cert->altOtherNamesRaw, cert->heap);
if (cert->permittedNames)
FreeNameSubtrees(cert->permittedNames, cert->heap);
if (cert->excludedNames)
@@ -17622,6 +17624,19 @@ int wolfssl_local_MatchIpSubnet(const byte* ip, int ipSz,
return match;
}
/* RFC 5280 4.2.1.10: otherName matching is byte-exact comparison of the
* full OtherName encoding (OID || [0] EXPLICIT value). Both the leaf SAN
* (cert->altOtherNamesRaw) and the constraint subtree (Base_entry from
* DecodeSubtree) store the same form, so a memcmp suffices. */
static int MatchOtherNameConstraint(DNS_entry* name, Base_entry* current)
{
if (name == NULL || current == NULL)
return 0;
if (name->len != current->nameSz)
return 0;
return XMEMCMP(name->name, current->name, (size_t)current->nameSz) == 0;
}
/* Search through the list to find if the name is permitted.
* name The DNS name to search for
* dnsList The list to search through
@@ -17656,21 +17671,7 @@ static int PermittedListOk(DNS_entry* name, Base_entry* dnsList, byte nameType)
}
}
else if (nameType == ASN_OTHER_TYPE) {
/* RFC 5280 4.2.1.10: otherName matching is byte-exact
* comparison of the full OtherName encoding. The FPKI/SEP
* path also stores entries that contain only the parsed
* UPN/FASCN value and have oidSum != 0; those are not
* byte-comparable with the OID || [0] EXPLICIT value form
* stored for the constraint, so we explicitly skip them.
* Without that guard, a coincidental length match could
* mis-validate. */
if (
#ifdef WOLFSSL_FPKI
name->oidSum == 0 &&
#endif
name->len == current->nameSz &&
XMEMCMP(name->name, current->name,
(size_t)current->nameSz) == 0) {
if (MatchOtherNameConstraint(name, current)) {
match = 1;
break;
}
@@ -17723,14 +17724,7 @@ static int IsInExcludedList(DNS_entry* name, Base_entry* dnsList, byte nameType)
}
}
else if (nameType == ASN_OTHER_TYPE) {
/* See note in PermittedListOk about byte-exact matching. */
if (
#ifdef WOLFSSL_FPKI
name->oidSum == 0 &&
#endif
name->len == current->nameSz &&
XMEMCMP(name->name, current->name,
(size_t)current->nameSz) == 0) {
if (MatchOtherNameConstraint(name, current)) {
ret = 1;
break;
}
@@ -17825,15 +17819,12 @@ static int ConfirmNameConstraints(Signer* signer, DecodedCert* cert)
name = cert->altNames;
break;
case ASN_OTHER_TYPE:
/* otherName SAN entries are stored on cert->altNames with
* type ASN_OTHER_TYPE. Match by byte-exact comparison of
* the OtherName encoding (OID || [0] EXPLICIT value). For
* FPKI/SEP builds, altNames may also contain entries that
* hold only the parsed UPN/FASCN value (oidSum != 0); the
* explicit oidSum guard in IsInExcludedList /
* PermittedListOk skips those so a coincidental length
* match cannot mis-validate. */
name = cert->altNames;
/* otherName SAN entries are stored on cert->altOtherNamesRaw
* (kept separate from altNames so the public altNames view
* is unaffected). Each entry holds the raw OtherName
* encoding (OID || [0] EXPLICIT value) and is byte-matched
* against the issuing CA's subtree. */
name = cert->altOtherNamesRaw;
break;
default:
return 0;
@@ -18192,37 +18183,37 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
}
#endif /* WOLFSSL_RID_ALT_NAME */
#endif /* IGNORE_NAME_CONSTRAINTS */
#if defined(WOLFSSL_SEP) || defined(WOLFSSL_FPKI)
/* GeneralName choice: otherName */
#ifndef IGNORE_NAME_CONSTRAINTS
/* GeneralName choice: otherName.
* Store the raw OtherName encoding (OID || [0] EXPLICIT value) on a
* dedicated internal list so ConfirmNameConstraints() can byte-match
* it against the issuing CA's nameConstraints subtree (RFC 5280
* 4.2.1.10). The raw form is kept separate from cert->altNames so
* the public altNames view (used by OpenSSL-compat APIs) reflects
* exactly what the SAN extension carries. */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | ASN_OTHER_TYPE)) {
/* TODO: test data for code path */
#ifndef IGNORE_NAME_CONSTRAINTS
/* Store the raw OtherName encoding so ConfirmNameConstraints() can
* byte-match it against the issuing CA's subtree (RFC 5280
* 4.2.1.10). DecodeOtherName() may also add an entry that holds
* only the parsed UPN/FASCN value with oidSum != 0; the explicit
* oidSum guard in IsInExcludedList()/PermittedListOk() ensures
* those parsed entries are skipped during byte-comparison. */
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_OTHER_TYPE, &cert->altNames);
ASN_OTHER_TYPE, &cert->altOtherNamesRaw);
if (ret != 0) {
return ret;
}
#endif
#if defined(WOLFSSL_SEP) || defined(WOLFSSL_FPKI)
/* FPKI/SEP also OID-decode the otherName into a separate altNames
* entry that holds the parsed UPN/FASCN value (with oidSum != 0).
* That parsed entry is consumed by wc_GetUUIDFromCert /
* wc_GetFASCNFromCert; ConfirmNameConstraints() does not look at
* it - it iterates altOtherNamesRaw instead. */
ret = DecodeOtherName(cert, input, &idx, len);
#else
idx += (word32)len;
#endif
}
#elif !defined(IGNORE_NAME_CONSTRAINTS)
/* GeneralName choice: otherName.
* No OID-specific decoding in this build, but we store the raw
* OtherName encoding (OID || [0] EXPLICIT value) on altNames so
* ConfirmNameConstraints() can byte-match it against the issuing CA's
* nameConstraints subtree (RFC 5280 4.2.1.10). */
#elif defined(WOLFSSL_SEP) || defined(WOLFSSL_FPKI)
/* No name constraints support in the build, but FPKI/SEP still need
* the parsed otherName entry for wc_GetUUIDFromCert /
* wc_GetFASCNFromCert. */
else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | ASN_OTHER_TYPE)) {
ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
ASN_OTHER_TYPE, &cert->altNames);
if (ret == 0) {
idx += (word32)len;
}
ret = DecodeOtherName(cert, input, &idx, len);
}
#endif
/* GeneralName choice: dNSName, x400Address, ediPartyName */
+37 -11
View File
@@ -1760,6 +1760,13 @@ struct DecodedCert {
#ifndef IGNORE_NAME_CONSTRAINTS
DNS_entry* altEmailNames; /* alt names list of RFC822 entries */
DNS_entry* altDirNames; /* alt names list of DIR entries */
/* Raw OtherName GeneralName encodings (OID || [0] EXPLICIT value)
* for any otherName SAN seen on this certificate. Used internally by
* ConfirmNameConstraints() for byte-exact matching against the
* issuing CA's nameConstraints subtrees (RFC 5280 4.2.1.10). Kept
* separate from altNames so OpenSSL-compat APIs that iterate
* altNames see exactly the entries the SAN extension carries. */
DNS_entry* altOtherNamesRaw;
Base_entry* permittedNames; /* Permitted name bases */
Base_entry* excludedNames; /* Excluded name bases */
#endif /* IGNORE_NAME_CONSTRAINTS */
@@ -2062,11 +2069,22 @@ struct DecodedCert {
WC_BITFIELD extSubjAltNameCrit:1;
WC_BITFIELD extAuthKeyIdCrit:1;
#ifndef IGNORE_NAME_CONSTRAINTS
/*!
* \brief Set when the certificate's nameConstraints extension was
* present and marked critical.
*/
WC_BITFIELD extNameConstraintCrit:1;
/* Set when DecodeSubtree encountered a constraint form (e.g.
* registeredID, x400Address, ediPartyName) we cannot enforce. Used
* together with extNameConstraintCrit to implement the RFC 5280
* 4.2.1.10 fail-closed requirement. */
/*!
* \brief Set when decoding the nameConstraints extension encountered
* at least one permittedSubtrees or excludedSubtrees entry whose
* GeneralName form (e.g. registeredID, x400Address,
* ediPartyName) wolfSSL does not enforce.
*
* During verification, ConfirmNameConstraints() implements the RFC
* 5280 4.2.1.10 fail-closed requirement: when both this flag and
* extNameConstraintCrit are set, the chain is rejected rather than
* the unsupported constraint form being silently ignored.
*/
WC_BITFIELD extNameConstraintHasUnsupported:1;
#endif
WC_BITFIELD extSubjKeyIdCrit:1;
@@ -2136,13 +2154,21 @@ struct Signer {
word16 maxPathLen;
WC_BITFIELD selfSigned:1;
#ifndef IGNORE_NAME_CONSTRAINTS
/* Mirror of DecodedCert::extNameConstraintCrit and
* extNameConstraintHasUnsupported so ConfirmNameConstraints can
* implement the RFC 5280 4.2.1.10 fail-closed requirement when a
* critical nameConstraints extension imposes a constraint form we
* cannot fully enforce. Co-located with selfSigned to share its
* bitfield storage word and avoid growing sizeof(Signer), which is
* load-bearing for PERSIST_CERT_CACHE. */
/*!
* \brief Mirrors DecodedCert::extNameConstraintCrit and
* DecodedCert::extNameConstraintHasUnsupported so the
* nameConstraints state survives onto the CA Signer and is
* available during chain verification.
*
* ConfirmNameConstraints() uses these flags to implement the RFC 5280
* 4.2.1.10 fail-closed requirement: when extNameConstraintCrit is set
* and extNameConstraintHasUnsupported is also set, verification fails
* rather than the unsupported constraint form being silently ignored.
*
* Co-located with selfSigned to share its bitfield storage word and
* avoid growing sizeof(Signer), which is load-bearing for
* PERSIST_CERT_CACHE.
*/
WC_BITFIELD extNameConstraintCrit:1;
WC_BITFIELD extNameConstraintHasUnsupported:1;
#endif