Check for duplicate extensions in a CRL

Author: anhu

tests/api.c

@@ -4209,6 +4209,44 @@ static int test_wolfSSL_CertManagerNameConstraint5(void)
     return EXPECT_RESULT();
 }
 
+static int test_wolfSSL_CRL_duplicate_extensions(void)
+{
+    EXPECT_DECLS;
+#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && defined(HAVE_CRL) && \
+    !defined(NO_RSA)
+    const unsigned char crl_duplicate_akd[] =
+        "-----BEGIN X509 CRL-----\n"
+        "MIICCDCB8QIBATANBgkqhkiG9w0BAQsFADB5MQswCQYDVQQGEwJVUzETMBEGA1UE\n"
+        "CAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzETMBEGA1UECgwK\n"
+        "TXkgQ29tcGFueTETMBEGA1UEAwwKTXkgUm9vdCBDQTETMBEGA1UECwwKTXkgUm9v\n"
+        "dCBDQRcNMjQwOTAxMDAwMDAwWhcNMjUxMjAxMDAwMDAwWqBEMEIwHwYDVR0jBBgw\n"
+        "FoAU72ng99Ud5pns3G3Q9+K5XGRxgzUwHwYDVR0jBBgwFoAU72ng99Ud5pns3G3Q\n"
+        "9+K5XGRxgzUwDQYJKoZIhvcNAQELBQADggEBAIFVw4jrS4taSXR/9gPzqGrqFeHr\n"
+        "IXCnFtHJTLxqa8vUOAqSwqysvNpepVKioMVoGrLjFMjANjWQqTEiMROAnLfJ/+L8\n"
+        "FHZkV/mZwOKAXMhIC9MrJzifxBICwmvD028qnwQm09EP8z4ICZptD6wPdRTDzduc\n"
+        "KBuAX+zn8pNrJgyrheRKpPgno9KsbCzK4D/RIt1sTK2M3vVOtY+vpsN70QYUXvQ4\n"
+        "r2RZac3omlT43x5lddPxIlcouQpwWcVvr/K+Va770MRrjn88PBrJmvsEw/QYVBXp\n"
+        "Gxv2b78HFDacba80sMIm8ltRdqUCa5qIc6OATsz7izCQXEbkTEeESrcK1MA=\n"
+        "-----END X509 CRL-----\n";
+
+    WOLFSSL_CERT_MANAGER* cm = NULL;
+    int ret;
+
+    cm = wolfSSL_CertManagerNew();
+    ExpectNotNull(cm);
+
+    /* Test loading CRL with duplicate extensions */
+    WOLFSSL_MSG("Testing CRL with duplicate Authority Key Identifier extensions");
+    ret = wolfSSL_CertManagerLoadCRLBuffer(cm, crl_duplicate_akd,
+                                           sizeof(crl_duplicate_akd),
+                                           WOLFSSL_FILETYPE_PEM);
+    ExpectIntEQ(ret, ASN_PARSE_E);
+
+    wolfSSL_CertManagerFree(cm);
+#endif
+    return EXPECT_RESULT();
+}
+
 static int test_wolfSSL_CertManagerCRL(void)
 {
     EXPECT_DECLS;
@@ -67425,6 +67463,7 @@ TEST_CASE testCases[] = {
     TEST_DECL(test_wolfSSL_CertManagerNameConstraint4),
     TEST_DECL(test_wolfSSL_CertManagerNameConstraint5),
     TEST_DECL(test_wolfSSL_CertManagerCRL),
+    TEST_DECL(test_wolfSSL_CRL_duplicate_extensions),
     TEST_DECL(test_wolfSSL_CertManagerCheckOCSPResponse),
     TEST_DECL(test_wolfSSL_CheckOCSPResponse),
 #ifdef HAVE_CERT_CHAIN_VALIDATION

wolfcrypt/src/asn.c

@@ -39147,6 +39147,9 @@ static int ParseCRL_Extensions(DecodedCRL* dcrl, const byte* buf, word32 idx,
 {
     DECL_ASNGETDATA(dataASN, certExtASN_Length);
     int ret = 0;
+    /* Track if we've seen these extensions already */
+    word32 seenAuthKey = 0;
+    word32 seenCrlNum = 0;
 
     ALLOC_ASNGETDATA(dataASN, certExtASN_Length, ret, dcrl->heap);
 
@@ -39168,47 +39171,64 @@ static int ParseCRL_Extensions(DecodedCRL* dcrl, const byte* buf, word32 idx,
             /* Length of extension data. */
             int length = (int)dataASN[CERTEXTASN_IDX_VAL].length;
 
-            if (oid == AUTH_KEY_OID) {
-            #ifndef NO_SKID
-                /* Parse Authority Key Id extension.
-                 * idx is at start of OCTET_STRING data. */
-                ret = ParseCRL_AuthKeyIdExt(buf + idx, length, dcrl);
-                if (ret != 0) {
-                    WOLFSSL_MSG("\tcouldn't parse AuthKeyId extension");
-                }
-            #endif
+            /* Check for duplicate extension */
+            if ((oid == AUTH_KEY_OID && seenAuthKey) ||
+                (oid == CRL_NUMBER_OID && seenCrlNum)) {
+                WOLFSSL_MSG("Duplicate CRL extension found");
+                ret = ASN_PARSE_E;
             }
-            else if (oid == CRL_NUMBER_OID) {
-            #ifdef WOLFSSL_SMALL_STACK
-                mp_int* m = (mp_int*)XMALLOC(sizeof(*m), NULL,
-                        DYNAMIC_TYPE_BIGINT);
-                if (m == NULL) {
-                    ret = MEMORY_E;
-                }
-            #else
-                mp_int m[1];
-            #endif
 
-                if (ret == 0) {
-                    if (mp_init(m) != MP_OKAY) {
-                        ret = MP_INIT_E;
+            /* Track this extension if no duplicate found */
+            if (ret == 0) {
+                if (oid == AUTH_KEY_OID)
+                    seenAuthKey = 1;
+                else if (oid == CRL_NUMBER_OID)
+                    seenCrlNum = 1;
+            }
+
+            if (ret == 0) {
+                if (oid == AUTH_KEY_OID) {
+                #ifndef NO_SKID
+                    /* Parse Authority Key Id extension.
+                     * idx is at start of OCTET_STRING data. */
+                    ret = ParseCRL_AuthKeyIdExt(buf + idx, length, dcrl);
+                    if (ret != 0) {
+                        WOLFSSL_MSG("\tcouldn't parse AuthKeyId extension");
                     }
+                #endif
                 }
-                if (ret == 0) {
-                    ret = GetInt(m, buf, &idx, maxIdx);
-                }
-                if (ret == 0) {
-                    dcrl->crlNumber = (int)m->dp[0];
-                }
+                else if (oid == CRL_NUMBER_OID) {
+                #ifdef WOLFSSL_SMALL_STACK
+                    mp_int* m = (mp_int*)XMALLOC(sizeof(*m), NULL,
+                            DYNAMIC_TYPE_BIGINT);
+                    if (m == NULL) {
+                        ret = MEMORY_E;
+                    }
+                #else
+                    mp_int m[1];
+                #endif
 
-                mp_free(m);
-            #ifdef WOLFSSL_SMALL_STACK
-                XFREE(m, NULL, DYNAMIC_TYPE_BIGINT);
-            #endif
+                    if (ret == 0) {
+                       if (mp_init(m) != MP_OKAY) {
+                           ret = MP_INIT_E;
+                        }
+                    }
+                    if (ret == 0) {
+                        ret = GetInt(m, buf, &idx, maxIdx);
+                    }
+                    if (ret == 0) {
+                        dcrl->crlNumber = (int)m->dp[0];
+                    }
+
+                    mp_free(m);
+                #ifdef WOLFSSL_SMALL_STACK
+                    XFREE(m, NULL, DYNAMIC_TYPE_BIGINT);
+                #endif
+                }
+                /* TODO: check criticality */
+                /* Move index on to next extension. */
+                idx += (word32)length;
             }
-            /* TODO: check criticality */
-            /* Move index on to next extension. */
-            idx += (word32)length;
         }
     }