bfdd: harden packet validation and reflector handling (backport #21105)

[Jafaral]

No description provided.

[Jafaral]

@greptile review backport

[Jafaral]

@Mergifyio backport stable/10.5 stable/10.4 stable/10.3 stable/10.2

[mergify]

backport stable/10.5 stable/10.4 stable/10.3 stable/10.2

✅ Backports have been created

Details

• #21272 bfdd: harden packet validation and reflector handling (backport #21105) (backport #21255) has been created for branch stable/10.5 but encountered conflicts

Cherry-pick of aa52430 has failed:

On branch mergify/bp/stable/10.5/pr-21255
Your branch is up to date with 'origin/stable/10.5'.

You are currently cherry-picking commit aa524306a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of f508822 has failed:

On branch mergify/bp/stable/10.5/pr-21255
Your branch is ahead of 'origin/stable/10.5' by 4 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit f5088225a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of d2645ab has failed:

On branch mergify/bp/stable/10.5/pr-21255
Your branch is ahead of 'origin/stable/10.5' by 5 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit d2645ab31.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #21273 bfdd: harden packet validation and reflector handling (backport #21105) (backport #21255) has been created for branch stable/10.4 but encountered conflicts

Cherry-pick of aa52430 has failed:

On branch mergify/bp/stable/10.4/pr-21255
Your branch is up to date with 'origin/stable/10.4'.

You are currently cherry-picking commit aa524306a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of f508822 has failed:

On branch mergify/bp/stable/10.4/pr-21255
Your branch is ahead of 'origin/stable/10.4' by 4 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit f5088225a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of d2645ab has failed:

On branch mergify/bp/stable/10.4/pr-21255
Your branch is ahead of 'origin/stable/10.4' by 5 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit d2645ab31.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #21274 bfdd: harden packet validation and reflector handling (backport #21105) (backport #21255) has been created for branch stable/10.3 but encountered conflicts

Cherry-pick of aa52430 has failed:

On branch mergify/bp/stable/10.3/pr-21255
Your branch is up to date with 'origin/stable/10.3'.

You are currently cherry-picking commit aa524306a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of f508822 has failed:

On branch mergify/bp/stable/10.3/pr-21255
Your branch is ahead of 'origin/stable/10.3' by 4 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit f5088225a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of d2645ab has failed:

On branch mergify/bp/stable/10.3/pr-21255
Your branch is ahead of 'origin/stable/10.3' by 5 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit d2645ab31.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #21275 bfdd: harden packet validation and reflector handling (backport #21105) (backport #21255) has been created for branch stable/10.2 but encountered conflicts

Cherry-pick of aa52430 has failed:

On branch mergify/bp/stable/10.2/pr-21255
Your branch is up to date with 'origin/stable/10.2'.

You are currently cherry-picking commit aa524306a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of f508822 has failed:

On branch mergify/bp/stable/10.2/pr-21255
Your branch is ahead of 'origin/stable/10.2' by 4 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit f5088225a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of d2645ab has failed:

On branch mergify/bp/stable/10.2/pr-21255
Your branch is ahead of 'origin/stable/10.2' by 5 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit d2645ab31.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   bfdd/bfd_packet.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[greptile-apps]

Greptile Summary

This PR backports security and correctness hardening for BFD packet handling from #21105 into stable/10.6. It addresses several independent issues across bfdd/bfd_packet.c.

Key changes:

• Critical errno fix (bfd_recv_ipv4_fp): The original condition errno != EAGAIN || errno != EWOULDBLOCK || errno != EINTR was always true (by De Morgan's law, a value cannot simultaneously equal all three), causing error messages to log even for expected non-blocking errors. Changed to && — this is the most impactful correctness fix in the PR.
• Consistent non-blocking errno handling: bfd_recv_ipv4 and bfd_recv_ipv6 now also suppress log noise for EWOULDBLOCK and EINTR, matching the fixed bfd_recv_ipv4_fp pattern.
• Echo socket reflector hardening: Before reflecting an IPv6 BFD echo packet, the code now validates the source against a known BFD session (bfd_key_lookup), preventing the echo socket from acting as an unauthenticated packet reflector. Echo packet length (bep->len) is also validated and used as the sendto size instead of the raw rlen.
• SBFD reflector init hardening: ptm_bfd_reflector_process_init_packet now validates BFD version and packet-declared length, and uses cp->len instead of rlen for sendto, preventing the reflector from echoing excess trailing bytes.
• rlen < 0 early returns: Added in bp_bfd_echo_in, bfd_recv_cb, and ptm_bfd_reflector_process_init_packet so receive errors are handled before any buffer reads.
• bfd_check_auth refactoring: Logic inverted for clarity; direct auth->type switch (removing spurious ntohs() on a uint8_t field).
• Session lookup reordered in bfd_recv_cb: ptm_bfd_sess_find now runs after the minimum-size check but before the full RFC 5880 §6.8.6 validation, presumably to allow per-session bad-packet accounting in the future.

Confidence Score: 4/5

• This PR is safe to merge; it is a well-scoped security/correctness backport with no regressions introduced.
• All changes are targeted bug fixes and defensive hardening. The errno logic fix is unambiguously correct. The reflector and echo-length guards are straightforward security improvements. The one noteworthy functional change — session lookup before full packet validation — is intentional but undocumented, and a minor comment typo remains; neither prevents merging.
• The reordered session-lookup section in bfd_recv_cb (around line 933) warrants a brief comment explaining why the lookup was moved earlier relative to TTL and RFC 5880 §6.8.6 checks.

Important Files Changed

Filename	Overview
bfdd/bfd_packet.c	Security and correctness hardening: fixes a critical always-true errno condition (

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([bfd_recv_cb]) --> B{mlen < 0?}
    B -- yes --> Z1([return])
    B -- no --> C{mlen < BFD_PKT_LEN?}
    C -- yes --> Z2([return])
    C -- no --> D["Parse cp = (bfd_pkt*)msgbuf"]
    D --> E[ptm_bfd_sess_find]
    E --> F{bfd == NULL?}
    F -- yes --> Z3([return: no session])
    F -- no --> G{TTL valid?}
    G -- no --> Z4([return])
    G -- yes --> H{Version / detect_mult / len / M-bit / my_discr valid?}
    H -- no --> Z5([return])
    H -- yes --> I{VRF match?}
    I -- no --> Z6([return])
    I -- yes --> J[bfd_check_auth]
    J --> K{Auth OK?}
    K -- no --> Z7([return])
    K -- yes --> L[ptm_bfd_snd / state machine]

    subgraph "Echo IPv6 Reflector path (bp_bfd_echo_in)"
        M([bp_bfd_echo_in]) --> N{rlen < 0?}
        N -- yes --> Z8([return -1])
        N -- no --> O{rlen < bfd_offset+sizeof bep?}
        O -- yes --> Z9([return -1])
        O -- no --> P["bep = msgbuf + bfd_offset"]
        P --> Q{TTL==255 && bg_echov6?}
        Q -- yes --> R[bfd_key_lookup]
        R --> S{Session found?}
        S -- no --> Z10([return -1: drop unknown source])
        S -- yes --> T{bep->len valid?}
        T -- no --> Z11([return -1])
        T -- yes --> U["bp_udp_send (bep->len bytes)"]
        Q -- no --> V[Read my_discr / process echo]
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: bfdd/bfd_packet.c
Line: 841-853

Comment:
**Typo in comments ("finshed")**

Three of the four `switch` case comments still use the original misspelling `"To be finshed."` while the `BFD_AUTH_NULL` case was corrected to `"To be finished."` — a good opportunity to fix the remaining ones for consistency.

```suggestion
	case BFD_AUTH_SIMPLE:
		/* RFC5880 6.7: To be finished. */
		return false;
	case BFD_AUTH_CRYPTOGRAPHIC:
		/* RFC5880 6.7: To be finished. */
		return false;
	default:
		/* RFC5880 6.7: To be finished. */
		return false;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: bfdd/bfd_packet.c
Line: 933-941

Comment:
**Session lookup before TTL and full packet validation**

Session lookup (`ptm_bfd_sess_find`) has been moved earlier — before the TTL check and before the version/detect_mult/len/M-bit/my_discr validation. In the old code the session was only resolved after the packet passed all RFC 5880 §6.8.6 checks.

The practical consequence is that malformed or TTL-violated packets now perform a hash/key lookup instead of being dropped immediately. For most deployments this is negligible, but it also means that an on-path attacker who knows a peer address can cause a session lookup (and indirectly confirm session existence via timing) with a badly-formed packet, whereas before the packet would have been rejected at the version or TTL check first.

Notably `my_discr == 0` is still validated *after* the lookup: if the session is found (line 935) but the packet later fails the `my_discr` guard (line ~990), the packet is discarded, but the session was already touched. This is benign today because neither the session state nor statistics are updated until after all checks pass, but it is a change in the ordering contract that future contributors should be aware of.

If the primary motivation for moving the lookup earlier is to allow `rx_bad_ctrl_pkt` to be charged against the matched session, a comment explaining the rationale would help reviewers understand the intentional trade-off.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "bfdd: cap IPv6 echo ..."}

[Jafaral]

@greptile review

[Jafaral]

@greptile review