bfd: fix RFC 5880 compliance violations in packet reception

[SpadeMomo]

Fixes in this commit:

1. RFC violation on session reset.
2. RFC violation on checking of cp->len.
3. RFC violation on the timing of updating remote discriminator.

For 1, the previous logic in bfd_recv_cb incorrectly discarded all packets with Your Discriminator set to 0 if the local session state was UP or INIT. This violated RFC 5880 Section 6.8.6, which requires accepting such packets (specifically when State is DOWN) to handle remote peer restarts. This discrepancy may cause "zombie sessions" where the local side remained UP ignoring the peer's reset signal.

The original check was introduced with the intention to prevent session aliasing, which caused session state flapping. However, the check was too broad.

This patch replaces the state-based check with a more strict address-based check:

• Removed the conditions of bfd->ses_state != which would block legitimate resets.
• Added match_ip_address to verify that if a zero-discriminator packet arrives for an UP session. its destination address matches the session's bound local address.
• Added the check required by the RFC 5880 that if a zero remote discriminator packet arrives with a state that is not DOWN or ADMIN_DOWN, the packet should be dropped.

This allows legitimate peer resets (correct destination) to proceed while dropping aliased packets that causes vibration.

For 2, this is a relatively minor one and potentially nitpicking. But RFC 5880 requires that the parser should check cp->len < 26 if A bit is set, so I include it.

For 3, the patch delays the update of remote discriminator to after bfd_check_auth returns success. This is required by the RFC. Updating remote discriminator before auth check would incorrectly update local state even when authentication fails, which can be harmful.

This patch also modifies bfd.h to add a new pair of marco that get and set A bit. It also changes the implementation of BFD_SETCBIT to align with other marco's way of implementation.

[riw777]

looks good

[ton31337]

Let's use IPV6_ADDR_CMP().

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[mergify]

⚠️ The sha of the head commit of this PR conflicts with #20333. Mergify cannot evaluate rules on this PR. ⚠️

[greptile-apps]

Greptile Overview

Greptile Summary

Changes

Fixed three RFC 5880 compliance violations in BFD packet reception:

• Session reset handling: Replaced overly broad state-based check with address-based validation. Now properly accepts zero-discriminator packets from legitimate peer resets (when destination address matches) while preventing session aliasing attacks. Added separate check to enforce RFC requirement that zero-discriminator packets with non-DOWN state must be discarded.

• Packet length validation: Enhanced cp->len check to require minimum of BFD_PKT_LEN + 2 when Authentication (A) bit is set, per RFC 5880 specification.

• Remote discriminator update timing: Moved bfd->discrs.remote_discr update to occur after bfd_check_auth() validation. This prevents local state corruption when authentication fails.

• Header file improvements: Added BFD_SETABIT/BFD_GETABIT macros for authentication bit manipulation and fixed BFD_SETCBIT implementation to align with other macro patterns.

Key Improvements

The address-based check for zero-discriminator packets correctly handles the zombie session problem where peers restart but the local side remains UP. The new logic validates the destination address matches the session's bound local address before accepting the reset, preventing session flapping from aliased packets while allowing legitimate resets to proceed.

Confidence Score: 4/5

• Safe to merge with minor whitespace cleanup recommended
• Core RFC compliance fixes are technically sound and well-reasoned. The address-based session reset logic correctly implements RFC 5880 Section 6.8.6 requirements. Auth check ordering fix prevents state corruption. Only minor style issues (trailing whitespace, missing EOF newline) prevent a perfect score.
• No files require special attention - both files contain straightforward improvements

Important Files Changed

Filename	Overview
bfdd/bfd.h	Added BFD_SETABIT/BFD_GETABIT macros and fixed BFD_SETCBIT implementation - all changes are safe and correct
bfdd/bfd_packet.c	RFC 5880 compliance fixes for packet reception - core logic is sound, minor whitespace issues present

Sequence Diagram

sequenceDiagram
    participant Remote as Remote BFD Peer
    participant Net as Network
    participant Recv as bfd_recv_cb()
    participant Sess as BFD Session
    participant Auth as bfd_check_auth()
    
    Remote->>Net: BFD Control Packet
    Net->>Recv: Packet arrives
    
    Note over Recv: Basic validation<br/>(version, length, TTL)
    
    alt A bit set
        Recv->>Recv: Check len >= BFD_PKT_LEN + 2
    else A bit clear
        Recv->>Recv: Check len >= BFD_PKT_LEN
    end
    
    Recv->>Sess: Find session by discriminator
    
    alt Your Discriminator = 0
        alt Session state = UP
            Recv->>Recv: Verify local address matches
            alt Address mismatch
                Recv-->>Net: Drop packet (prevent aliasing)
            end
        end
        
        Recv->>Recv: Check packet state field
        alt Packet state != DOWN or ADMIN_DOWN
            Recv-->>Net: Drop packet per RFC 5880
        end
    end
    
    Note over Recv: Log discriminator changes
    
    Recv->>Auth: Perform authentication check
    Auth-->>Recv: Return result
    
    alt Authentication failed
        Recv-->>Net: Drop packet
    else Authentication succeeded
        Note over Recv: Update remote discriminator<br/>after auth verification
        Recv->>Sess: Update remote discriminator
        Recv->>Sess: Update timers and state
    end

Loading

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Trailing whitespace on this line

Suggested change

	/* Check authentication. */

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

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

Comment:
Trailing whitespace on this line

```suggestion
	/* Check authentication. */
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

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

[greptile-apps]

Missing newline at end of file

Suggested change

	#endif /* _BFD_H_ */
	#endif /* _BFD_H_ */

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: bfdd/bfd.h
Line: 900:900

Comment:
Missing newline at end of file

```suggestion
#endif /* _BFD_H_ */

```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

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