Skip to content

fix(tls): return OK not BufferOverflow when wrap is handshake-blocked - #66

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-tls-return-ok-not-bufferoverflow-when-wrap-is-dd40de
Open

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-tls-return-ok-not-bufferoverflow-when-wrap-is-dd40de

Conversation

@detail-app

@detail-app detail-app Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Change

Engine::wrap in crates/bemo/src/tls/engine.rs mislabeled a handshake-blocked 0/0 wrap as Status::BufferOverflow instead of Status::Ok. When the handshake is blocked waiting for peer input (Handshake::NeedUnwrap with no pending outbound records), calling wrap with non-empty application data produced consumed=0/produced=0 and the status classification took the BufferOverflow branch because sink.consumed < sink.source.len(). This violates the SSLEngine contract (OpenJDK returns OK/0/0/NEED_UNWRAP in this state) and causes Netty's SslHandler to retry BUFFER_OVERFLOW/0/0 in a loop, growing its output buffer until ensureWritable throws — hanging or failing the write path (DoS).

Fix: Added a condition in the wrap status classification that returns Status::Ok when 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 existing BufferOverflow branch 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

  • Rust unit tests (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 the NeedUnwrap state for both client/server roles and both TLS 1.3/1.2, asserting Status::Ok with 0/0 and verifying the handshake still completes and post-handshake app data flows correctly.
  • Rust integration tests (tls.rs, engine.rs, tls_abi.rs): 23/23 passed.
  • Full Rust test suite (cargo nextest run -p bemo --locked): 382/384 passed; 2 pre-existing failures in driver/http modules confirmed on baseline (unrelated to this change).
  • Epoll backend (ELIDE_TRANSPORT_TEST_BACKEND=epoll): 384/384 passed.
  • Miri (make test-miri): passed.
  • ASAN/TSAN (make test-asan, make test-tsan): no sanitizer violations; same 2 pre-existing test failures.
  • Fuzz (tls target, 1000 runs): no crashes or failures; lock files unchanged.
  • JVM tests (make test-jvm): all contracts passed, including NativeSslEngineTest (SslHandler over native sockets and NIO), NativeSslInteropTest (JSSE and OpenSSL interop), and NativeSslPolicyTest.
  • Native Image (make test-native-image): CapiContract, ReusableGzipTest, and CapiTlsChannelTest (all transport TLS contracts) passed as native executables.
  • Formatting/lint/build: cargo fmt, cargo clippy --workspace --all-targets --locked -- -D warnings, cargo build --workspace --locked, cargo doc, and google-java-format all clean.
  • Java static analysis (check_java.py — Error Prone, NullAway, Checker Framework): passed.
  • Coverage (Rust cargo-llvm-cov + JVM JaCoCo): reports generated.

Automatic Fixes PRs can be configured here.

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.84%. Comparing base (26fb123) to head (710188e).
✅ All tests successful. No failed tests found.

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     
Flag Coverage Δ
jvm 57.47% <ø> (-0.04%) ⬇️
rust 93.03% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@codspeed

codspeed Bot commented Oct 17, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 10.46%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ 5 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 112 untouched benchmarks
⏩ 65 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

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)

Open in CodSpeed

Footnotes

  1. 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. ↩

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Detail Bug] Netty TLS writes can hang/fail when app data is flushed before handshake completes

1 participant