Review of draft-ietf-idr-bgp-bfd-strict-mode-18

[Alvaro Retana <[e-mail]>]

 Dear authors:

As I read draft-ietf-idr-bgp-fsm-iana, I reviewed this document as well.

I added inline comments (below). I have a significant concern that I want
to highlight here first:

   Does strict-more mean "BFD Up", or are there other cases?

   The Introduction defines strict-mode as: "BFD "strict-mode" operation as
   preventing BGP session establishment until both the local and remote
   speakers have an established BFD session."  However, the FSM specifies
   cases where BfdAdminDown or BfdDisabled are TRUE, but session
establishment
   continues. See related comments near line 200.


Thanks in advance for considering my comments.

Alvaro.


[Line numbers from idnits.]

...
121 1.  Introduction
...
129   The BFD interaction with BGP is specified in Section 10.2 of
130   [RFC5882].  When BFD is enabled for a BGP neighbor, faults in the
131   bidirectional forwarding detected by BFD result in BGP session
132   termination.  It is possible in some failure scenarios for the
133   network to be in a state such that a BGP session may be established
134   but a BFD session cannot be established.  In some other scenarios, it
135   may be possible to establish a BGP session, but a degraded or poor-
136   quality link may result in the corresponding BFD session going up and
137   down frequently.

[nit] "BFD interaction with BGP is specified in Section 10.2 of [RFC5882]"

§10/rfc5882 is non-normative.  s/specified/described



...
197 4.  BGP FSM Events for BFD Strict-Mode

199   Event 30: BfdAdminDown
200      Definition:
201         The BFD session associated with this BGP session has
202         transitioned to the AdminDown state.

[minor] Because this draft is referencing 2 different FSMs, it would be
helpful to indicate when the BGP FSM is not the one referenced (which
should be a lot less than when the BFD FSM is referenced).  For example,
AdminDown is a state in the BFD FSM [rfc5880].



[major] The use of BfdAdminDown is not consistent with "strict-mode".

[The same comment applies to BfdDisabled.]

The use of BfdAdminDown, for example in §8.3.1:

=====
8.3.1. Connect - Handling BfdAdminDown / BfdDisabled / BfdUp
   In response to the BfdAdminDown event (Event 30), the BfdDisabled
   event (Event 33), or the BfdUp event (Event 32) the the local system
   checks to see if it is in the ConnectDelayOpenBfdUpPending sub-state.
   If the FSM is in the ConnectDelayOpenBfdUpPending sub-state, the
   local system:

   * sends a KEEPALIVE message,
   ...
=====

...is not consistent with how strict-mode is defined: "BFD "strict-mode"
operation as preventing BGP session establishment until both the local and
remote speakers have an established BFD session." (from the Introduction).

[Several places in the draft group BfdAdminDown / BfdDisabled / BfdUp.]

By definition, an AdminDown BFD session is not Up, so in a "strict-mode"
the BGP session should not be sending KEEPALIVEs (as specified above).
Note that §8.1 (Overview of BFD Strict FSM Changes) also mentions the
expected behavior:

   When BFD is enabled, and BFD strict-mode is enabled and negotiated,
   the BGP finite state machine is prevented from send a KEEPALIVE to
   the remote BGP speaker and advancing to the OpenConfirm state until
   the associated BFD session has reached the Up state.

Again, in AdminDown the BFD session is not Up!



204      Status:
205         Optional

207      Optional Attribute Status:
208         The BfdEnabled attribute for this BGP session SHOULD be set to
209         TRUE.

[major] Setting BfdEnabled to TRUE when BFD is AdminDown doesn't make sense
to me.  Did you mean FALSE?


[major] When is it ok to not set BfdEnabled?  IOW, why is this action
recommended and not required?



211   Event 31: BfdDown
212      Definition:
213         The BFD session associated with this BGP session has
214         transitioned to the Down state.

216      Status:
217         Optional

219      Optional Attribute Status:

[nit] Some of the "Optional Attribute Status" descriptions are left blank
-- it gives the impression that something may be missing.  Consider adding
"None".



221   Event 32: BfdUp
222      Definition:
223         The BFD session associated with this BGP session has
224         transitioned to the Up state.

226      Status:
227         Optional

229      Optional Attribute Status:
230         The BfdEnabled attribute for this BGP session SHOULD be set to
231         TRUE.

[major] When is it ok to not set BfdEnabled?  IOW, why is this action
recommended and not required?



...
242   Event 34: BfdHoldTimerExpires
243      Definition:
244         The BFD holdtimer, which is set when the negotiated BGP hold
245         time is zero, has expired.

247      Status:
248         Optional

250      Optional Attribute Status:
251         *  The HoldTimer SHOULD NOT be running.

253         *  The negotiated HoldTime SHOULD be zero.

255         *  The BGP session state SHOULD be in Connect, Active, or
256            OpenSent.

[major] These 3 sentences sound like statements (expectations), not
normative actions, which makes the use of SHOULD/SHOULD NOT inappropriate.
Is that the intent?



258   Event 35: BfdStrictConfigChanged
259      Definition:
260         The configuration for the BFD strict configuration for the BGP
261         session has been changed.

263      Status:
264         Optional

266      Optional Attribute Status:
267         If BfdEnabled is FALSE, this event MUST NOT occur.  When BFD
268         has been disabled, the local system will trigger a BfdAdminDown
269         event instead.

[major] "this event MUST NOT occur"

This fragment seems to be a statement of fact, not a Normative requirement:
s/MUST NOT/must not



...
292 7.  Starting and Stopping BFD Sessions Associated BGP BFD Strict-Mode

[nit] s/BFD Sessions Associated BGP/BFD Sessions Associated with BGP



294   Implementations SHOULD start the BFD session associated with the BGP
295   BFD strict-mode session prior to the BGP FSM starting.  The
296   motivation is to avoid delaying BGP FSM transitions while waiting for
297   the BFD session reach the Up state.

[nit] s/session reach/session to reach



299   Similarly, to support BFD hold-down requirements for detecting BFD
300   session stability (see Section 10), implementations SHOULD NOT
301   immediately destroy BFD sessions when associated BGP connections
302   transition to Idle.

[major] When is it ok to immediately destroy BFD sessions?  Or is the
"later" destruction ok?  Why is this behavior recommended and not required?

The general suggestion seems valuable, regardless of hold-down.  Alternate
suggestion>

   When associated BGP connections transition to Idle, an implementation
   MAY destroy the BFD session, but doing so may delay the next BGP
   establishment.



304 8.  BGP FSM State Changes

306 8.1.  Overview of BFD Strict FSM Changes

308   When BFD is enabled, and BFD strict-mode is enabled and negotiated,
309   the BGP finite state machine is prevented from send a KEEPALIVE to
310   the remote BGP speaker and advancing to the OpenConfirm state until
311   the associated BFD session has reached the Up state.

[nit] s/prevented from send/prevented from sending



...
324   For each of these scenarios, when BFD is enabled, and BFD strict-mode
325   is negotiated, a sub-state is introduced to track the pending BFD Up
326   event:

328   *  ConnectDelayOpenBfdUpPending

330   *  ActiveDelayOpenBfdUpPending

332   *  OpenSentBfdUpPending
333   *  OpenSentConfirmedBfdUpPending

[minor] It would be nice to explicitly state what each sub-state implies
(for example, KEEPALIVE received, BFD no up, etc.).  Yes, this information
is in the FSM, but it could make further implementations easier.



...
360 8.3.1.  Connect - Handling BfdAdminDown / BfdDisabled / BfdUp

362   In response to the BfdAdminDown event (Event 30), the BfdDisabled
363   event (Event 33), or the BfdUp event (Event 32) the the local system
364   checks to see if it is in the ConnectDelayOpenBfdUpPending sub-state.
365   If the FSM is in the ConnectDelayOpenBfdUpPending sub-state, the
366   local system:

[nit] s/the the/the



...
727 8.5.1.  OpenSent - Handling BfdAdminDown / BfdDisabled / BfdUp

729   In response to the the BfdAdminDown event (Event 30), the BfdDisabled
730   event (Event 33), or the BfdUp event (Event 32), and the FSM is in
731   the OpenSentBfdUpPending or the OpenSentConfirmedBfdUpPending sub-
732   states, the local system:

[nit] s/the the/the



...
909 8.5.6.  OpenSent - Handling Event 26, KeepAliveMsg

911   When BFD strict-mode is not in use, receiving a KEEPALIVE message
912   while in the OpenSent state is a finite state machine error.
...
960   When a KEEPALIVE message is received, and either BfdEnabled is FALSE
961   or BfdStrictNegotiated is FALSE, or in response to any other event
962   (Events 9, 11-13, 20, 25, 27-28), the local system:

964   *  sends the NOTIFICATION with the Error Code Finite State Machine
965      Error,

[major] "BfdEnabled is FALSE or BfdStrictNegotiated is FALSE"

Maybe I'm lost...  We should only have advanced to OpenSent if BFD
strict-mode was enabled and negotiated, right?  If so, how can they now be
FALSE?  I'm assuming BFD was disabled manually (that explains BfdEnabled
being FALSE), but what about the negotiation?

IF we somehow got to a place where BfdStrictNegotiated is FALSE (but
BfdEnabled is TRUE), then we're at the boundary between strict-mode (what
is defined in this document) and "normal" (non-strict operation:
BfdStrictNegotiated is FALSE). This makes me uneasy because the scope of
this document is strict-mode; "normal" is specified in rfc5882, so if we
change anything here to operate in "normal” mode, we might need to consider
rfc5882 (even if some of the BGP-specific guidance is non-normative).



[major] Using FSM Errors is consistent with the FSM in rfc4271. The logic
here seems reasonable too.  My concern is that the resulting state machine
is becoming increasingly nontrivial (which is not unexpected).

The problem is that an implementation can easily miss an event that is
supposed to be inherited from rfc4271 versus one that is newly overridden.
I strongly suggest adding a state/event table for the four new pending
states, rather than only relying on prose replacements of rfc4271
paragraphs.

[I realize that there's no such table in rfc4271...something for the WG to
consider, maybe as a reference artifact...]



...
1064 9.  Closing BGP Sessions

1066   When BGP sessions are closed according to the procedures in this
1067   document, the session SHOULD be terminated with a NOTIFICATION
1068   message with the Cease Code (6) and the "BFD Down" Subcode (10); see
1069   [RFC9384].  This informs the operator that interaction with BFD is
1070   the root cause of the BGP session being unable to move to the
1071   Established state.

[major] rfc9384 only considers using this NOTIFICATION when BFD goes Down,
but this document adds other reasons.  IMO, this document should explicitly
Update rfc9384.



1073 10.  Stability Considerations
...
1082   To avoid deadlock when utilizing both BFD hold-down and BFD strict-
1083   mode, when strict-mode is enabled for a peer, the BGP FSM MUST be
1084   enabled.  That is, BFD hold-down procedures MUST NOT prevent BGP
from
1085   establishing a connection with the remote BGP speaker.

[major] "BGP FSM MUST be enabled"

What does this mean? Isn't the BGP FSM enabled whenever BGP runs?  I see no
interoperability requirement to use "MUST".



[major] "BFD hold-down procedures MUST NOT prevent..."

This seems to be a statement of requirements, not something that needs
Normative language for interoperability.  What am I missing?



...
1093   It is RECOMMENDED that the BFD hold-down intervals used with BFD
1094   strict-mode, when configured, use similar values.  Similarly, the
1095   negotiated BGP holdtime SHOULD be long enough to account for the
time
1096   between the BGP FSM reaching the OpenConfirm state, the BFD
hold-down
1097   interval, and any delay for the BFD session being initiated.
Failure
1098   to do so can result in the BGP speaker that has transitioned to the
1099   Established state expiring its BGP holdtime and closing the
1100   connection.  This is because the remote BGP speaker hasn't
1101   transitioned to Established and begun sending KEEPALIVE messages.

[major] "RECOMMENDED that the BFD hold-down intervals used with BFD
strict-mode, when configured, use similar values"

Similar, but not the same?  From an interoperability point of view, what is
"similar".  For example, are 1s and 30s "similar" (they are both less than
a minute)?  Also, why is this action recommended and not required?  IMO,
this draft should explain the operational reason and consequences of using
values that are not "similar" -- the use of Normative language is not
necessary, but the explanation would be valuable.



[major] "negotiated BGP holdtime SHOULD be long enough"

The HoldTime is negotiated, so the local BGP Speaker cannot guarantee that
the result will be "long enough".  Please add this caveat at the end of the
paragraph.



...
1106   The behavior of BGP speakers implementing BFD hold-down without
1107   negotiating the BFD strict-mode feature is out of scope of this
1108   document.  However, the authors are aware that inconsistent
behaviors
1109   in BGP implementations for BFD hold-down without BFD strict-mode may
1110   result in BGP session deadlock.

[major] Leaving the use of BFD hold-down in general BGP sessions out of
scope is fine. However, we should add more coverage if one peer is trying
to negotiate BFD strict-mode. In other words, even if the configuration is
asymmetric, one side follows the recommendations in this document -- what
are the issues in those cases?



1112 11.  Manageability Considerations
...
1118   To simplify troubleshooting and avoid inconsistencies, it is
1119   RECOMMENDED that BFD strict-mode configuration be consistent for
both
1120   BGP peers.

[major] "RECOMMENDED that BFD strict-mode configuration be consistent"

Capability negotiation allows the configuration to be asymmetric. I
understand the rationale behind this recommendation, but it is not required
for interoperability.  s/RECOMMENDED/recommended



1122   This draft introduces sub-states in the existing BGP finite state
1123   machine for tracking BFD session status inputs for strict mode
1124   operation.  Implementations SHOULD provide visibility for these sub-
1125   states in its display of the BGP finite state machine.

[major] "SHOULD provide visibility"

Same as above: good recommendation, but not needed for interoperability.
s/SHOULD/should



1127 12.  Security Considerations

[major] Please start this section by stating that the Security
Considerations in rfc5880 and rfc4271 apply.



1129   The mechanism defined in this document interacts with the BGP finite
1130   state machine when so configured.  The security considerations for
1131   BFD thus, become BGP-4 considerations [RFC4271] when so used.  Given
1132   that a BFD session is required for a BGP session, a
Denial-of-Service
1133   (DoS) attack on BGP can now be mounted by preventing a BFD session
1134   between the BGP peers from reaching the Up state, or interrupting an
1135   existing BFD session.  The use of a BFD Authentication mechanism,
1136   some of which are defined in [RFC5880], is thus RECOMMENDED when
used
1137   to protect BGP-4 [RFC4271].

[major] A series of rogue node-related attacks are not covered in this
section.  For example:

- as mentioned in §10, the BGP session may deadlock -- this type of
risk/threat should be included here by reference.

- Authentication can help, but it may not stop a DoS attack where the BFP
packets are simply dropped/filtered, delayed, etc.

- There's a distinction between preventing the session from reaching Up and
tearing down an Established session -- also possible by configuration.

- An attacker can make the receiver believe BFD is Up when the forwarding
path isn't actually usable -- I think rfc5880 mentions this too.


Some of these attacks are not new wrt BFD or BGP, but they should be
mentioned nonetheless -- and the fact that they are known should be
highlighted.  Even if known threats, the attack surface is different.



[minor] s/The use of a BFD Authentication mechanism, some of which are
defined in [RFC5880], is thus RECOMMENDED.../The use of BFD Authentication
[RFC5880] is thus RECOMMENDED...

I'm suggesting this wording change to narrow the recommendation and avoid
opening the door to discussion about rfc9985/rfc9986, which are
Experimental.



[major] "BFD Authentication...is thus RECOMMENDED"

Why is it recommended and not required? When is it ok not to use it?

Instead of going through any in-depth treatment of the threat model, etc.,
stating up front that the Security Considerations in rfc5880 apply would
help.



...
1146 13.2.  BGP-4 FSM Optional Session Attributes Sub-Registries

1148   This document defines new BGP finite state machine session
1149   attributes.  [BGP-IANA-FSM] manages these new registrations.

[] This section and the next don't ask IANA for an action, so they
shouldn't be included here.

IMHO, a better model would be to let BGP-IANA-FSM define the registry and
have this document request the values to be registered (an IANA action).



...
1202 15.  Normative References

1204   [BGP-IANA-FSM]
1205              Haas, J., Hares, S., and K. Patel, "IANA Registrations
for
1206              the BGP Finite State Machine (FSM)", Work in Progress,
1207              Internet-Draft, draft-ietf-idr-bgp-fsm-iana-01, 12 June
1208              2026, <https://datatracker.ietf.org/doc/html/draft-ietf-
1209              idr-bgp-fsm-iana-01>.

[minor] This reference can be Informative: it is used only to indicate
where the registries are defined.

[EoR-18]