in_forward: Fix incorrect user auth sequence

[cosmo0920]

The previous pingpong handshaking is not enough for failing handshake case.
We need to distinguish where the error happens on pingpong handshaking.
This could be made rigid approach to handle this.

Follows up #11026.

This PR is tested with Fluent Bit (receiver)/Fluent Bit (sender) and Fluent Bit (receiver)/Fluentd (sender) cases.

---

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

Fluent Bit (Server Configs)

[SERVICE]
    Log_Level    debug

[INPUT]
    Name           forward
    Listen         0.0.0.0
    Port           9999
    Empty_Shared_Key     On
    security.users user pass

[OUTPUT]
    Name           stdout
    Match          *

[SERVICE]
    Log_Level    debug

[INPUT]
    Name           forward
    Listen         0.0.0.0
    Port           9999
    Shared_Key     key
    security.users user pass

[OUTPUT]
    Name           stdout
    Match          *

Fluent Bit (Client Configs)

[SERVICE]
    Log_Level    debug

[INPUT]
    Name           dummy

[OUTPUT]
    Name           forward
    Match          *
    Host           127.0.0.1
    Port           9999
    Shared_Key     key
    Username       user
    Password       pass

[SERVICE]
    Log_Level    debug

[INPUT]
    Name           dummy

[OUTPUT]
    Name           forward
    Match          *
    Host           127.0.0.1
    Port           9999
    Shared_Key     key
    Username       user
    Password       pass

Fluentd Sender configs

<system>
  log_level debug
</system>

<source>
  @type dummy
  tag dummy.events
</source>

<match **>
  @type forward
  <server>
    host 127.0.0.1
    port 9999
    username   user
    password   pass
  </server>

  <security>
    shared_key key
    self_hostname fluentd-sender.local
  </security>

  <buffer>
    @type memory
    flush_interval 1s
  </buffer>
</match>

<system>
  log_level debug
</system>

<source>
  @type dummy
  tag dummy.events
</source>

<match **>
  @type forward
  <server>
    host 127.0.0.1
    port 9999
    username   user
    password   pass
  </server>

  <security>
    shared_key ""
    self_hostname fluentd-sender.local
  </security>

  <buffer>
    @type memory
    flush_interval 1s
  </buffer>
</match>

Both of Fluent Bit case

Shared_Key -> OK
Empty_Shared_Key -> OK

Fluent Bit (receiver)/Fluentd (sender) case

Shared_Key -> OK
Empty_Shared_Key -> OK

• Debug log output from testing the change

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

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

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

Documentation

• Documentation required for this feature

Backporting

• Backport to latest stable release.

---

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 handling of authentication failures during the secure forward handshake to avoid treating non-errors as failures.
  • Clarified and enhanced logging when reply messages fail to send, making troubleshooting more straightforward.
  • Ensured proper cleanup and connection termination on handshake failures to prevent resource leaks.

• Refactor

  • Streamlined handshake control flow with clearer success/failure paths for more consistent outcomes.

[coderabbitai]

Walkthrough

The secure-forward handshake in plugins/in_forward/fw_prot.c was refactored to use explicit ping_ret and pong_ret variables, adjust send_pong error handling, document authentication-failure semantics, and ensure consistent cleanup and handshake termination after PING/PONG processing.

Changes

Cohort / File(s)

Summary

Secure forward handshake flow plugins/in_forward/fw_prot.c

- send_pong: removed unconditional error path when userauth is FLB_FALSE; treat write failure as the error and added clarifying comments. - fw_prot_secure_forward_handshake: introduced ping_ret and pong_ret; used ping_ret for check_ping results and pong_ret for send_pong results; refined logging and error branches; ensure resources are destroyed and handshake returns appropriately when authentication fails or on write errors.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Client
    participant S as Server (fw_prot)
    participant P as check_ping
    participant W as send_pong
    participant L as Logger

    C->>S: PING (secure_forward)
    S->>P: validate/authenticate
    P-->>S: ping_ret (>=0 ok, <0 auth fail, -1 error)

    alt ping_ret == -1 (validation error)
        S->>L: log validation error
        S-->>C: Close connection / handshake error
    else ping_ret < 0 (authentication failure)
        S->>W: send PONG (auth-fail signal)
        W-->>S: pong_ret (>=0 ok, -1 write error)
        alt pong_ret == -1 (write failure)
            S->>L: log write failure
            S-->>C: Close connection
        else pong_ret >= 0
            S->>L: note auth failure and terminate handshake (clean up)
            S-->>C: End handshake (no error)
        end
    else ping_ret >= 0 (auth ok)
        S->>W: send PONG (success)
        W-->>S: pong_ret (>=0 ok, -1 write error)
        alt pong_ret == -1 (write failure)
            S->>L: log write failure
            S-->>C: Close connection
        else pong_ret >= 0
            S-->>C: Handshake complete
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• fujimotos
• koleini

Poem

I thump my paw—PING drifts by the fen,
A careful PONG returns from my den.
If auth should fail, I tidy the floor,
Close the tunnel, leave no door.
Logs neat as carrots, handshake done—hop once, then hop more. 🥕

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 (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title concisely identifies the in_forward plugin and clearly states the core fix of correcting the user authentication sequence in the handshake protocol. It directly relates to the changeset’s primary objective and is specific enough for maintainers to understand the main update at a glance.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cosmo0920-fix-forward-protocol-handlings

---

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.}