diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index 82d91f351bb..3499e7e2ded 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -2610,6 +2610,103 @@ int test_ParseCert_validity_length_overrun(void) return EXPECT_RESULT(); } +/* wolfSSL stores at most MAX_CERTPOL_NB certificate policies. Before the fix + * the decoder stopped at that limit and returned success, so a critical + * certificatePolicies extension carrying MAX_CERTPOL_NB+1 unique policies was + * accepted with its extra policies left uninterpreted (issue 10628). RFC 5280 + * 4.2 requires rejecting a critical extension that cannot be fully processed: + * it is now reported as an unsupported critical extension (ASN_CRIT_EXT_E, + * unless WOLFSSL_NO_ASN_STRICT). A non-critical over-limit extension keeps the + * first MAX_CERTPOL_NB policies and is still accepted, as is an at-limit one. */ +#if defined(WOLFSSL_CERT_EXT) && !defined(NO_CERTS) && !defined(NO_ASN) +/* Build a certificatePolicies extnValue with n PolicyInformation entries, + * each SEQUENCE { policyIdentifier OID 1.2.3.4.(k+1) }. The outer SEQUENCE + * length is written in short form, or long form with a single length octet + * once the content reaches 0x80, so the result is valid DER for content up to + * 255 bytes (n <= 31) - ample for the handful of policies used here. Returns + * the encoded length. out must hold 3 + 8*n bytes. n <= 127 keeps the OID arc + * single-byte. */ +static word32 test_certpol_build(byte* out, int n) +{ + word32 content = (word32)(8 * n); + word32 o; + int k; + + out[0] = 0x30; /* certificatePolicies SEQUENCE OF */ + if (content < 0x80U) { + out[1] = (byte)content; + o = 2; + } + else { + out[1] = 0x81U; /* long form, one length octet */ + out[2] = (byte)content; + o = 3; + } + for (k = 0; k < n; k++) { + out[o++] = 0x30; out[o++] = 0x06; /* PolicyInformation SEQ */ + out[o++] = 0x06; out[o++] = 0x04; /* policyIdentifier OID */ + out[o++] = 0x2A; out[o++] = 0x03; /* 1.2.3 */ + out[o++] = 0x04; out[o++] = (byte)(k + 1); /* .4.(k+1) */ + } + return o; +} +#endif + +int test_DecodeCertPolicy_tooMany(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_CERT_EXT) && !defined(NO_CERTS) && !defined(NO_ASN) + byte ext[3 + 8 * (MAX_CERTPOL_NB + 1)]; + word32 len; + DecodedCert cert; + int isUnknown; + + /* Exactly MAX_CERTPOL_NB policies: fully processed, accepted. */ + len = test_certpol_build(ext, MAX_CERTPOL_NB); + isUnknown = 0; + wc_InitDecodedCert(&cert, ext, len, NULL); + ExpectIntEQ(DecodeExtensionType(ext, len, CERT_POLICY_OID, 1, &cert, + &isUnknown), 0); + ExpectIntEQ(cert.extCertPoliciesNb, MAX_CERTPOL_NB); + wc_FreeDecodedCert(&cert); + +#ifndef WOLFSSL_NO_ASN_STRICT + /* One more than can be stored, critical: cannot be fully processed, so it + * is rejected as an unsupported critical extension rather than accepted + * with the extra policy left uninterpreted. */ + len = test_certpol_build(ext, MAX_CERTPOL_NB + 1); + isUnknown = 0; + wc_InitDecodedCert(&cert, ext, len, NULL); + ExpectIntEQ(DecodeExtensionType(ext, len, CERT_POLICY_OID, 1, &cert, + &isUnknown), WC_NO_ERR_TRACE(ASN_CRIT_EXT_E)); + wc_FreeDecodedCert(&cert); +#endif + + /* Non-critical: RFC 5280 4.2 lets a relying party ignore a non-critical + * extension it cannot fully process, so an over-limit non-critical + * certificatePolicies is still accepted (this is what FPKI certs, which + * carry many non-critical policies, rely on). */ + len = test_certpol_build(ext, MAX_CERTPOL_NB + 1); + isUnknown = 0; + wc_InitDecodedCert(&cert, ext, len, NULL); + ExpectIntEQ(DecodeExtensionType(ext, len, CERT_POLICY_OID, 0, &cert, + &isUnknown), 0); + /* The policies that fit are still stored. */ + ExpectIntEQ(cert.extCertPoliciesNb, MAX_CERTPOL_NB); + wc_FreeDecodedCert(&cert); + + /* A single policy is still accepted. */ + len = test_certpol_build(ext, 1); + isUnknown = 0; + wc_InitDecodedCert(&cert, ext, len, NULL); + ExpectIntEQ(DecodeExtensionType(ext, len, CERT_POLICY_OID, 1, &cert, + &isUnknown), 0); + ExpectIntEQ(cert.extCertPoliciesNb, 1); + wc_FreeDecodedCert(&cert); +#endif + return EXPECT_RESULT(); +} + int test_ParseCert_SM3wSM2_short_pubkey(void) { EXPECT_DECLS; diff --git a/tests/api/test_asn.h b/tests/api/test_asn.h index 89a03a672f5..45fa775ea26 100644 --- a/tests/api/test_asn.h +++ b/tests/api/test_asn.h @@ -41,6 +41,7 @@ int test_DecodeCertExtensions_empty_certpol(void); int test_DecodeCertExtensions_certpol_trailing_junk(void); int test_DecodeCertExtensions_empty_certpol_trailing(void); int test_ParseCert_validity_length_overrun(void); +int test_DecodeCertPolicy_tooMany(void); int test_ParseCert_SM3wSM2_short_pubkey(void); int test_ParseCert_dnBufferBoundary(void); int test_ParseCert_nameComponentIds(void); @@ -77,6 +78,7 @@ int test_wc_AsnFeatureCoverage(void); TEST_DECL_GROUP("asn", test_DecodeCertExtensions_certpol_trailing_junk), \ TEST_DECL_GROUP("asn", test_DecodeCertExtensions_empty_certpol_trailing), \ TEST_DECL_GROUP("asn", test_ParseCert_validity_length_overrun), \ + TEST_DECL_GROUP("asn", test_DecodeCertPolicy_tooMany), \ TEST_DECL_GROUP("asn", test_ParseCert_SM3wSM2_short_pubkey), \ TEST_DECL_GROUP("asn", test_ParseCert_dnBufferBoundary), \ TEST_DECL_GROUP("asn", test_ParseCert_nameComponentIds), \ diff --git a/tests/unit-mcdc/test_asn_ext_whitebox.c b/tests/unit-mcdc/test_asn_ext_whitebox.c index b770ecd9051..4c65e0ccc7a 100644 --- a/tests/unit-mcdc/test_asn_ext_whitebox.c +++ b/tests/unit-mcdc/test_asn_ext_whitebox.c @@ -1484,6 +1484,9 @@ static void wb_decode_policy_oid(void) * total_length==0 empty-SEQUENCE check, reached before that loop * :21369 ret==0 && cert->deviceType==NULL (WOLFSSL_SEP) * :21401 duplicate-OID scan loop (WOLFSSL_CERT_EXT, !WOLFSSL_DUP_CERTPOL) + * post-loop (ret==0) && critical && (idx < policyEnd) (WOLFSSL_CERT_EXT, + * !WOLFSSL_NO_ASN_STRICT): reject a critical certificatePolicies + * whose policies could not all be stored (ASN_CRIT_EXT_E). * MAX_CERTPOL_NB is 2, so three policies exercise the count limit. * ------------------------------------------------------------------------- */ #if defined(WOLFSSL_SEP) || defined(WOLFSSL_CERT_EXT) @@ -1533,13 +1536,14 @@ static void wb_decode_cert_policy(void) WB_NOTE("DecodeCertPolicy(): empty SEQUENCE rejected before the loop (total_length==0)"); XMEMSET(&cert, 0, sizeof(cert)); - ret = DecodeCertPolicy(noPolicies, sizeof(noPolicies), &cert); + /* 4-arg signature (critical) + upstream's empty-SEQUENCE rejection. */ + ret = DecodeCertPolicy(noPolicies, sizeof(noPolicies), &cert, 0); WB_CHECK(ret == WC_NO_ERR_TRACE(ASN_PARSE_E), "empty SEQUENCE rejected: RFC 5280 4.2.1.4 requires SIZE (1..MAX)"); WB_NOTE("DecodeCertPolicy(): one policy (loop true then false) [:21346,:21369]"); XMEMSET(&cert, 0, sizeof(cert)); - ret = DecodeCertPolicy(onePolicy, sizeof(onePolicy), &cert); + ret = DecodeCertPolicy(onePolicy, sizeof(onePolicy), &cert, 0); #if defined(WOLFSSL_CERT_EXT) WB_CHECK(ret == 0 && cert.extCertPoliciesNb == 1, "single policy accepted"); @@ -1556,7 +1560,7 @@ static void wb_decode_cert_policy(void) WB_NOTE("DecodeCertPolicy(): second policy, deviceType already set [:21369]"); XMEMSET(&cert, 0, sizeof(cert)); - ret = DecodeCertPolicy(twoPolicies, sizeof(twoPolicies), &cert); + ret = DecodeCertPolicy(twoPolicies, sizeof(twoPolicies), &cert, 0); WB_CHECK(ret == 0, "two policies accepted (:21369 2nd operand false on " "the second, :21401 2nd operand true)"); #ifdef WOLFSSL_SEP @@ -1567,14 +1571,14 @@ static void wb_decode_cert_policy(void) WB_NOTE("DecodeCertPolicy(): malformed PolicyInformation [:21369 1st operand]"); XMEMSET(&cert, 0, sizeof(cert)); - ret = DecodeCertPolicy(badPolicy, sizeof(badPolicy), &cert); + ret = DecodeCertPolicy(badPolicy, sizeof(badPolicy), &cert, 0); WB_CHECK(ret != 0 && cert.deviceType == NULL, "INTEGER in place of the policy OID (:21369 1st operand false)"); #if defined(WOLFSSL_CERT_EXT) WB_NOTE("DecodeCertPolicy(): MAX_CERTPOL_NB cap with 3 policies [:21346 3rd operand]"); XMEMSET(&cert, 0, sizeof(cert)); - ret = DecodeCertPolicy(threePolicies, sizeof(threePolicies), &cert); + ret = DecodeCertPolicy(threePolicies, sizeof(threePolicies), &cert, 0); WB_CHECK(ret == 0 && cert.extCertPoliciesNb == MAX_CERTPOL_NB, "loop stops at MAX_CERTPOL_NB even though bytes remain " "(3rd operand false while 1st/2nd stay true)"); @@ -1584,10 +1588,39 @@ static void wb_decode_cert_policy(void) } #endif +#ifndef WOLFSSL_NO_ASN_STRICT + /* Post-loop critical check: (ret==0) && critical && (idx < policyEnd). + * Independence pair for the `critical` and `idx < policyEnd` operands. */ + WB_NOTE("DecodeCertPolicy(): critical + policies remain -> ASN_CRIT_EXT_E"); + XMEMSET(&cert, 0, sizeof(cert)); + ret = DecodeCertPolicy(threePolicies, sizeof(threePolicies), &cert, 1); + WB_CHECK(ret == WC_NO_ERR_TRACE(ASN_CRIT_EXT_E) && + cert.extCertPoliciesNb == MAX_CERTPOL_NB, + "critical cert policies over MAX_CERTPOL_NB rejected as " + "unsupported-critical (critical true, idx < policyEnd true)"); +#ifdef WOLFSSL_SEP + if (cert.deviceType != NULL) { + XFREE(cert.deviceType, cert.heap, DYNAMIC_TYPE_X509_EXT); + } +#endif + + WB_NOTE("DecodeCertPolicy(): critical but all policies consumed -> accepted"); + XMEMSET(&cert, 0, sizeof(cert)); + ret = DecodeCertPolicy(twoPolicies, sizeof(twoPolicies), &cert, 1); + WB_CHECK(ret == 0, + "critical cert policies within MAX_CERTPOL_NB accepted " + "(critical true, idx < policyEnd false)"); +#ifdef WOLFSSL_SEP + if (cert.deviceType != NULL) { + XFREE(cert.deviceType, cert.heap, DYNAMIC_TYPE_X509_EXT); + } +#endif +#endif /* !WOLFSSL_NO_ASN_STRICT */ + #ifndef WOLFSSL_DUP_CERTPOL WB_NOTE("DecodeCertPolicy(): duplicate-OID rejection [:21401]"); XMEMSET(&cert, 0, sizeof(cert)); - ret = DecodeCertPolicy(dupPolicies, sizeof(dupPolicies), &cert); + ret = DecodeCertPolicy(dupPolicies, sizeof(dupPolicies), &cert, 0); WB_CHECK(ret == WC_NO_ERR_TRACE(CERTPOLICIES_E), "duplicate policy OID rejected (loop finds a match)"); #ifdef WOLFSSL_SEP diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 2df6050a862..5f51484750e 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -4656,7 +4656,8 @@ static int DecodeSubtree(const byte* input, word32 sz, Base_entry** head, static int DecodeNameConstraints(const byte* input, word32 sz, DecodedCert* cert); #endif #if defined(WOLFSSL_SEP) || defined(WOLFSSL_CERT_EXT) -static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert); +static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert, + int critical); #endif #ifdef WOLFSSL_SUBJ_DIR_ATTR static int DecodeSubjDirAttr(const byte* input, word32 sz, DecodedCert* cert); @@ -22266,7 +22267,8 @@ int DecodePolicyOID(char *out, word32 outSz, const byte *in, word32 inSz) /* Reference: https://tools.ietf.org/html/rfc5280#section-4.2.1.4 */ #ifdef WOLFSSL_ASN_TEMPLATE -static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) +static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert, + int critical) { word32 idx = 0; word32 seqEnd = 0; @@ -22278,6 +22280,10 @@ static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) WOLFSSL_ENTER("DecodeCertPolicy"); +#if !defined(WOLFSSL_CERT_EXT) || defined(WOLFSSL_NO_ASN_STRICT) + (void)critical; +#endif + /* Check if cert is null before dereferencing below */ if (cert == NULL) { ret = BAD_FUNC_ARG; @@ -22376,7 +22382,25 @@ static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) #endif /* WOLFSSL_CERT_EXT */ } - WOLFSSL_LEAVE("DecodeCertPolicy", 0); +#if defined(WOLFSSL_CERT_EXT) && !defined(WOLFSSL_NO_ASN_STRICT) + /* RFC 5280 4.2: reject a critical extension whose information cannot be + * fully processed. The loop above stops once the fixed-size store fills + * (MAX_CERTPOL_NB); if policies remain (idx has not reached the end of the + * SEQUENCE), a critical certificatePolicies cannot be honoured. Report it + * as an unsupported critical extension (ASN_CRIT_EXT_E) - the same code + * the "policy support not compiled" case returns for identical input - + * so DecodeCertExtensions defers it like every other such extension + * instead of aborting the parse immediately. A non-critical one may be + * left partially processed (relying parties may ignore it), so it is + * still accepted. */ + if ((ret == 0) && critical && (idx < seqEnd)) { + WOLFSSL_MSG("Cannot fully process critical certificatePolicies"); + WOLFSSL_ERROR_VERBOSE(ASN_CRIT_EXT_E); + ret = ASN_CRIT_EXT_E; + } +#endif /* WOLFSSL_CERT_EXT && !WOLFSSL_NO_ASN_STRICT */ + + WOLFSSL_LEAVE("DecodeCertPolicy", ret); return ret; } #endif /* WOLFSSL_ASN_TEMPLATE */ @@ -22887,7 +22911,11 @@ WOLFSSL_TEST_VIS int DecodeExtensionType(const byte* input, word32 length, #ifdef WOLFSSL_SEP cert->extCertPolicyCrit = critical ? 1 : 0; #endif - if (DecodeCertPolicy(input, length, cert) < 0) { + ret = DecodeCertPolicy(input, length, cert, critical); + /* Preserve ASN_CRIT_EXT_E so it is deferred like the other + * unsupported-critical-extension paths; map any other failure to + * ASN_PARSE_E. */ + if ((ret != 0) && (ret != WC_NO_ERR_TRACE(ASN_CRIT_EXT_E))) { ret = ASN_PARSE_E; } #else diff --git a/wolfcrypt/src/asn_orig.c b/wolfcrypt/src/asn_orig.c index 49438749f8c..e596231c0e5 100644 --- a/wolfcrypt/src/asn_orig.c +++ b/wolfcrypt/src/asn_orig.c @@ -4215,7 +4215,8 @@ static int DecodeNameConstraints(const byte* input, word32 sz, #endif #if defined(WOLFSSL_SEP) || defined(WOLFSSL_CERT_EXT) -static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) +static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert, + int critical) { word32 idx = 0; word32 oldIdx; @@ -4229,6 +4230,10 @@ static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) WOLFSSL_ENTER("DecodeCertPolicy"); +#if !defined(WOLFSSL_CERT_EXT) || defined(WOLFSSL_NO_ASN_STRICT) + (void)critical; +#endif + /* Check if cert is null before dereferencing below */ if (cert == NULL) return BAD_FUNC_ARG; @@ -4247,7 +4252,6 @@ static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) WOLFSSL_MSG("\tCertPolicy length mismatch"); return ASN_PARSE_E; } - if (total_length == 0) { WOLFSSL_MSG("\tCertPolicy empty sequence"); return ASN_PARSE_E; @@ -4326,6 +4330,20 @@ static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert) #endif ); +#if defined(WOLFSSL_CERT_EXT) && !defined(WOLFSSL_NO_ASN_STRICT) + /* RFC 5280 4.2: a critical certificatePolicies whose policies could not + * all be stored (the loop stopped at MAX_CERTPOL_NB) cannot be fully + * processed. Report it as an unsupported critical extension so the shared + * DecodeExtensionType()/DecodeCertExtensions() defer it, matching the + * template back-end. Non-critical extensions may be partially processed. */ + if (critical && (idx < seqEnd)) { + WOLFSSL_MSG("Cannot fully process critical certificatePolicies"); + WOLFSSL_ERROR_VERBOSE(ASN_CRIT_EXT_E); + WOLFSSL_LEAVE("DecodeCertPolicy", ASN_CRIT_EXT_E); + return ASN_CRIT_EXT_E; + } +#endif + WOLFSSL_LEAVE("DecodeCertPolicy", 0); return 0; }