Merge pull request #10336 from julek-wolfssl/wolfSSL_PEM_read_bio_X509_CRL-multi-crl-fix

src/x509.c: refactor wolfSSL_PEM_read_bio_X509_CRL onto the per-block reader

Author: dgarske

src/x509.c

@@ -13494,36 +13494,35 @@ WOLFSSL_X509 *wolfSSL_PEM_read_bio_X509_REQ(WOLFSSL_BIO *bp, WOLFSSL_X509 **x,
             WOLFSSL_X509_CRL **x, wc_pem_password_cb *cb, void *u)
     {
 #if defined(WOLFSSL_PEM_TO_DER) && defined(HAVE_CRL)
-        unsigned char* pem = NULL;
-        int pemSz = 0;
-        int derSz = 0;
-        DerBuffer* der = NULL;
         WOLFSSL_X509_CRL* crl = NULL;
 
         WOLFSSL_ENTER("wolfSSL_PEM_read_bio_X509_CRL");
 
-        if ((pem = ReadPemFromBioToBuffer(bp, &pemSz)) == NULL) {
-            goto err;
-        }
-
-        if ((PemToDer(pem, pemSz, CRL_TYPE, &der, NULL, NULL, NULL)) < 0) {
-            goto err;
-        }
-        derSz = (int)der->length;
-        if ((crl = wolfSSL_d2i_X509_CRL(x, der->buffer, derSz)) == NULL) {
-            goto err;
+        /* OpenSSL's PEM_read_bio_X509_CRL skips intervening cert/key blocks
+         * and returns the next CRL in the stream (NULL only at EOF). Mirror
+         * that by looping over the per-block reader until we get a CRL or
+         * the BIO has nothing left to parse. */
+        for (;;) {
+            WOLFSSL_X509*      x509   = NULL;
+            WOLFSSL_X509_PKEY* x_pkey = NULL;
+            if (wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio(bp, cb,
+                        &x509, &crl, &x_pkey) != WOLFSSL_SUCCESS) {
+                break;
+            }
+            if (crl != NULL) {
+                break;
+            }
+            wolfSSL_X509_free(x509);
+            wolfSSL_X509_PKEY_free(x_pkey);
         }
 
-err:
-        if (pemSz == 0) {
-            WOLFSSL_ERROR(ASN_NO_PEM_HEADER);
-        }
-        XFREE(pem, 0, DYNAMIC_TYPE_PEM);
-        if (der != NULL) {
-            FreeDer(&der);
+        if (x != NULL) {
+            if (*x != NULL && *x != crl) {
+                wolfSSL_X509_CRL_free(*x);
+            }
+            *x = crl;
         }
 
-        (void)cb;
         (void)u;
 
         return crl;
@@ -13686,7 +13685,7 @@ int wolfSSL_write_X509_CRL(WOLFSSL_X509_CRL* crl, const char* path, int type)
 #endif /* !NO_FILESYSTEM */
 
 #endif /* OPENSSL_EXTRA || OPENSSL_ALL */
-#ifdef OPENSSL_ALL
+#if defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL)
 
 #ifndef NO_BIO
     /* create and return a new WOLFSSL_X509_PKEY structure or NULL on failure */
@@ -13706,7 +13705,7 @@ int wolfSSL_write_X509_CRL(WOLFSSL_X509_CRL* crl, const char* path, int type)
 
 
     /* free up all memory used by "xPkey" passed in */
-    static void wolfSSL_X509_PKEY_free(WOLFSSL_X509_PKEY* xPkey)
+    void wolfSSL_X509_PKEY_free(WOLFSSL_X509_PKEY* xPkey)
     {
         if (xPkey != NULL) {
             wolfSSL_EVP_PKEY_free(xPkey->dec_pkey);
@@ -13732,7 +13731,7 @@ int wolfSSL_write_X509_CRL(WOLFSSL_X509_CRL* crl, const char* path, int type)
      * @param x_pkey Output
      * @return WOLFSSL_SUCCESS on success and WOLFSSL_FAILURE otherwise
      */
-    static int wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio(
+    int wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio(
             WOLFSSL_BIO* bio, wc_pem_password_cb* cb, WOLFSSL_X509** x509,
             WOLFSSL_X509_CRL** crl, WOLFSSL_X509_PKEY** x_pkey)
     {
@@ -13762,7 +13761,11 @@ int wolfSSL_write_X509_CRL(WOLFSSL_X509_CRL* crl, const char* path, int type)
             return WOLFSSL_FAILURE;
         }
 
-        if (l <= pem_struct_min_sz) {
+        if (l == 0) {
+            /* Streaming BIO (pipe/FIFO/socket): size unknown, use the cap. */
+            l = MAX_BIO_READ_BUFFER;
+        }
+        else if (l <= pem_struct_min_sz) {
             /* No certificate in buffer */
             WOLFSSL_ERROR(ASN_NO_PEM_HEADER);
             return WOLFSSL_FAILURE;
@@ -13916,6 +13919,9 @@ int wolfSSL_write_X509_CRL(WOLFSSL_X509_CRL* crl, const char* path, int type)
 #endif /* WOLFSSL_PEM_TO_DER || WOLFSSL_DER_TO_PEM */
     }
 
+#endif /* OPENSSL_EXTRA || OPENSSL_ALL */
+#ifdef OPENSSL_ALL
+
 #ifndef NO_FILESYSTEM
     WOLF_STACK_OF(WOLFSSL_X509_INFO)* wolfSSL_PEM_X509_INFO_read(
             XFILE fp, WOLF_STACK_OF(WOLFSSL_X509_INFO)* sk,
@@ -14053,7 +14059,7 @@ int wolfSSL_write_X509_CRL(WOLFSSL_X509_CRL* crl, const char* path, int type)
         return localSk;
     }
 #endif /* !NO_BIO */
-#endif /* OPENSSL_ALL */
+#endif /* OPENSSL_EXTRA || OPENSSL_ALL */
 
     void wolfSSL_X509_NAME_ENTRY_free(WOLFSSL_X509_NAME_ENTRY* ne)
     {

tests/api/test_ossl_x509_lu.c

@@ -312,6 +312,17 @@ int test_wolfSSL_X509_load_crl_file(void)
     #endif
         ""
     };
+    int pemCount[] = {
+        1,
+        2,
+        1,
+        1,
+        1,
+    #ifdef WC_RSA_PSS
+        1,
+    #endif
+        0
+    };
     char der[][100] = {
         "./certs/crl/crl.der",
         "./certs/crl/crl2.der",
@@ -342,7 +353,7 @@ int test_wolfSSL_X509_load_crl_file(void)
     ExpectIntEQ(X509_load_crl_file(lookup, pem[0], 0), 0);
     for (i = 0; pem[i][0] != '\0'; i++) {
         ExpectIntEQ(X509_load_crl_file(lookup, pem[i], WOLFSSL_FILETYPE_PEM),
-            1);
+            pemCount[i]);
     }
 
     if (store) {
@@ -394,6 +405,45 @@ int test_wolfSSL_X509_load_crl_file(void)
 
     X509_STORE_free(store);
     store = NULL;
+
+    /* Combine crl.pem (1 CRL), a server cert, and crl2.pem (2 CRLs) into a
+     * single memory BIO. wolfSSL_PEM_read_bio_X509_CRL must walk past the
+     * intervening certificate and hand back all three CRLs before NULL,
+     * matching OpenSSL's PEM_read_bio_X509_CRL behaviour. */
+    {
+        WOLFSSL_BIO* fileBio = NULL;
+        WOLFSSL_BIO* memBio = NULL;
+        WOLFSSL_X509_CRL* crl = NULL;
+        unsigned char buf[4096];
+        int n;
+        int crlCount = 0;
+        const char* sources[] = {
+            "./certs/crl/crl.pem",
+            "./certs/server-cert.pem",
+            "./certs/crl/crl2.pem",
+            NULL
+        };
+
+        ExpectNotNull(memBio = wolfSSL_BIO_new(wolfSSL_BIO_s_mem()));
+        for (i = 0; sources[i] != NULL; i++) {
+            ExpectNotNull(fileBio = wolfSSL_BIO_new_file(sources[i], "rb"));
+            while (fileBio != NULL &&
+                   (n = wolfSSL_BIO_read(fileBio, buf, sizeof(buf))) > 0) {
+                ExpectIntEQ(wolfSSL_BIO_write(memBio, buf, n), n);
+            }
+            wolfSSL_BIO_free(fileBio);
+            fileBio = NULL;
+        }
+
+        while ((crl = wolfSSL_PEM_read_bio_X509_CRL(memBio, NULL, NULL, NULL))
+               != NULL) {
+            crlCount++;
+            wolfSSL_X509_CRL_free(crl);
+        }
+        ExpectIntEQ(crlCount, 3);
+
+        wolfSSL_BIO_free(memBio);
+    }
 #endif
     return EXPECT_RESULT();
 }

tests/api/test_ossl_x509_lu.h

@@ -34,7 +34,7 @@ int test_X509_LOOKUP_add_dir(void);
     TEST_DECL_GROUP("ossl_x509_lu", test_wolfSSL_X509_LOOKUP_load_file),       \
     TEST_DECL_GROUP("ossl_x509_lu", test_wolfSSL_X509_LOOKUP_ctrl_file),       \
     TEST_DECL_GROUP("ossl_x509_lu", test_wolfSSL_X509_LOOKUP_ctrl_hash_dir),   \
-    TEST_DECL_GROUP("ossl_x509_lu", test_wolfSSL_X509_LOOKUP_ctrl_hash_dir),   \
+    TEST_DECL_GROUP("ossl_x509_lu", test_wolfSSL_X509_load_crl_file),          \
     TEST_DECL_GROUP("ossl_x509_lu", test_X509_LOOKUP_add_dir)
 
 #endif /* WOLFCRYPT_TEST_OSSL_X509_LU_H */

wolfssl/internal.h

@@ -7470,6 +7470,15 @@ WOLFSSL_LOCAL int pkcs8_encrypt(WOLFSSL_EVP_PKEY* pkey,
         word32* keySz);
 #endif /* OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL */
 
+#if (defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL)) && !defined(NO_BIO)
+WOLFSSL_LOCAL int wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio(
+        WOLFSSL_BIO* bio, wc_pem_password_cb* cb, WOLFSSL_X509** x509,
+        WOLFSSL_X509_CRL** crl, WOLFSSL_X509_PKEY** x_pkey);
+#endif
+#if defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL)
+WOLFSSL_LOCAL void wolfSSL_X509_PKEY_free(WOLFSSL_X509_PKEY* xPkey);
+#endif
+
 WOLFSSL_LOCAL void wolfssl_local_MaybeCheckAlertOnErr(WOLFSSL* ssl, int err);
 
 #ifdef __cplusplus