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
