TLS shutdown tests and fixes #44
Conversation
📝 WalkthroughWalkthroughThe changes refine TLS stream read handling in both OpenSSL and WolfSSL implementations to distinguish between proper TLS shutdowns (EOF) and unexpected stream truncations. When SSL_read or WolfSSL operations report EOF, the code now checks if a TLS shutdown was received; if not, the error is downgraded to stream_truncated. Corresponding test helpers and test cases are added to exercise both shutdown and truncation scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
An automated preview of the documentation is available at https://44.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-21 03:17:56 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/wolfssl/src/wolfssl_stream.cpp`:
- Around line 424-439: The code uses SSL_RECEIVED_SHUTDOWN in the
wolfSSL_stream.cpp path that checks wolfSSL_get_shutdown(ssl_) &
SSL_RECEIVED_SHUTDOWN, but that constant may be missing in non-OpenSSL-compat
WolfSSL builds; update the project to handle both cases by either adding a
CMake/FindWolfSSL check to assert OpenSSL-compat mode is present or add a local
fallback macro so compilation succeeds (e.g., detect if SSL_RECEIVED_SHUTDOWN is
undefined and define a sensible fallback) and keep the existing logic that calls
wolfSSL_get_shutdown(ssl_) unchanged; reference SSL_RECEIVED_SHUTDOWN and
wolfSSL_get_shutdown(ssl_) when implementing the check or defining the fallback.
🧹 Nitpick comments (1)
test/unit/tls/test_utils.hpp (1)
466-660: Consider extracting common test phases.Both
run_tls_shutdown_testandrun_tls_truncation_testshare identical handshake and data transfer phases. While some duplication is acceptable in test code for readability, you could consider extracting these common phases into a helper that returns the connected streams, reducing maintenance burden if the setup logic needs to change.♻️ Optional helper extraction
// Helper to set up connected TLS streams after handshake and data transfer template<typename ClientStreamFactory, typename ServerStreamFactory> auto setup_tls_connection( io_context& ioc, context client_ctx, context server_ctx, ClientStreamFactory make_client, ServerStreamFactory make_server) { auto [s1, s2] = corosio::test::make_socket_pair(ioc); auto client = make_client(s1, client_ctx); auto server = make_server(s2, server_ctx); // ... handshake and data transfer phases ... return std::tuple{std::move(s1), std::move(s2), std::move(client), std::move(server)}; }
| if(rec) | ||
| { | ||
| if(rec == make_error_code(capy::error::eof)) | ||
| { | ||
| // Check if we got a proper TLS shutdown | ||
| if(wolfSSL_get_shutdown(ssl_) & SSL_RECEIVED_SHUTDOWN) | ||
| ec = make_error_code(capy::error::eof); | ||
| else | ||
| ec = make_error_code(capy::error::stream_truncated); | ||
| } | ||
| else | ||
| { | ||
| ec = rec; | ||
| } | ||
| goto done; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the wolfssl_stream.cpp file
find . -name "wolfssl_stream.cpp" -type fRepository: cppalliance/corosio
Length of output: 134
🏁 Script executed:
# Check the file size and content around lines 424-439
wc -l $(find . -name "wolfssl_stream.cpp" -type f)Repository: cppalliance/corosio
Length of output: 158
🏁 Script executed:
# Examine the specific lines in question
sed -n '420,445p' $(find . -name "wolfssl_stream.cpp" -type f 2>/dev/null | head -1)Repository: cppalliance/corosio
Length of output: 1370
🏁 Script executed:
# Search for SSL_RECEIVED_SHUTDOWN in WolfSSL headers
find . -name "*.h" -path "*/wolfssl/*" -type f | head -20Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look for SSL_RECEIVED_SHUTDOWN definition
rg "SSL_RECEIVED_SHUTDOWN" --type hRepository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check WolfSSL version and compatibility layer
rg "include.*wolf" $(find . -name "wolfssl_stream.cpp" -type f 2>/dev/null | head -1) | head -20Repository: cppalliance/corosio
Length of output: 195
🏁 Script executed:
# Search for SSL_RECEIVED_SHUTDOWN definition in WolfSSL headers
rg "SSL_RECEIVED_SHUTDOWN" . --type hRepository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check WolfSSL version or configuration
rg "WOLFSSL_VERSION\|OPENSSL_COMPATIBLE" . --type h | head -20Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look for wolfssl headers location
find . -path "*/wolfssl/*.h" -type f | head -20Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check if SSL_RECEIVED_SHUTDOWN is defined in the included headers
cat $(find . -path "*/wolfssl/ssl.h" -type f 2>/dev/null | head -1) | grep -A 2 -B 2 "SSL_RECEIVED_SHUTDOWN" || echo "Not found in ssl.h"Repository: cppalliance/corosio
Length of output: 81
🏁 Script executed:
# Search more broadly for SSL_RECEIVED_SHUTDOWN or similar constants
rg "RECEIVED_SHUTDOWN|SENT_SHUTDOWN" .Repository: cppalliance/corosio
Length of output: 306
🏁 Script executed:
# Check if SSL_RECEIVED_SHUTDOWN is defined anywhere in the codebase
rg "define.*SSL_RECEIVED_SHUTDOWN" .Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look for OpenSSL compatibility mode definitions
rg "OPENSSL_EXTRA\|OPENSSL_ALL\|WOLFSSL_OPENSSL_COMPAT" . --type h | head -20Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check the full context around the SSL_RECEIVED_SHUTDOWN usage in both files
sed -n '415,450p' ./src/wolfssl/src/wolfssl_stream.cppRepository: cppalliance/corosio
Length of output: 1834
🏁 Script executed:
# Check the OpenSSL version for comparison
sed -n '415,450p' ./src/openssl/src/openssl_stream.cppRepository: cppalliance/corosio
Length of output: 1305
🏁 Script executed:
# Look at includes in the test file to understand how this should work
head -50 ./test/unit/tls/wolfssl_stream.cppRepository: cppalliance/corosio
Length of output: 1769
🏁 Script executed:
# Search for any shutdown-related constants in WolfSSL
rg "SHUTDOWN\|shutdown" . --type h | grep -i "define\|enum\|const" | head -30Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check WolfSSL documentation or constants in available headers
find . -path "*/wolfssl/*.h" -type f -exec grep -l "shutdown" {} \;Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look for how WolfSSL defines shutdown status
rg "wolfSSL_get_shutdown\|SSL_get_shutdown" . --type hRepository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check if there's a compatibility shim or definition we need to add
rg "SSL_RECEIVED_SHUTDOWN\|SSL_SENT_SHUTDOWN" . --type cpp | head -20Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look at compiler configuration or CMake to see how WolfSSL is linked
find . -name "CMakeLists.txt" -o -name "*.cmake" | xargs grep -l "wolfssl\|WolfSSL" 2>/dev/null | head -5Repository: cppalliance/corosio
Length of output: 195
🏁 Script executed:
# Check if SSL_RECEIVED_SHUTDOWN might be from OpenSSL headers that WolfSSL redefines
rg "include.*openssl\|include.*wolfssl" ./src/wolfssl/src/wolfssl_stream.cppRepository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check the CMakeLists.txt for WolfSSL configuration
cat ./CMakeLists.txt | grep -A 20 -B 5 "wolfssl\|WolfSSL" | head -50Repository: cppalliance/corosio
Length of output: 2503
🏁 Script executed:
# Check the FindWolfSSL.cmake file
cat ./cmake/FindWolfSSL.cmake | head -100Repository: cppalliance/corosio
Length of output: 957
🏁 Script executed:
# Look for any defines or compiler flags that might define SSL_RECEIVED_SHUTDOWN
rg "SSL_RECEIVED_SHUTDOWN\|OPENSSL_EXTRA\|OPENSSL_ALL" ./CMakeLists.txt ./cmake/Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check if wolfssl/options.h sets any compatibility flags
cat $(find . -name "options.h" -path "*/wolfssl/*" 2>/dev/null | head -1) | head -100Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Search for macro definitions or missing constants related to shutdown
rg "#define.*SHUTDOWN|const.*SHUTDOWN|enum.*SHUTDOWN" . --type cpp | head -20Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look at the complete includes and early definitions in wolfssl_stream.cpp
head -100 ./src/wolfssl/src/wolfssl_stream.cppRepository: cppalliance/corosio
Length of output: 3802
🏁 Script executed:
# Search for any conditional compilation or runtime checks that might define SSL_RECEIVED_SHUTDOWN
rg "#ifdef.*OPENSSL|#if.*OPENSSL" . --type cpp --type h | head -20Repository: cppalliance/corosio
Length of output: 522
🏁 Script executed:
# Check if there's a compatibility header that might define this
find . -name "*.hpp" -o -name "*.h" | xargs grep -l "SSL_RECEIVED_SHUTDOWN" 2>/dev/nullRepository: cppalliance/corosio
Length of output: 47
🏁 Script executed:
# Try a web search to verify SSL_RECEIVED_SHUTDOWN in WolfSSLRepository: cppalliance/corosio
Length of output: 45
🌐 Web query:
SSL_RECEIVED_SHUTDOWN WolfSSL OpenSSL constant definition
💡 Result:
Short answer:
-
OpenSSL: SSL_SENT_SHUTDOWN = 0x01 (value 1), SSL_RECEIVED_SHUTDOWN = 0x02 (value 2); they are shutdown-state bitflags (sent = close_notify sent, received = close_notify received). [1]
-
wolfSSL: wolfSSL_get_shutdown() returns 1 for SSL_SENT_SHUTDOWN and 2 for SSL_RECEIVED_SHUTDOWN (same numeric meanings). [2]
Sources:
[1] OpenSSL manpage SSL_set_shutdown / SSL_shutdown.
[2] wolfSSL documentation (wolfSSL_shutdown / wolfSSL_get_shutdown).
🏁 Script executed:
# Check what constants WolfSSL actually provides for shutdown status
rg "get_shutdown\|wolfSSL_get_shutdown" . --type cpp -B 3 -A 3Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look for any `#define` or constant that might be a substitute
rg "define.*SHUTDOWN|SHUTDOWN.*=" . --type cpp --type h | grep -v "Binary"Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check if SSL_RECEIVED_SHUTDOWN is supposed to come from wolfssl/ssl.h
# Let's verify by looking at what WolfSSL actually defines
rg "SSL_RECEIVED_SHUTDOWN|SSL_SENT_SHUTDOWN" . --type cppRepository: cppalliance/corosio
Length of output: 306
🏁 Script executed:
# Check if it's conditionally defined or if there's a missing include
rg "^\s*#define\s+SSL_RECEIVED_SHUTDOWN|^\s*#define\s+SSL_SENT_SHUTDOWN" . --type cpp --type hRepository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Look at what wolfssl/ssl.h actually provides in the system
# First check if wolfssl/ssl.h from the include path defines these
python3 << 'EOF'
import subprocess
import re
# Search for any file that might define these constants
result = subprocess.run(['rg', '-l', 'SSL_RECEIVED_SHUTDOWN', '.'],
capture_output=True, text=True)
if result.stdout.strip():
for line in result.stdout.strip().split('\n'):
print(f"Found in: {line}")
else:
print("SSL_RECEIVED_SHUTDOWN not found in codebase")
# Check if it appears in comments or documentation
result = subprocess.run(['rg', 'SSL_RECEIVED_SHUTDOWN|SSL_SENT_SHUTDOWN', '.', '--type', 'cpp'],
capture_output=True, text=True)
print("\nAll uses of SSL_*_SHUTDOWN:")
print(result.stdout)
EOFRepository: cppalliance/corosio
Length of output: 149
🏁 Script executed:
# Check OpenSSL stream implementation to see how it's used there
sed -n '410,450p' ./src/openssl/src/openssl_stream.cppRepository: cppalliance/corosio
Length of output: 1484
🏁 Script executed:
# Search in wolfSSL documentation or comments what value to use
rg "0x02\|shutdown.*2\|RECEIVED" ./src/wolfssl/src/wolfssl_stream.cpp -B 2 -A 2Repository: cppalliance/corosio
Length of output: 45
🏁 Script executed:
# Check if there's a compatibility layer or definition we're missing
grep -r "SSL_RECEIVED_SHUTDOWN\|SSL_SENT_SHUTDOWN" include/ src/ 2>/dev/null || echo "Not found in include/src"Repository: cppalliance/corosio
Length of output: 302
SSL_RECEIVED_SHUTDOWN may not be available in all WolfSSL builds.
The code uses SSL_RECEIVED_SHUTDOWN without verifying it's defined. While WolfSSL does support this constant when built with OpenSSL compatibility (--enable-opensslextra or --enable-opensslall), many standard distributions omit these flags. This will cause compilation errors on incompatible WolfSSL installations.
Either:
- Add a CMake check in
FindWolfSSL.cmaketo verify OpenSSL compatibility mode is enabled, or - Provide a fallback definition (e.g.,
#if !defined(SSL_RECEIVED_SHUTDOWN)#defineSSL_RECEIVED_SHUTDOWN 0x02#endif``)
🤖 Prompt for AI Agents
In `@src/wolfssl/src/wolfssl_stream.cpp` around lines 424 - 439, The code uses
SSL_RECEIVED_SHUTDOWN in the wolfSSL_stream.cpp path that checks
wolfSSL_get_shutdown(ssl_) & SSL_RECEIVED_SHUTDOWN, but that constant may be
missing in non-OpenSSL-compat WolfSSL builds; update the project to handle both
cases by either adding a CMake/FindWolfSSL check to assert OpenSSL-compat mode
is present or add a local fallback macro so compilation succeeds (e.g., detect
if SSL_RECEIVED_SHUTDOWN is undefined and define a sensible fallback) and keep
the existing logic that calls wolfSSL_get_shutdown(ssl_) unchanged; reference
SSL_RECEIVED_SHUTDOWN and wolfSSL_get_shutdown(ssl_) when implementing the check
or defining the fallback.
2 participants
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.