Re: Secdir early review of draft-ietf-bfd-stability-13

[Christian Huitema <[e-mail]>]

On 6/13/2024 8:46 AM, Reshad Rahman wrote:
>   Chiming in late. Inline.
>      On Monday, June 10, 2024, 12:22:13 PM EDT, Jeffrey Haas <[e-mail]> wrote:
>   
>   Christian,
> 
> Thanks for your review.  Some of my comments will overlap those from Alan.
> 
> On Fri, Jun 07, 2024 at 09:54:57PM -0700, Christian Huitema via Datatracker wrote:
>> The authentication sequence number is a 32 bit field. Such numbers can roll
>> over, either after a long duration session or due to a packet injection attack.
> 
> As Alan points out, normal rollover is something we're usually unbothered by
> in the existing authentication algorithms.
> 
> The point you have here is far more about the underlying issue for the null
> authentication procedures:
> 
>> There is some text about that in the description of the NULL authentication. It
>> says:
>>
>>      If bfd.AuthSeqKnown is 1, and the received Sequence Number field is
>>      not equal to bfd.RcvAuthSeq + 1 (in a circular number space), then
>>      the loss count is incremented by one and bfd.RcvAuthSeq is set to the
>>      received Sequence Number.
>>
>> That does not look quite right. Suppose that due to out of order delivery, the
>> packets are received in order 1-3-2-4. Upon reception of packet 3, the
>> algorithm counts one loss and set the next expected value to 4. After packet 2,
>> another loss and expected value to 3. After packet 4, another loss and expected
>> value to 5. So, three losses when none actually occurred.
> 
> Agreed.  We do mention this here:
> 
> : Implementations MAY provide mechanisms wherein all expected packets received
> : across an expected interval but delivered out of order are not considered
> : lost packets.
> 
> We indeed discussed the option about how to avoid some of these out of order
> issues as part of active attacks vs. BFD sessions with NULL authentication.
> The conclusion from that thread is we simply CANNOT leverage the sequence
> numbers for purposes of "do we pass the authentication checks".  As you note
> here:
> 
> <RR> Was there any consideration to change the procedure to increment the loss count so that if we get 1-3-2-4, we increment loss count when we receive 3 (2 is deemed lost) but not when we receive 2 (2 < 3 so that means out of order). Also when we receive 2, since 2 < 3 (OOO) if we don't update bfd.RcvAuthSeq, then when we receive 4 we won't increment loss count. So it'd be counted as 1 loss.

Or, you could use the same "windowed" process as used with the 
authenticated modes. But the security issue is obvious.

The current specification for loss detection with NULL auth is almost 
stateless. A spoofed packet does increase the loss counter, but the next 
packet restores the state to what it needed to be. Adopting your 
proposed rule or the windowed rule will make it stateful. A spoofed 
packet would increment bfd.RcvAuthSeq, and could cause lots of "good" 
packets to be ignored.

This brings back my argument against using NULL authentication. I would 
prefer to not see it used at all, but barring that you could at a 
minimum have a warning in the security section. RFC 5880 says that "BFD 
can provide failure detection on any kind of path between systems, 
including direct physical links, virtual circuits, tunnels, MPLS Label 
Switched Paths (LSPs), multihop routed paths, and unidirectional links 
(so long as there is some return path, of course)." I understand that 
the risk of using NULL auth appears limited if the endpoints are 
connected by direct physical links, and maybe also if tunnels are 
protected by IPSEC. But the risks are obvious for multihop routed paths, 
tunnels without authentication, and probably virtual circuits or MPLS 
Label Switched Paths as well. So, if you cannot swallow a blanket ban on 
NULL auth, at a minimum you should have some MUST NOT specifications for 
any path in which packets can be injected by third parties.

With a precaution like that in place, you could use the same windowed 
procedure for all authentication types, including NULL. That would 
probably simplify implementations.

-- Christian Huitema