out_forward: align secure forward handshake and chunk ack with protocol spec

[edsiper]

This PR aligns out_forward with the Fluentd Forward protocol specification (v1 / v1.5), based on a review of the local Forward implementation against the spec documents.

A few details of the out_forward implementation diverged from what the spec describes: the client did not check the server_hostname and shared_key_hexdigest fields that servers return in the PONG message, username/password settings were silently unused when no shared_key was configured, the chunk ack token was sent as a hex string although the spec defines it as a Base64 representation of a 128-bit unique id, and the formatter had two small robustness bugs (a failed metadata transcode still returned success, and a debug log could print an uninitialized buffer).

---

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

• New Features

  • Added secure forward handshake support for authenticated sender/receiver communication.
  • Forward acknowledgments now use a compact Base64 chunk token.

• Bug Fixes

  • Improved handshake validation to reject invalid, missing, or malformed server responses.
  • Added stricter configuration checks and clearer handling for payload formatting failures.
  • Fixed transcode failure handling so failed sends retry correctly.

• Tests

  • Added integration and runtime coverage for successful and failing secure-forward handshake flows.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR reworks the secure forward handshake in out_forward to use a generalized shared-key digest incorporating hostname and nonce, tightens PONG validation, adds config checks, switches the ACK "chunk" token from hex to Base64, and adds runtime/integration test coverage.

Changes

Secure forward handshake and chunk token changes

Layer / File(s)	Summary
Generalized shared-key digest helper plugins/out_forward/forward.c	Adds secure_forward_hash_key_digest parameterized by hostname/nonce and rewires the existing digest helper to use it.
PING/PONG validation rework plugins/out_forward/forward.c	secure_forward_ping now takes a parsed flb_forward_ping struct; secure_forward_pong requires config/nonce and enforces exact 5-field format; mutual authentication digest is recomputed and compared with detailed error handling.
Handshake nonce persistence and wiring plugins/out_forward/forward.c	Introduces a dedicated nonce buffer, parses HELO via secure_forward_set_ping, validates/copies nonce bytes, and threads the nonce through PING and PONG calls.
Configuration validation and flush error handling plugins/out_forward/forward.c	Checks config_set_properties() return values in both config loaders, rejects username/password without a shared key, and handles formatter failures during flush with cleanup and FLB_RETRY.
Chunk ack token: hex to Base64 plugins/out_forward/forward.h, plugins/out_forward/forward_format.c	Replaces checksum_hex[33] with chunk_token[FLB_FORWARD_CHUNK_TOKEN_SIZE], and updates message/forward/forward-compat modes to Base64-encode a 16-byte SHA512-derived token.
Runtime tests for secure handshake and chunk validation tests/runtime/out_forward.c	Extends the mock server with PONG mode variations, adds out_len reporting to msgpack reading, updates chunk assertions to Base64 form, and adds four new handshake test cases.
Integration test harness for secure forward tests/integration/scenarios/out_forward/*, tests/integration/src/server/forward_server.py	Adds a secure-sender YAML config, a Python secure handshake implementation in the test forward server, and a SecureForwardChain harness with positive/negative handshake tests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• fluent/fluent-bit#11945: Both PRs modify plugins/out_forward/forward.c's secure forward handshake flow, tightening secure_forward_pong/secure_forward_handshake validation.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server
  Client->>Server: HELO
  Server-->>Client: HELO options (nonce)
  Client->>Server: PING (hostname, nonce, digest)
  Server->>Server: verify client digest via secure_forward_ping
  Server-->>Client: PONG (server_hostname, server digest)
  Client->>Client: compute expected digest via secure_forward_hash_key_digest
  Client->>Client: compare expected vs received digest, accept/reject

Loading

Suggested labels: backport to v4.2.x

Suggested reviewers: cosmo0920

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main changes: secure forward handshake validation and chunk acknowledgement formatting updates.
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.

✨ 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 forward-enhancements

---

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

[coderabbitai]

🧹 Nitpick comments (3)

tests/integration/src/server/forward_server.py (1)

316-326: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Server never rejects on invalid client digest.

client_digest_valid is computed and recorded (Line 318) but the PONG always sends auth_result=True (Line 326), regardless of that value. This means there's no test coverage for the server-side rejection path (client presents wrong shared key). Consider wiring auth_result/reason to client_digest_valid so future tests can exercise that scenario too.

♻️ Proposed enhancement

     expected_client_digest = _sha512_hex(shared_key_salt, client_hostname,
                                          nonce, shared_key)
+    client_digest_valid = client_digest == expected_client_digest
     data_storage["handshakes"].append({
         "client_hostname": client_hostname,
-        "client_digest_valid": client_digest == expected_client_digest,
+        "client_digest_valid": client_digest_valid,
     })

     server_digest = _sha512_hex(shared_key_salt, self_hostname, nonce, shared_key)
     if server_options["corrupt_pong_digest"]:
         first = "1" if server_digest[0] == "0" else "0"
         server_digest = first + server_digest[1:]

-    conn.sendall(_pack_obj(["PONG", True, "", self_hostname, server_digest]))
+    conn.sendall(_pack_obj([
+        "PONG",
+        client_digest_valid,
+        "" if client_digest_valid else "shared key mismatch",
+        self_hostname,
+        server_digest,
+    ]))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/src/server/forward_server.py` around lines 316 - 326, The
forward_server handshake currently records client_digest_valid in data_storage
but still always sends PONG with auth_result set to true, so the
invalid-client-digest path is never exercised. Update the handshake logic in the
server connection flow to derive the PONG auth_result and reason from
client_digest_valid, using the existing client_digest_valid/server_digest logic
in forward_server.py so invalid shared-key clients are rejected and can be
covered by tests.

tests/integration/scenarios/out_forward/tests/test_out_forward_secure_001.py (1)

23-58: 🚀 Performance & Scalability | 🔵 Trivial

Duplicated msgpack packing helpers across test-support files.

_pack_uint/_pack_str/_pack_obj here largely re-implement logic already present in tests/integration/src/server/forward_server.py. Could be consolidated into a shared test utility module to avoid drift between the two encoders.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/scenarios/out_forward/tests/test_out_forward_secure_001.py`
around lines 23 - 58, The msgpack packing helpers in this test file duplicate
the encoder logic already implemented in forward_server.py, so consolidate them
into a shared test utility module and have both places import the same
_pack_uint, _pack_str, and _pack_obj behavior. Update the test helper to
reference the shared encoder symbols instead of maintaining a separate copy,
keeping the existing packing API intact while removing the duplicate
implementation.

plugins/out_forward/forward_format.c (1)

457-466: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

append_options return value is ignored.

append_options can return -1 (hash/base64 failure) before it calls flb_mp_map_header_end, leaving the msgpack map header unterminated. Both call sites here ignore the return and still emit out_buf with return 0, so a malformed payload could be sent. The new transcode-failure handling above sets a good precedent; consider propagating append_options failures the same way.

♻️ Proposed handling

             entries = flb_mp_count(transcoded_buffer, transcoded_length);
-            append_options(ctx, fc, event_type, &mp_pck, entries,
-                           transcoded_buffer,
-                           transcoded_length,
-                           NULL, chunk);
-
-            free(transcoded_buffer);
+            result = append_options(ctx, fc, event_type, &mp_pck, entries,
+                                    transcoded_buffer,
+                                    transcoded_length,
+                                    NULL, chunk);
+            free(transcoded_buffer);
+            if (result != 0) {
+                msgpack_sbuffer_destroy(&mp_sbuf);
+                return -1;
+            }
         }
         else {
-            append_options(ctx, fc, event_type, &mp_pck, entries, (char *) data, bytes, NULL, chunk);
+            if (append_options(ctx, fc, event_type, &mp_pck, entries,
+                               (char *) data, bytes, NULL, chunk) != 0) {
+                msgpack_sbuffer_destroy(&mp_sbuf);
+                return -1;
+            }
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/out_forward/forward_format.c` around lines 457 - 466, The
append_options call in forward_format.c is not checked, so failures from
hash/base64 encoding can still leave the msgpack map incomplete and the function
incorrectly returns success. Update both call sites in the transcode and
non-transcode paths to capture the return from append_options and propagate a
failure the same way the transcode-failure handling does, ensuring the out_buf
payload is only emitted after a successful append_options and completed msgpack
header.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@plugins/out_forward/forward_format.c`:
- Around line 457-466: The append_options call in forward_format.c is not
checked, so failures from hash/base64 encoding can still leave the msgpack map
incomplete and the function incorrectly returns success. Update both call sites
in the transcode and non-transcode paths to capture the return from
append_options and propagate a failure the same way the transcode-failure
handling does, ensuring the out_buf payload is only emitted after a successful
append_options and completed msgpack header.

In
`@tests/integration/scenarios/out_forward/tests/test_out_forward_secure_001.py`:
- Around line 23-58: The msgpack packing helpers in this test file duplicate the
encoder logic already implemented in forward_server.py, so consolidate them into
a shared test utility module and have both places import the same _pack_uint,
_pack_str, and _pack_obj behavior. Update the test helper to reference the
shared encoder symbols instead of maintaining a separate copy, keeping the
existing packing API intact while removing the duplicate implementation.

In `@tests/integration/src/server/forward_server.py`:
- Around line 316-326: The forward_server handshake currently records
client_digest_valid in data_storage but still always sends PONG with auth_result
set to true, so the invalid-client-digest path is never exercised. Update the
handshake logic in the server connection flow to derive the PONG auth_result and
reason from client_digest_valid, using the existing
client_digest_valid/server_digest logic in forward_server.py so invalid
shared-key clients are rejected and can be covered by tests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e238ed8f-303e-4bdc-b52a-735b7c576d1d

📥 Commits

Reviewing files that changed from the base of the PR and between b0fdb50 and 68850b2.

📒 Files selected for processing (7)

• plugins/out_forward/forward.c
• plugins/out_forward/forward.h
• plugins/out_forward/forward_format.c
• tests/integration/scenarios/out_forward/config/out_forward_secure_sender.yaml
• tests/integration/scenarios/out_forward/tests/test_out_forward_secure_001.py
• tests/integration/src/server/forward_server.py
• tests/runtime/out_forward.c

[cosmo0920]

Looks good to me! 👍