Fix/tls gaps#311
Merged
Merged
Conversation
|
An automated preview of the documentation is available at https://311.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-07-10 19:28:56 UTC |
mvandeberg
added a commit
to mvandeberg/corosio
that referenced
this pull request
Jul 9, 2026
…n, 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.
mvandeberg
added a commit
to mvandeberg/corosio
that referenced
this pull request
Jul 9, 2026
…edence) Address the three lower-confidence PR cppalliance#311 review candidates: - Inverted protocol window (min > max) produced a working, min-only context on WolfSSL (wolfssl_method_for keys off min and drops the ceiling) instead of the failing handshake the docs promise. Both backends now detect min > max and fail closed. - Native setup calls whose failure was ignored now fail closed rather than silently falling back: SSL_CTX_set_min/max_proto_version and set_cipher_list/set_ciphersuites (OpenSSL); SetMinVersion, set_cipher_list and EnableCRL (WolfSSL). A rejected cipher string no longer silently reverts to the default suites. - Discrete cert/key/chain fields could override a PKCS#12 credential even though the docs say they are consulted only when no bundle is supplied; both backends now skip them when a bundle is present. The per-context CRL-parse flag added earlier is generalized to a single setup_failed_ signal covering all of the above; the handshake refuses (std::errc::invalid_argument) when set. New tests: an inverted window fails closed, and a PKCS#12 bundle wins over an (expired) discrete cert. Verified on system WolfSSL (HAVE_CRL off), vcpkg-flag WolfSSL 5.8.2 (HAVE_CRL on), and asan+ubsan — all TLS suites pass.
b7d46d7 to
0be9581
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop-2 #311 +/- ##
=============================================
+ Coverage 77.89% 77.94% +0.05%
=============================================
Files 96 96
Lines 7139 7083 -56
Branches 1744 1705 -39
=============================================
- Hits 5561 5521 -40
+ Misses 1076 1061 -15
+ Partials 502 501 -1
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
…ecking Wire up TLS features that were declared but not applied, with OpenSSL and WolfSSL parity (fail-closed on WolfSSL builds lacking a capability). - ALPN: negotiate protocols on OpenSSL and add an alpn_protocol() accessor; fail closed on WolfSSL builds without HAVE_ALPN. - Protocol version: apply the configured min/max window; an inverted window fails closed. - Ciphersuites: select TLS 1.3 suites on both backends and drop the forced security level so a weak cipher string fails loudly. - PKCS#12: load bundles (leaf, key, and intermediate chain) on both backends; a bundle takes precedence over discrete cert/key fields. - CRL revocation: check on OpenSSL (PEM or DER, soft/hard fail) and fail closed on WolfSSL builds without HAVE_CRL. - Remove the OCSP stapling API, which shipped non-functional and is not implementable on the current transport. - Document the wired features and add cross-backend tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(tls)!: wire ALPN, protocol/cipher selection, PKCS#12, and CRL checking
Wire up TLS features that were declared but not applied, with OpenSSL and WolfSSL parity (fail-closed on WolfSSL builds lacking a capability).