Skip to content

out_forward: process type and length of pong strictly#11945

Merged
edsiper merged 2 commits into
masterfrom
cosmo0920-process-length-of-pong
Jun 15, 2026
Merged

out_forward: process type and length of pong strictly#11945
edsiper merged 2 commits into
masterfrom
cosmo0920-process-length-of-pong

Conversation

@cosmo0920

@cosmo0920 cosmo0920 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

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

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

Release Notes

  • Bug Fixes

    • Improved validation of forward plugin authentication data with stricter type checking
    • Enhanced error handling with improved bounds checking and message processing
    • Added validation checks for handshake protocol exchanges
  • Tests

    • Added test coverage for forward plugin response handling

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4294bae0-06ac-45fa-8ab7-66488a1ff07f

📥 Commits

Reviewing files that changed from the base of the PR and between 47db630 and 444af18.

📒 Files selected for processing (2)
  • plugins/out_forward/forward.c
  • tests/runtime/out_forward.c

📝 Walkthrough

Walkthrough

Hardened msgpack parsing across the secure forward handshake in plugins/out_forward/forward.c: adds type/shape checks for HELO (array, exact token, map options), PING (string/bin nonce/auth), PONG (exact size+content, bounded error copy), and ACK (string value type, corrected log format). A new runtime test in tests/runtime/out_forward.c exercises the hardened path by simulating a server that sends an oversized PONG response.

Changes

Secure Forward Handshake Hardening

Layer / File(s) Summary
msgpack type/shape validation in handshake and ACK
plugins/out_forward/forward.c
secure_forward_set_ping validates the options object is a map and skips non-string keys, accepting nonce/auth only when their types are string or bin. secure_forward_pong requires exact size+content match for "PONG" and uses bounded memcpy with explicit null-termination for error strings. secure_forward_handshake validates the HELO root is an array, the token string size equals 4, and options is a map. forward_read_ack rejects non-string ack values and updates mismatch log formatting to %zu.
Oversized PONG runtime test harness and entrypoint
tests/runtime/out_forward.c
Adds (non-Windows) server struct, TCP loopback helpers, msgpack HELO/oversized-PONG builders, a read/unpack helper, and a server thread that performs the full handshake and records ping receipt. flb_test_secure_forward_oversized_pong_reason starts the thread, configures a forward output with shared_key, runs Fluent Bit briefly, joins the thread, and asserts got_ping == FLB_TRUE. Registered in TEST_LIST under #ifndef FLB_SYSTEM_WINDOWS.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • fluent/fluent-bit#11028: Modifies the in_forward plugin's secure forward handshake and pong handling (fw_prot_secure_forward_handshake, send_pong), which mirrors the handshake/pong logic being hardened in out_forward/forward.c here.

Suggested reviewers

  • edsiper

Poem

🐇 Hop hop, the handshake must be true,
A map, a string, a size of four or two!
No oversized PONG shall slip on past,
These bounded checks are built to last.
The rabbit guards each msgpack byte,
And tests confirm the parse is right! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 accurately captures the main change: stricter type and length validation for pong message processing in the out_forward plugin.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 cosmo0920-process-length-of-pong

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 and usage tips.

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.

2 participants