Skip to content

tls: openssl: preserve syscall error details - #12178

Open
edsiper wants to merge 4 commits into
masterfrom
ssl-syscall-error-noissue
Open

tls: openssl: preserve syscall error details#12178
edsiper wants to merge 4 commits into
masterfrom
ssl-syscall-error-noissue

Conversation

@edsiper

@edsiper edsiper commented Jul 30, 2026

Copy link
Copy Markdown
Member

Summary

  • preserve errno immediately after SSL_read() and SSL_write()
  • distinguish real syscall failures, legacy unexpected peer EOF, and authoritative OpenSSL protocol/library errors
  • report fatal SSL_ERROR_SSL failures instead of silently discarding the error queue
  • mark every fatal OpenSSL I/O result as non-reusable while preserving a meaningful syscall errno when present
  • add deterministic real-TLS regressions for TCP RST/SSL_ERROR_SYSCALL and corrupted-record/SSL_ERROR_SSL paths

Root cause

The SSL_ERROR_SYSCALL read path called flb_errno() before using errno again. Fluent Bit's error rendering calls isatty(stderr); with redirected stderr that call can replace the original value with ENOTTY. The following strerror(errno) and connection->net_error assignment therefore reported and stored the diagnostic side effect. The write path had the same delayed-errno pattern.

The read path also had a dead ssl_ret < 0 condition. SSL_get_error() returns nonnegative constants, so SSL_ERROR_SSL and other fatal results fell through silently and left connection->net_error == -1. On OpenSSL 3.x this included unexpected peer EOF, which is reported as SSL_ERROR_SSL with SSL_R_UNEXPECTED_EOF_WHILE_READING in the queue.

The fix snapshots errno immediately after OpenSSL I/O and classifies failures as follows:

  • non-empty OpenSSL queue: report the OpenSSL protocol/library error
  • SSL_ERROR_SYSCALL with saved nonzero errno: report the real syscall failure and preserve that errno
  • SSL_ERROR_SYSCALL with an empty queue and zero errno: report legacy unexpected EOF and use ECONNRESET as the terminal marker
  • other fatal OpenSSL results: report the queued error, or the numeric SSL result if the queue is unexpectedly empty, and prevent reuse

WANT_READ/WANT_WRITE and the existing SSL_ERROR_ZERO_RETURN close-notify handling remain unchanged. This does not include #12141's retry work or alter handshake EOF behavior.

Compatibility and severity

An established-session peer EOF without close_notify is logged at error level. OpenSSL 3.x classifies this as a fatal TLS truncation/protocol error, and older OpenSSL versions already reached the error-level SSL_ERROR_SYSCALL path. This may make abrupt disconnects from non-compliant clients more visible, but Fluent Bit does not enable SSL_OP_IGNORE_UNEXPECTED_EOF because that would discard TLS truncation protection. An abrupt EOF during an incomplete SSL_write() is also error-level because application data may have been lost. Ordinary handshake-disconnect behavior is untouched.

Regression proof

The known-broken syscall path emitted contradictory diagnostics:

[openssl.c:1594 errno=104] Connection reset by peer
[tls] syscall error: Inappropriate ioctl for device

It now reports only the saved reset error. Before the fatal-error correction, a corrupted encrypted record reached SSL_ERROR_SSL but produced no TLS log. It now reports the authoritative queue entry, such as decryption failed or bad record mac.

Both tests submit a valid TLS Forward payload afterward and verify responsiveness.

Validation

cmake --build build -j8

tests/integration/.venv/bin/python -m pytest \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_syscall_error_preserves_errno \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_protocol_error_is_reported -q

VALGRIND=1 VALGRIND_STRICT=1 \
tests/integration/.venv/bin/python -m pytest \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_syscall_error_preserves_errno \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_protocol_error_is_reported -q

Both normal and strict-Valgrind tests pass. Valgrind reports 0 errors, all heap blocks freed, and no allocations remaining at exit. The full commit range passes the repository prefix checker against master.

edsiper added 2 commits July 30, 2026 17:06
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

TLS OpenSSL read and write paths now centralize error logging and preserve syscall errno values. Forward integration tests inject connection resets and corrupted TLS records, verify diagnostic output, and confirm subsequent valid traffic remains accepted.

Changes

TLS I/O error handling

Layer / File(s) Summary
Centralize TLS read and write error handling
src/tls/openssl.c
Adds shared reporting for OpenSSL, syscall, EOF, and unknown errors while preserving errno or assigning ECONNRESET.
Validate reset and protocol-error behavior
tests/integration/scenarios/in_forward/tests/test_in_forward_001.py
Adds TLS fault-injection helpers and tests for errno preservation, protocol logs, and continued ingestion.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ForwardTest
  participant TLSListener
  participant tls_net_read
  participant tls_log_io_error
  ForwardTest->>TLSListener: send reset or corrupted TLS record
  TLSListener->>tls_net_read: read TLS data
  tls_net_read->>tls_log_io_error: report TLS I/O error
  tls_log_io_error-->>TLSListener: emit diagnostic and errno result
  ForwardTest->>TLSListener: send subsequent valid traffic
  TLSListener-->>ForwardTest: accept and ingest traffic
Loading

Possibly related PRs

Suggested labels: backport to v5.0.x

Suggested reviewers: cosmo0920

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preserving syscall error details in OpenSSL TLS handling.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ssl-syscall-error-noissue

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

edsiper added 2 commits July 30, 2026 17:28
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant