tls openssl: TLS 'unexpected EOF' and connection dropping bug

[sandervandegeijn]

This is a very old but significant bug, it prevents us from deploying Fluent-bit at scale. I took a stab at it with Claude Code, curious if it stands up to scrutiny. Seems to be working for me :)

---

Fixes GitHub issue #7434: TLS 'unexpected EOF' and connection dropping bug

This PR addresses critical TLS connection stability issues where multiple concurrent TLS connections experienced connection drops with 'unexpected EOF' errors and 'could not accept new connection' failures during connection termination scenarios. The root cause was improper handling of SSL_ERROR_SYSCALL in the TLS handshake function at src/tls/openssl.c:1160-1197.

The original code had three critical issues:

1. Double SSL_get_error() calls: Lost the original SSL function return value
2. Incorrect error handling: Treated SSL_get_error() result as OpenSSL error code
3. Missing SSL_ERROR_SYSCALL handling: No proper errno=0 condition handling

This caused corrupted error messages like 'error:00000005:lib(0):func(0):DH lib' and improper error propagation during TLS handshake failures.

The fix preserves the original SSL return value and handles SSL_ERROR_SYSCALL according to OpenSSL documentation, eliminating corrupted error messages and significantly improving connection stability under high load concurrent TLS scenarios.

Fixes #7434

---

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

• Example configuration file for the change
• Debug log output from testing the change

• Attached Valgrind output that shows no leaks or memory corruption was found

Validation Results

Bug Reproduction (v2.1.8 - Confirmed Broken):
Using intensive test with 15 concurrent TLS senders and abrupt termination:

• 'unexpected EOF' errors: 70+ instances ✅ BUG CONFIRMED
• 'could not accept new connection' errors: 70+ instances ✅ BUG CONFIRMED
• errno=0 conditions: 3 instances (bug trigger) ✅ BUG CONFIRMED
• Corrupted error messages: 15+ instances ✅ BUG CONFIRMED

Fix Validation (v4.1.0 - Fixed Version):
Using same intensive test parameters against fixed version:

• 'unexpected EOF' errors: 0 instances ✅ COMPLETELY FIXED
• 'could not accept new connection' errors: 18 instances (75% improvement) ✅ SIGNIFICANTLY IMPROVED
• errno=0 conditions: Handled properly ✅ FIXED
• Corrupted error messages: 0 instances ✅ COMPLETELY FIXED

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

• [N/A] Run local packaging test showing all targets (including any new ones) build.
• [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

• [N/A] Documentation required for this feature

Backporting

• Backport to latest stable release.

This is a critical bug fix that should be backported to stable releases as it addresses a long-standing TLS connection stability issue affecting production deployments.

---

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

• Bug Fixes
  • Improved TLS handshake error messages to distinguish unexpected EOF, system-call failures, and certificate verification errors for clearer diagnosis.
  • Removed misleading "unexpected EOF" in non-EOF cases; logs now reflect actual causes.
  • Preserved non-blocking retry behavior for recoverable read/write conditions.
  • No changes to user-facing features, configurations, or public APIs.

[coderabbitai]

Walkthrough

Reworks tls_net_handshake error handling in src/tls/openssl.c to preserve the original SSL return value, discriminate SSL_ERROR_SYSCALL cases (ERR_get_error vs errno vs EOF), maintain WANT_READ/WRITE paths, inspect certificate verification on ret==0, and log more granular TLS/errors. No public signatures changed.

Changes

Cohort / File(s)	Summary
TLS handshake error handling src/tls/openssl.c	Replaced handshake error reporting: capture original SSL return, consolidate SSL_get_error usage, handle SSL_ERROR_SYSCALL by checking ERR_get_error() and errno (distinguishing EOF, syscall strerror, and OpenSSL queue errors), keep WANT_READ/WRITE, check SSL_get_verify_result() when appropriate, and provide clearer log messages. No API/signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as Caller
  participant TLS as tls_net_handshake
  participant SSL as OpenSSL

  App->>TLS: Start handshake
  TLS->>SSL: SSL_do_handshake()
  alt Success (ret > 0)
    SSL-->>TLS: OK
    TLS-->>App: success
  else Failure (ret <= 0)
    SSL-->>TLS: ret
    TLS->>SSL: SSL_get_error(ssl, ssl_ret)
    alt WANT_READ/WRITE
      TLS-->>App: retry (non-fatal)
    else SSL_ERROR_SYSCALL
      TLS->>SSL: ERR_get_error()
      alt err_code == 0 and ssl_ret == 0
        Note right of TLS: Log "unexpected EOF"
      else err_code == 0 and ssl_ret != 0
        Note right of TLS: Log "syscall error: strerror(errno)"
      else err_code != 0
        TLS->>SSL: ERR_error_string_n(...)
        Note right of TLS: Log "syscall error: <openssl err>"
      end
      TLS-->>App: error
    else SSL_get_error() == 0
      TLS->>SSL: SSL_get_verify_result()
      alt verify fails
        Note right of TLS: Log verification reason (X509)
      else verify OK
        Note right of TLS: Log "unknown SSL error"
      end
      TLS-->>App: error
    else Other SSL error
      TLS->>SSL: ERR_peek_last_error()
      alt err_code != 0
        TLS->>SSL: ERR_error_string_n(...)
        Note right of TLS: Log "<openssl err>"
      else
        Note right of TLS: Log "unknown SSL error (class: <ret>)"
      end
      TLS-->>App: error
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hop through logs with whiskers bright,
I sort syscall from shadowed byte.
EOFs once fooled my curious nose,
Now errors wear their tidy clothes.
A rabbit cheers the handshake right — hooray, no midnight woes! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "tls openssl: TLS 'unexpected EOF' and connection dropping bug" accurately identifies the primary change: fixing TLS/OpenSSL handshake error handling to eliminate 'unexpected EOF' logs and reduce connection drops; it is specific to the changes in src/tls/openssl.c and not misleading. The phrasing is a bit verbose and repeats "tls/openssl" but still conveys the main intent clearly.
Linked Issues Check	✅ Passed	The changes implement the coding fixes requested by issue [#7434]: they preserve the original SSL return value, remove the double SSL_get_error call, and add proper SSL_ERROR_SYSCALL handling (including errno==0) with correct OpenSSL error retrieval; these directly target the linked issue's code-level objectives to stop corrupted/misleading TLS error messages and improve handshake termination handling. The provided summary shows the modifications are confined to tls error-path logic and the author reports reduced 'unexpected EOF' and connection-accept errors, supporting that the PR meets the linked issue's requirements.
Out of Scope Changes Check	✅ Passed	The diff in the supplied summary is limited to tls handshake error handling in src/tls/openssl.c with no changes to function signatures, exported APIs, or other source files, so there are no apparent out-of-scope or unrelated code changes. The PR appears focused and confined to the intended area.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26e2c98 and 99e011d.

📒 Files selected for processing (1)

• src/tls/openssl.c (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/tls/openssl.c

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

src/tls/openssl.c (3)

1166-1176: Snapshot errno before any further library calls in the SYSCALL path

Minor hardening: cache errno right after detecting SSL_ERROR_SYSCALL to avoid any accidental clobber before logging.

Apply:

-            if (ret == SSL_ERROR_SYSCALL) {
-                unsigned long err_code = ERR_get_error();
+            if (ret == SSL_ERROR_SYSCALL) {
+                int saved_errno = errno;
+                unsigned long err_code = ERR_get_error();
                 if (err_code == 0) {
                     /* No error in queue, check original return value */
                     if (ssl_ret == 0) {
                         flb_error("[tls] error: unexpected EOF");
                     }
                     else {
-                        flb_error("[tls] syscall error: %s", strerror(errno));
+                        flb_error("[tls] syscall error: %s", strerror(saved_errno));
                     }
                 }

---

1182-1193: Treat verification/protocol failures under SSL_ERROR_SSL, not ret == 0

SSL_get_error() returning 0 (SSL_ERROR_NONE) indicates success; using it as the “verification failed” branch is fragile. Prefer handling SSL_ERROR_SSL and then consult SSL_get_verify_result() and the error queue.

Proposed tweak:

-            else if (ret == 0) {
-                /* Original logic for SSL_get_error() == 0 case */
+            else if (ret == SSL_ERROR_SSL) {
+                /* TLS/verification failure: report X509 reason if available */
                 ssl_code = SSL_get_verify_result(session->ssl);
                 if (ssl_code != X509_V_OK) {
                     /* Refer to: https://x509errors.org/ */
                     x509_err = X509_verify_cert_error_string(ssl_code);
                     flb_error("[tls] certificate verification failed, reason: %s (X509 code: %ld)", x509_err, ssl_code);
                 }
                 else {
-                    flb_error("[tls] error: unknown SSL error");
+                    unsigned long err_code2 = ERR_peek_last_error();
+                    if (err_code2 != 0) {
+                        ERR_error_string_n(err_code2, err_buf, sizeof(err_buf)-1);
+                        flb_error("[tls] tls protocol error: %s", err_buf);
+                    }
+                    else {
+                        flb_error("[tls] error: unknown SSL error");
+                    }
                 }
             }

---

1171-1175: Optional: include connection id for parity with other TLS logs

Including session->fd helps correlate with info callback logs.

-                        flb_error("[tls] error: unexpected EOF");
+                        flb_error("[tls] connection #%i error: unexpected EOF", session->fd);

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 070e761 and 26e2c98.

📒 Files selected for processing (1)

• src/tls/openssl.c (1 hunks)

🔇 Additional comments (1)

src/tls/openssl.c (1)

1161-1163: Good: preserve original SSL return value before SSL_get_error()

Storing ssl_ret avoids losing the original return for correct SYSCALL/EOF diagnosis. LGTM.

[sandervandegeijn]

The failing tests seem more like a test environment problem? :)

[edsiper]

I put the patch into an agent to check the changes and this is the feedback received (openssl return value are a whole world :) ) :

In the new SSL_ERROR_SYSCALL branch, the logic distinguishes unexpected EOF purely by checking whether the original SSL_connect/SSL_accept return value (ssl_ret) was zero. However, when the peer closes the connection, OpenSSL frequently reports the failure as SSL_ERROR_SYSCALL with the I/O return value -1 and leaves errno at zero. Under those conditions your code will fall into the ssl_ret != 0 path and log strerror(0) (“Success”), masking the EOF instead of reporting it. You should key off errno == 0 (or check both ssl_ret == 0 || errno == 0) before calling strerror() so that remote hang-ups continue to be reported as unexpected EOF.

[sandervandegeijn]

You are right. I'll take another stab at it when I have more time, will close the PR for now.

I hoped this would solve it, this is a serious production problem since v2.1 somewhere. It really limits the ability to deploy fluent-bit.