On Apr 25, 2022, at 6:23 AM, Gļebs Ivanovskis <gl...@mikrotik.com> wrote: > 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. > "Keyed MD5 and Meticulous Keyed MD5 Authentication" and 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, presumably > followed by this comment: > > 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.
From a security point of view, the usual process is to verify packets first, and then use the contents of the packets. This can be expensive from a CPU point of view, but it's the cost of being secure. > 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. It would be good to say that packets which fail authentication MUST NOT affect the BFD state. > 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). That seems reasonable. > 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. Yes. The "secure sequence numbers" draft attempts to address these issues. Alan DeKok.