oxidecomputer / oxidecomputer/maghemite

BFD doesn't account for FSM state when reacting to peer's Demand mode

Open
#861 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bfd Bug mgd needs testing rust
Dominant language
Rust
Stars
94
Forks
6
Avg merge
1d 8h
Merged PRs (30d)
12

Description

The following is an issue identified by AI when I asked it to do an adversarial code review of #795. This was pre-existing in the sync/pthread impl, which is why it was not addressed as part of the async swap-out.

Problem

The BFD state machine currently treats the last received Demand bit as
sufficient to suppress unsolicited control packets.

RFC 5880 defines remote Demand mode as active only when all of the
following are true:

  • the remote D bit is set;
  • the local session state is Up;
  • the last reported remote session state is Up.

The implementation does not retain the remote session state needed to
apply this rule. As a result, a packet carrying D can suppress periodic
transmission while the session is Down or Init.

Failure scenario

  1. The local session starts in Down.
  2. It receives a packet with State=Down and D=1.
  3. The local state transitions to Init.
  4. Periodic transmission is suppressed because the remote D bit is set.
  5. The peer may never observe the local Init state, preventing the
    handshake from converging.

A stale D bit can similarly prevent periodic Down packets from being sent
while recovering from a failed Up session.

Impact

A premature, stale, or non-conforming Demand bit can deadlock session
establishment or recovery.

Proposed direction

Track RemoteSessionState as required by RFC 5880.

Remote Demand mode should be active only when:

remote_demand_mode && local_state == Up && remote_state == Up

When any condition stops being true, periodic control transmission must
resume immediately.

The local D bit must likewise not be transmitted unless local Demand mode
is configured and both sides are perceived Up.

Testing

Cover at least:

  • Down+D while local Down;
  • Init+D while local Down or Init;
  • Up+D while local is not Up;
  • Up/Up+D suppresses periodic transmission;
  • leaving Up resumes periodic transmission even if the last received D
    bit remains set.

The existing demand_mode_suppresses_unsolicited_sends test should be
updated because it currently asserts the overbroad behavior.

References

  • RFC 5880 §6.6
  • RFC 5880 §6.8
  • RFC 5880 §6.8.6
  • RFC 5880 §6.8.7

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the BFD state machine and the existing demand_mode_suppresses_unsolicited_sends test, then consult RFC 5880 sections 6.6, 6.8, 6.8.6, and 6.8.7. Done means remote session state is tracked, Demand mode is honored only for an Up/Up session, periodic sends resume when leaving Up, and the listed Down, Init, and Up cases are covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.