From 4a85f00240dbdd2dbcede832a255b6cadb32dd82 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 28 Apr 2026 10:06:47 +0000 Subject: [PATCH 1/2] src/x509.c: refactor wolfSSL_PEM_read_bio_X509_CRL onto the per-block reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReadPemFromBioToBuffer slurps the entire BIO in one shot, so iterative callers like wolfSSL_PEM_read_bio_X509_CRL (and by extension wolfSSL_X509_load_crl_file's BIO branch) saw EOF after the first block and silently dropped every CRL after the first in a multi-CRL bundle. Refactor wolfSSL_PEM_read_bio_X509_CRL to delegate to wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio, which already reads one PEM BEGIN/END pair per call and leaves the BIO positioned just past the END line. Loop over it so we skip past intervening cert/key blocks and return the next CRL in the stream — matching OpenSSL's PEM_read_bio_X509_CRL, verified against OpenSSL 3.0.13 with cases {cert,CRL}, {CRL,cert}, {CRL,cert,CRL}, {key,CRL}, {CRL,key,CRL}: in each case OpenSSL skips non-CRL blocks until EOF. When the caller passes a non-NULL `x` whose `*x` is already populated, free the previous CRL before overwriting the slot — matching the d2i_X509_CRL reuse contract the old body relied on. To keep both helpers visible at the new call site, drop their `static` qualifier (wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio for the per-block read, wolfSSL_X509_PKEY_free to free defensively-allocated keys parsed from intervening non-CRL blocks). Their definitions in src/x509.c and declarations in wolfssl/internal.h are widened from OPENSSL_ALL to OPENSSL_EXTRA || OPENSSL_ALL so the OPENSSL_EXTRA-only build (which compiles wolfSSL_PEM_read_bio_X509_CRL) links cleanly. The unrelated INFO_read_bio / INFO_read_bio_X509_INFO group below them keeps its OPENSSL_ALL gate because it depends on wolfSSL_X509_INFO_new/free that are still OPENSSL_ALL-only. Also register the previously-orphaned test_wolfSSL_X509_load_crl_file (its slot in TEST_OSSL_X509_LOOKUP_DECLS was a duplicated test_wolfSSL_X509_LOOKUP_ctrl_hash_dir entry), update its assertion for crl2.pem (which already contains two CRLs) to expect 2 instead of 1, and add a multi-CRL bundle case that builds a memory BIO from crl.pem + server-cert.pem + crl2.pem and asserts that the reader walks past the cert and returns all 3 CRLs before NULL. --- src/x509.c | 54 ++++++++++++++++++----------------- tests/api/test_ossl_x509_lu.c | 52 ++++++++++++++++++++++++++++++++- tests/api/test_ossl_x509_lu.h | 2 +- wolfssl/internal.h | 9 ++++++ 4 files changed, 89 insertions(+), 28 deletions(-) diff --git a/src/x509.c b/src/x509.c index b0e695d707..19052d00dc 100644 --- a/src/x509.c +++ b/src/x509.c @@ -13499,36 +13499,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; + /* 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); } - 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; + if (x != NULL) { + if (*x != NULL && *x != crl) { + wolfSSL_X509_CRL_free(*x); + } + *x = crl; } -err: - if (pemSz == 0) { - WOLFSSL_ERROR(ASN_NO_PEM_HEADER); - } - XFREE(pem, 0, DYNAMIC_TYPE_PEM); - if (der != NULL) { - FreeDer(&der); - } - - (void)cb; (void)u; return crl; @@ -13691,7 +13690,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 */ @@ -13711,7 +13710,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); @@ -13737,7 +13736,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) { @@ -13921,6 +13920,9 @@ err: #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, @@ -14058,7 +14060,7 @@ err: return localSk; } #endif /* !NO_BIO */ -#endif /* OPENSSL_ALL */ +#endif /* OPENSSL_EXTRA || OPENSSL_ALL */ void wolfSSL_X509_NAME_ENTRY_free(WOLFSSL_X509_NAME_ENTRY* ne) { diff --git a/tests/api/test_ossl_x509_lu.c b/tests/api/test_ossl_x509_lu.c index 541c9045e3..07213e6136 100644 --- a/tests/api/test_ossl_x509_lu.c +++ b/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(); } diff --git a/tests/api/test_ossl_x509_lu.h b/tests/api/test_ossl_x509_lu.h index 4073982cfb..72a7991b1e 100644 --- a/tests/api/test_ossl_x509_lu.h +++ b/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 */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 96606ba4f9..368bb15ac1 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -7469,6 +7469,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 From b261ee623891d597677252cb97e06f6b84e4f31c Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 29 Apr 2026 15:24:05 +0000 Subject: [PATCH 2/2] src/x509.c: handle streaming BIOs in PEM block reader The CRL refactor broke nginx's ssl_cache.t (and the wolfSSL/wolfssl nginx_check matrix on 1.24.0/1.25.0/1.28.1) because nginx loads the test CRL through a FIFO. wolfSSL_PEM_X509_X509_CRL_X509_PKEY_read_bio() asks wolfSSL_BIO_get_len() for the BIO size up front; for a FIFO the underlying ftell() returns ESPIPE, wolfssl_file_len() reports WOLFSSL_BAD_FILETYPE, and BIO_get_len() returns 0. The function then hit the l <= pem_struct_min_sz guard and bailed with ASN_NO_PEM_HEADER before reading a byte, so the caller's loop saw "no CRL" and nginx emitted "PEM_read_bio_X509_CRL() failed". Treat l == 0 as "streaming source, size unknown" and allocate up to MAX_BIO_READ_BUFFER (the same cap ReadPemFromBioToBuffer used for this case before the refactor). The existing byte-by-byte reader already stops at the END marker or at EOF, so this is enough; if the upstream short-reads we still surface ASN_NO_PEM_HEADER from the pem_struct_min_sz read below. Keep rejecting tiny non-zero lengths since those are real "buffer too small" cases. --- src/x509.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/x509.c b/src/x509.c index 19052d00dc..f7df3d75b4 100644 --- a/src/x509.c +++ b/src/x509.c @@ -13766,7 +13766,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;