Skip to content

tls: openssl: preserve syscall error details (5.0) - #12179

Open
edsiper wants to merge 6 commits into
5.0from
ssl-syscall-error-noissue-5.0
Open

tls: openssl: preserve syscall error details (5.0)#12179
edsiper wants to merge 6 commits into
5.0from
ssl-syscall-error-noissue-5.0

Conversation

@edsiper

@edsiper edsiper commented Jul 30, 2026

Copy link
Copy Markdown
Member

Summary

Backport the OpenSSL I/O error-classification fix and deterministic TLS regressions to 5.0.

  • 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 fatal OpenSSL I/O results as non-reusable while preserving a meaningful syscall errno when present
  • preserve 5.0's existing silent clean-close_notify behavior without importing master's separate lifecycle fix
  • exercise TCP RST, corrupted TLS record, and clean TLS shutdown paths

Root cause

The SSL_ERROR_SYSCALL path read errno after flb_errno() and logging operations could replace it with an unrelated value such as ENOTTY. The read path also used a dead ssl_ret < 0 condition; SSL_get_error() returns nonnegative constants, so SSL_ERROR_SSL and other fatal failures were silently discarded and left connection->net_error == -1.

The backport classifies failures as follows:

  • non-empty OpenSSL queue: report the OpenSSL protocol/library error
  • SSL_ERROR_SYSCALL with saved nonzero errno: report and preserve the real syscall failure
  • 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
  • SSL_ERROR_ZERO_RETURN: return terminally without logging, preserving the 5.0 baseline behavior

WANT_READ/WANT_WRITE behavior is unchanged. This branch does not import master's #12079 close-notify recycling fix, #12141's retry work, or any handshake EOF behavior change.

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. A clean close_notify remains silent on 5.0.

Regression proof

The known-broken syscall path emitted the real reset followed by an unrelated Inappropriate ioctl for device; it now reports only Connection reset by peer. A corrupted encrypted record previously reached SSL_ERROR_SSL without a TLS log; it now reports the authoritative bad record mac queue entry.

The initial backport catch-all also reported a clean shutdown as:

[tls] unknown error (ssl_error=6)

An explicit SSL_ERROR_ZERO_RETURN branch removes that regression while preserving the 5.0 baseline. All three 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 \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_close_notify_is_not_an_error -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 \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_close_notify_is_not_an_error -q

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

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

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b6094461-3336-41b7-9ba6-20c461f6af75

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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>
edsiper added 2 commits July 30, 2026 17:39
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@edsiper
edsiper marked this pull request as ready for review July 30, 2026 23:59
@edsiper
edsiper requested a review from cosmo0920 as a code owner July 30, 2026 23:59
@edsiper edsiper added this to the Fluent Bit v5.0.10 milestone Jul 30, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e4b494f8af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/tls/openssl.c
Comment on lines +1490 to +1492
errno = 0;
ret = SSL_read(backend_session->ssl, buf, len);
saved_errno = errno;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Capture the Winsock error for TLS syscalls

On Windows, socket failures are reported through WSAGetLastError() rather than errno, as reflected by the repository's Windows implementations of flb_socket_error() and flb_wsa_get_last_error(). Therefore a reset or timeout from SSL_read() leaves saved_errno at zero, causing SSL_ERROR_SYSCALL to be mislabeled as an unexpected EOF and recorded as generic ECONNRESET; the identical pattern affects SSL_write(). Capture the platform socket error immediately after both OpenSSL calls so Windows retains the actual failure details.

Useful? React with 👍 / 👎.

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