fix(tls): address PR #311 review (crl der/fail-closed, asan, docs)

Four review findings on the TLS-gaps work:

1. use_pkcs12_file assigned its password with std::string(passphrase),
   reintroducing the one-past-the-end pointer that trips ASan
   detect_invalid_pointer_pairs on a non-null-terminated view (the same
   bug just fixed in use_pkcs12). Use assign(ptr, len) to match.

2. CRL loading was PEM-only and dropped parse failures silently, so a
   DER or malformed CRL vanished — and under soft_fail a peer the missing
   CRL might have revoked was then accepted (fail-open), contradicting the
   "PEM or DER" contract. Both backends now try PEM then DER, and a CRL
   that parses as neither fails the handshake closed (OpenSSL in
   do_handshake, WolfSSL in init_ssl_for_role). New generic test asserts a
   malformed CRL under soft_fail fails rather than silently accepting.

3. wolfssl_stream comment claimed PKCS#12 CA/chain entries "are not
   loaded"; the code loads and sends them (and testPkcs12Chain proves it).
   Comment corrected.

4. Added the "Copyright (c) 2026 Michael Vandeberg" line to two files
   substantially modified in this PR (test/unit/tls_context.cpp,
   include/boost/corosio/tls_stream.hpp) per the repo convention.

Verified on system WolfSSL (HAVE_CRL off), a vcpkg-flag WolfSSL 5.8.2
build (HAVE_CRL on), and the asan+ubsan config with the CI ASAN_OPTIONS:
all TLS suites pass.

Author: mvandeberg

include/boost/corosio/tls_stream.hpp

@@ -1,5 +1,6 @@
 //
 // Copyright (c) 2025 Vinnie Falco (vinnie.falco@gmail.com)
+// Copyright (c) 2026 Michael Vandeberg
 //
 // Distributed under the Boost Software License, Version 1.0. (See accompanying
 // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

src/corosio/src/tls/context.cpp

@@ -117,8 +117,12 @@ tls_context::use_pkcs12_file(
 
     std::ostringstream ss;
     ss << file.rdbuf();
-    impl_->pkcs12_data     = ss.str();
-    impl_->pkcs12_password = std::string(passphrase);
+    impl_->pkcs12_data = ss.str();
+    // assign(ptr, len), not std::string(passphrase): see the note in
+    // use_pkcs12 — constructing from a string_view forms a one-past-the-end
+    // pointer that ASan's detect_invalid_pointer_pairs rejects on a
+    // non-null-terminated view.
+    impl_->pkcs12_password.assign(passphrase.data(), passphrase.size());
     return {};
 }
 

src/openssl/src/openssl_stream.cpp

@@ -314,6 +314,10 @@ class openssl_native_context : public native_context_base
 public:
     SSL_CTX* ctx_;
     tls_context_data const* cd_;
+    // Set when a caller-supplied CRL could not be parsed as PEM or DER.
+    // A dropped CRL would silently weaken revocation (fail-open under
+    // soft_fail), so do_handshake refuses the handshake instead.
+    bool crl_parse_failed_ = false;
 
     explicit openssl_native_context(tls_context_data const& cd)
         : ctx_(nullptr)
@@ -514,19 +518,36 @@ class openssl_native_context : public native_context_base
         // error.
         if (cd.revocation != tls_revocation_policy::disabled)
         {
-            for (auto const& crl_pem : cd.crls)
+            for (auto const& crl_data : cd.crls)
             {
                 BIO* bio = BIO_new_mem_buf(
-                    crl_pem.data(), static_cast<int>(crl_pem.size()));
+                    crl_data.data(), static_cast<int>(crl_data.size()));
                 if (!bio)
+                {
+                    crl_parse_failed_ = true;
                     continue;
+                }
+                // Accept PEM or DER (the documented contract). Try PEM first,
+                // then rewind and try DER.
                 X509_CRL* crl =
                     PEM_read_bio_X509_CRL(bio, nullptr, nullptr, nullptr);
+                if (!crl)
+                {
+                    BIO_reset(bio);
+                    crl = d2i_X509_CRL_bio(bio, nullptr);
+                }
                 if (crl)
                 {
                     X509_STORE_add_crl(store, crl);
                     X509_CRL_free(crl);
                 }
+                else
+                {
+                    // A supplied CRL that parses as neither PEM nor DER must
+                    // not be silently dropped; record it so the handshake
+                    // fails closed rather than weakening revocation.
+                    crl_parse_failed_ = true;
+                }
                 BIO_free(bio);
             }
             X509_STORE_set_flags(store, X509_V_FLAG_CRL_CHECK);
@@ -554,12 +575,18 @@ class openssl_native_context : public native_context_base
     }
 };
 
-inline SSL_CTX*
-get_openssl_context(tls_context_data const& cd)
+inline openssl_native_context*
+get_openssl_native_context(tls_context_data const& cd)
 {
     static char key;
     auto* p = cd.find(&key, [&] { return new openssl_native_context(cd); });
-    return static_cast<openssl_native_context*>(p)->ctx_;
+    return static_cast<openssl_native_context*>(p);
+}
+
+SSL_CTX*
+get_openssl_context(tls_context_data const& cd)
+{
+    return get_openssl_native_context(cd)->ctx_;
 }
 
 } // namespace detail
@@ -822,6 +849,14 @@ struct openssl_stream::impl
 
     capy::io_task<> do_handshake(int type)
     {
+        // A caller-supplied CRL that parsed as neither PEM nor DER weakens
+        // revocation. Refuse the handshake rather than fail open (soft_fail
+        // would otherwise treat the dropped CRL as "status unknown").
+        if (detail::get_openssl_native_context(
+                detail::get_tls_context_data(ctx_))
+                ->crl_parse_failed_)
+            co_return std::make_error_code(std::errc::invalid_argument);
+
         if (used_)
             reset();
 

src/wolfssl/src/wolfssl_stream.cpp

@@ -295,8 +295,12 @@ class wolfssl_native_context : public native_context_base
 public:
     WOLFSSL_CTX* client_ctx_;
     WOLFSSL_CTX* server_ctx_;
+    // Set when a caller-supplied CRL could not be parsed as PEM or DER, so
+    // init_ssl_for_role can fail closed rather than silently weaken
+    // revocation (fail-open under soft_fail).
+    bool crl_parse_failed_ = false;
 
-    static void
+    void
     apply_common_settings(WOLFSSL_CTX* ctx, tls_context_data const& cd)
     {
         if (!ctx)
@@ -345,7 +349,8 @@ class wolfssl_native_context : public native_context_base
 
         // PKCS#12 bundle: decode cert + key with the native wolfcrypt API
         // (the wolfSSL_d2i_PKCS12_bio wrapper needs OPENSSL_EXTRA). CA/chain
-        // entries inside the bundle are not loaded.
+        // entries inside the bundle are loaded and sent during the handshake
+        // (see below), matching the OpenSSL backend.
         if (!cd.pkcs12_data.empty())
         {
             WC_PKCS12* p12 = wc_PKCS12_new();
@@ -530,9 +535,19 @@ class wolfssl_native_context : public native_context_base
         {
             wolfSSL_CTX_EnableCRL(ctx, WOLFSSL_CRL_CHECK);
             for (auto const& crl : cd.crls)
-                wolfSSL_CTX_LoadCRLBuffer(
-                    ctx, reinterpret_cast<unsigned char const*>(crl.data()),
-                    static_cast<long>(crl.size()), WOLFSSL_FILETYPE_PEM);
+            {
+                auto const* buf =
+                    reinterpret_cast<unsigned char const*>(crl.data());
+                auto const sz = static_cast<long>(crl.size());
+                // Accept PEM or DER (the documented contract): try PEM, then
+                // DER. A supplied CRL that parses as neither must not be
+                // silently dropped, so record it for a fail-closed handshake.
+                if (wolfSSL_CTX_LoadCRLBuffer(
+                        ctx, buf, sz, WOLFSSL_FILETYPE_PEM) != WOLFSSL_SUCCESS &&
+                    wolfSSL_CTX_LoadCRLBuffer(
+                        ctx, buf, sz, WOLFSSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
+                    crl_parse_failed_ = true;
+            }
             // soft_fail tolerates an undeterminable revocation status (no CRL
             // loaded for a cert) the way OpenSSL does. Without this, WolfSSL's
             // WOLFSSL_CRL_CHECK hard-fails with CRL_MISSING; the callback
@@ -1285,6 +1300,12 @@ struct wolfssl_stream::impl
                 wolfSSL_get_error(nullptr, 0), wolfssl_category());
         }
 
+        // A caller-supplied CRL that could not be parsed (neither PEM nor
+        // DER) must not silently weaken revocation. Fail closed rather than
+        // let soft_fail accept a cert the dropped CRL might have revoked.
+        if (native->crl_parse_failed_)
+            return std::make_error_code(std::errc::invalid_argument);
+
         // Select appropriate context based on role
         WOLFSSL_CTX* native_ctx = (type == wolfssl_stream::client)
             ? native->client_ctx_

test/unit/tls_context.cpp

@@ -1,5 +1,6 @@
 //
 // Copyright (c) 2026 Steve Gerbino
+// Copyright (c) 2026 Michael Vandeberg
 //
 // Distributed under the Boost Software License, Version 1.0. (See accompanying
 // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

test/unit/tls_stream_tests.hpp

@@ -490,11 +490,15 @@ testAlpnAccessorEmpty(StreamFactory make_stream)
 
     The server presents a leaf that a CRL revokes.
 
-    When @p crl_supported (OpenSSL):
+    When @p crl_supported (OpenSSL, or WolfSSL built with HAVE_CRL):
       1. hard_fail with the CRL loaded rejects the revoked leaf.
       2. soft_fail with no CRL loaded accepts (status unknown is allowed).
     Otherwise (WolfSSL without HAVE_CRL): any revocation request fails the
     handshake with function_not_supported rather than skip the check.
+
+    On every build, a CRL that parses as neither PEM nor DER fails the
+    handshake closed rather than being silently dropped (which would let
+    soft_fail accept a peer the missing CRL might have revoked).
 */
 template<typename StreamFactory>
 void
@@ -552,6 +556,27 @@ testCrlRevocation(StreamFactory make_stream, bool crl_supported)
         run_tls_test_fail(
             ioc, client_ctx, server_ctx, make_stream, make_stream);
     }
+
+    // A supplied CRL that parses as neither PEM nor DER must fail the
+    // handshake, never silently downgrade to accepting the peer. This holds
+    // on every build: a HAVE_CRL backend rejects the unparseable CRL, and a
+    // backend without CRL support rejects any revocation request outright.
+    // Uses soft_fail specifically: without the fail-closed guard, soft_fail
+    // would treat the missing (dropped) CRL as "status unknown" and accept.
+    {
+        io_context ioc;
+        tls_context client_ctx;
+        // NOLINTNEXTLINE(bugprone-unused-return-value)
+        client_ctx.add_certificate_authority(root_ca_cert_pem);
+        // NOLINTNEXTLINE(bugprone-unused-return-value)
+        client_ctx.set_verify_mode(tls_verify_mode::peer);
+        // NOLINTNEXTLINE(bugprone-unused-return-value)
+        client_ctx.add_crl("this is not a valid PEM or DER CRL");
+        client_ctx.set_revocation_policy(tls_revocation_policy::soft_fail);
+        auto server_ctx = revoked_server();
+        run_tls_test_fail(
+            ioc, client_ctx, server_ctx, make_stream, make_stream);
+    }
 }
 
 /** Test loading server credentials from a PKCS#12 bundle.