- asn: reject empty certificatePolicies sequences

must be 1..MAX as per RFC 5280 4.2.1.4;
- asn: reject trailing bytes after the last PolicyInformation in certificatePolicies;
- minor edit updating the whitebox notes to match with these new
  changes;

Author: gasbytes

tests/api/test_asn.c

@@ -1577,6 +1577,60 @@ int test_DecodeCertExtensions_dup_certpol(void)
     return EXPECT_RESULT();
 }
 
+/* RFC 5280 4.2.1.4 defines certificatePolicies as SEQUENCE SIZE (1..MAX) OF
+ * PolicyInformation, so an empty SEQUENCE must be rejected instead of being
+ * accepted as zero policies. */
+int test_DecodeCertExtensions_empty_certpol(void)
+{
+    EXPECT_DECLS;
+#if (defined(WOLFSSL_SEP) || defined(WOLFSSL_CERT_EXT)) && \
+    !defined(NO_CERTS) && !defined(NO_ASN)
+    /* certificatePolicies extnValue carrying no PolicyInformation. */
+    static const byte emptyPolicy[] = {
+        0x30, 0x00                          /* certificatePolicies SEQUENCE */
+    };
+    DecodedCert cert;
+    int isUnknown = 0;
+
+    wc_InitDecodedCert(&cert, emptyPolicy, (word32)sizeof(emptyPolicy), NULL);
+
+    ExpectIntEQ(DecodeExtensionType(emptyPolicy, (word32)sizeof(emptyPolicy),
+        CERT_POLICY_OID, 0, &cert, &isUnknown),
+        WC_NO_ERR_TRACE(ASN_PARSE_E));
+
+    wc_FreeDecodedCert(&cert);
+#endif
+    return EXPECT_RESULT();
+}
+
+/* Trailing bytes after the last PolicyInformation must be rejected rather than
+ * skipped. */
+int test_DecodeCertExtensions_certpol_trailing_junk(void)
+{
+    EXPECT_DECLS;
+#if (defined(WOLFSSL_SEP) || defined(WOLFSSL_CERT_EXT)) && \
+    !defined(NO_CERTS) && !defined(NO_ASN)
+    /* One valid PolicyInformation followed by two bytes that are not one. */
+    static const byte trailingJunk[] = {
+        0x30, 0x09,                          /* certificatePolicies SEQUENCE */
+            0x30, 0x05,                      /* PolicyInformation SEQUENCE */
+                0x06, 0x03, 0x2A, 0x03, 0x04,/* policyIdentifier OID 1.2.3.4 */
+            0x00, 0x00                       /* trailing junk */
+    };
+    DecodedCert cert;
+    int isUnknown = 0;
+
+    wc_InitDecodedCert(&cert, trailingJunk, (word32)sizeof(trailingJunk), NULL);
+
+    ExpectIntEQ(DecodeExtensionType(trailingJunk, (word32)sizeof(trailingJunk),
+        CERT_POLICY_OID, 0, &cert, &isUnknown),
+        WC_NO_ERR_TRACE(ASN_PARSE_E));
+
+    wc_FreeDecodedCert(&cert);
+#endif
+    return EXPECT_RESULT();
+}
+
 int test_ParseCert_SM3wSM2_short_pubkey(void)
 {
     EXPECT_DECLS;

tests/api/test_asn.h

@@ -36,6 +36,8 @@ int test_wc_DecodeRsaPssParams(void);
 int test_SerialNumber0_RootCA(void);
 int test_DecodeAltNames_length_underflow(void);
 int test_DecodeCertExtensions_dup_certpol(void);
+int test_DecodeCertExtensions_empty_certpol(void);
+int test_DecodeCertExtensions_certpol_trailing_junk(void);
 int test_ParseCert_SM3wSM2_short_pubkey(void);
 int test_ParseCert_dnBufferBoundary(void);
 int test_wc_DecodeObjectId(void);
@@ -61,6 +63,8 @@ int test_wc_AsnFeatureCoverage(void);
     TEST_DECL_GROUP("asn", test_SerialNumber0_RootCA),              \
     TEST_DECL_GROUP("asn", test_DecodeAltNames_length_underflow),   \
     TEST_DECL_GROUP("asn", test_DecodeCertExtensions_dup_certpol),  \
+    TEST_DECL_GROUP("asn", test_DecodeCertExtensions_empty_certpol), \
+    TEST_DECL_GROUP("asn", test_DecodeCertExtensions_certpol_trailing_junk), \
     TEST_DECL_GROUP("asn", test_ParseCert_SM3wSM2_short_pubkey),    \
     TEST_DECL_GROUP("asn", test_ParseCert_dnBufferBoundary),        \
     TEST_DECL_GROUP("asn", test_wc_DecodeObjectId),                 \

tests/unit-mcdc/test_asn_ext_whitebox.c

@@ -1288,7 +1288,8 @@ static void wb_decode_policy_oid(void)
 /* ------------------------------------------------------------------------- *
  * Section 17: DecodeCertPolicy() (static, called directly).
  * Gated on WOLFSSL_SEP || WOLFSSL_CERT_EXT, same as the source.
- *   :21346  while ((ret==0) && (idx<total_length) && (extCertPoliciesNb<MAX_CERTPOL_NB))
+ *   :21346  while ((ret==0) && (idx<sz) && (extCertPoliciesNb<MAX_CERTPOL_NB))
+ *           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)
  * MAX_CERTPOL_NB is 2, so three policies exercise the count limit.
@@ -1338,10 +1339,11 @@ static void wb_decode_cert_policy(void)
     /* Zero policies. */
     static const byte noPolicies[] = { 0x30, 0x00 };
 
-    WB_NOTE("DecodeCertPolicy(): zero policies (loop false via idx<total_length) [:21346]");
+    WB_NOTE("DecodeCertPolicy(): empty SEQUENCE rejected before the loop (total_length==0)");
     XMEMSET(&cert, 0, sizeof(cert));
     ret = DecodeCertPolicy(noPolicies, sizeof(noPolicies), &cert);
-    WB_CHECK(ret == 0, "no policies present");
+    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));

wolfcrypt/src/asn.c

@@ -21481,10 +21481,15 @@ static int DecodeCertPolicy(const byte* input, word32 sz, DecodedCert* cert)
         {
             ret = ASN_PARSE_E;
         }
+        /* RFC 5280 4.2.1.4: certificatePolicies is SEQUENCE SIZE (1..MAX). */
+        else if (total_length == 0) {
+            ret = ASN_PARSE_E;
+        }
     }
 
-    /* Unwrap certificatePolicies */
-    while ((ret == 0) && ((int)idx < total_length)
+    /* Unwrap certificatePolicies. idx is an offset into input, so it is bound
+     * by sz. */
+    while ((ret == 0) && (idx < sz)
     #if defined(WOLFSSL_CERT_EXT)
         && (cert->extCertPoliciesNb < MAX_CERTPOL_NB)
     #endif