Regarding keyed MD5/SHA1 authentication for BFD (RFC 5880)

[Gļebs Ivanovskis <[e-mail]>]

Hi,

I have a question regarding the order of operations during receipt of 
BFD control packet using keyed MD5/SHA1 authentication. Both Section 
6.7.3. <https://www.rfc-editor.org/rfc/rfc5880.html#section-6.7.3> 
"Keyed MD5 and Meticulous Keyed MD5 Authentication" and Section 6.7.4. 
<https://www.rfc-editor.org/rfc/rfc5880.html#section-6.7.4> "Keyed SHA1 
and Meticulous Keyed SHA1 Authentication" (in parts "Receipt Using Keyed 
MD5 and Meticulous Keyed MD5 Authentication" and "Receipt Using Keyed 
SHA1 and Meticulous Keyed SHA1 Authentication" respectively) suggest 
that Sequence Number field of incoming control packet is examined prior 
to calculating any digest/hash. I have discovered that the order was the 
other way round in early drafts, but then was changed between versions 8 
and 9 
<https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht?url1=https://www.ietf.org/archive/id/draft-ietf-bfd-base-08.txt&url2=https://www.ietf.org/archive/id/draft-ietf-bfd-base-09.txt>, 
presumably followed by this comment 
<https://mailarchive.ietf.org/arch/msg/rtg-bfd/tk6gbThUz-_30QGUIV8o7701t-w/>:

    If we check the sequence number after doing a MD5 digest
    verification, even if we get replayed packets, we will be doing the
    costly hash operations, thus wasting a lot of CPU and exposing
    ourselves to simple CPU exhaustion attacks.

Unfortunately, I couldn't find any discussion following this suggestion. 
I understand the motivation of making a "cheaper" Sequence Number check 
before doing more "expensive" digest/hash calculation, but the 
reordering of stages does not seem to be thought through very well. The 
text of RFC 5880 mandates implementations /to act/ on received Sequence 
Number /before validating/ digest/hash:

    Otherwise (bfd.AuthSeqKnown is 0), bfd.AuthSeqKnown MUST be set to
    1, and bfd.RcvAuthSeq MUST be set to the value of the received
    Sequence Number field.

and

    Otherwise (bfd.AuthSeqKnown is 0), bfd.AuthSeqKnown MUST be set to
    1, bfd.RcvAuthSeq MUST be set to the value of the received Sequence
    Number field, and the received packet MUST be accepted.

This leaves the opportunity for unauthorized control packet to mess up 
session state variables, potentially leading to subsequent valid packets 
being dropped due to Sequence Number outside of expected range and BFD 
session being erroneously diagnosed as Down. Note that "and the received 
packet MUST be accepted" clause has been removed from MD5 part, but not 
from SHA1 part, requesting implementations to skip hash verification 
altogether if bfd.AuthSeqKnown is 0.

Is the sequence of operations described in Sections 6.7.3. and 6.7.4. 
correct?

In my opinion, as a minimum, the following changes need to be made:

  * The "and the received packet MUST be accepted" clause needs to be
    removed from Section 6.7.4.
  * Sections 6.7.3. and 6.7.4. need to split examining incoming packet's
    Sequence Number (which can be done before digest/hash verification)
    and acting upon it (which has to happen after verification).

Furthermore, I would like to suggest going back to original ordering 
with digest/hash verification being done before examining Sequence 
Number, because it simplifies the algorithm. I don't think that checking 
Sequence Number first provides much protection against CPU exhaustion 
attacks, because Sequence Numbers are transmitted in clear text and it 
wouldn't be that difficult for an attacker to come up with a valid 
Sequence Number to pass "cheap" check.

And finally, one more minor thing. Description of bfd.RcvAuthSeq in 
Section 6.8.1. 
<https://www.rfc-editor.org/rfc/rfc5880.html#section-6.8.1> "State 
Variables" implies that it needs to be updated every time a valid 
control packet is received:

    A 32-bit unsigned integer containing the last sequence number for
    Keyed MD5 or SHA1 Authentication that was received.  The initial
    value is unimportant.

However, Sections 6.7.3. and 6.7.4. only mention that "bfd.RcvAuthSeq 
MUST be set to the value of the received Sequence Number field" when 
bfd.AuthSeqKnown is 0. It would be nice to explicitly say that 
bfd.RcvAuthSeq needs to be updated in case bfd.AuthSeqKnown is 1 as 
well, so that implementations know when exactly this must be done (after 
digest/hash verification, I suppose).

Best regards,
Glebs