Repository navigation
fix(tls): return OK not BufferOverflow when wrap is handshake-blocked - #66
detail-app[bot] wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
============================================
+ Coverage 86.82% 86.84% +0.01%
+ Complexity 616 615 -1
============================================
Files 60 60
Lines 16979 17027 +48
Branches 490 490
============================================
+ Hits 14742 14787 +45
- Misses 1913 1917 +4
+ Partials 324 323 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will degrade performance by 10.46%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | retain-drop[1048576] |
300.8 ns | 354.9 ns | -15.26% |
| ❌ | Simulation | retain-drop[4096] |
300.8 ns | 354.9 ns | -15.26% |
| ❌ | Simulation | retain-drop[64] |
300.8 ns | 354.9 ns | -15.26% |
| ❌ | Simulation | retain-drop[65536] |
300.8 ns | 354.9 ns | -15.26% |
| ⚡ | Simulation | overwrite[64] |
520.2 ns | 466.1 ns | +11.62% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing detail/bug-fix/fix-tls-return-ok-not-bufferoverflow-when-wrap-is-dd40de (710188e) with main (26fb123)
Footnotes
-
65 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Detail bug report: View on Detail
Change
Engine::wrapincrates/bemo/src/tls/engine.rsmislabeled a handshake-blocked 0/0 wrap asStatus::BufferOverflowinstead ofStatus::Ok. When the handshake is blocked waiting for peer input (Handshake::NeedUnwrapwith no pending outbound records), callingwrapwith non-empty application data producedconsumed=0/produced=0and the status classification took theBufferOverflowbranch becausesink.consumed < sink.source.len(). This violates theSSLEnginecontract (OpenJDK returnsOK/0/0/NEED_UNWRAPin this state) and causes Netty'sSslHandlerto retryBUFFER_OVERFLOW/0/0 in a loop, growing its output buffer untilensureWritablethrows — hanging or failing the write path (DoS).Fix: Added a condition in the
wrapstatus classification that returnsStatus::Okwhen the engine is handshaking, produced nothing, and has no pending output — i.e., the handshake is blocked on peer input and no larger destination could help. The existingBufferOverflowbranch is preserved for genuine destination-capacity issues (pending output that didn't fit, or post-handshake app data that didn't fit). Affects the Rust TLS engine layer (crates/bemo); no JVM, ABI, or header changes.Closes #59
Validation
cargo nextest run -p bemo --locked -E "test(/tls::engine::tests::/)"): 12/12 passed, including a new regression test that wraps non-empty app data in theNeedUnwrapstate for both client/server roles and both TLS 1.3/1.2, assertingStatus::Okwith 0/0 and verifying the handshake still completes and post-handshake app data flows correctly.tls.rs,engine.rs,tls_abi.rs): 23/23 passed.cargo nextest run -p bemo --locked): 382/384 passed; 2 pre-existing failures indriver/httpmodules confirmed on baseline (unrelated to this change).ELIDE_TRANSPORT_TEST_BACKEND=epoll): 384/384 passed.make test-miri): passed.make test-asan,make test-tsan): no sanitizer violations; same 2 pre-existing test failures.tlstarget, 1000 runs): no crashes or failures; lock files unchanged.make test-jvm): all contracts passed, includingNativeSslEngineTest(SslHandler over native sockets and NIO),NativeSslInteropTest(JSSE and OpenSSL interop), andNativeSslPolicyTest.make test-native-image):CapiContract,ReusableGzipTest, andCapiTlsChannelTest(all transport TLS contracts) passed as native executables.cargo fmt,cargo clippy --workspace --all-targets --locked -- -D warnings,cargo build --workspace --locked,cargo doc, andgoogle-java-formatall clean.check_java.py— Error Prone, NullAway, Checker Framework): passed.cargo-llvm-cov+ JVM JaCoCo): reports generated.Automatic Fixes PRs can be configured here.