Re: Shepherd writeup for draft-ietf-bfd-optimizing-authentication

[Mahesh Jethanandani <[e-mail]>]

Hi Reshad,

Happy Canada day!

Thanks first of all for the shepherd writeup. Please see my comments/questions inline.

@Ashesh, please comment on the change to the table.

> On Jun 14, 2020, at 11:50 AM, Reshad Rahman (rrahman) <[e-mail]> wrote:
> 
> Authors, WG,
>  
> The writeup is available at https://datatracker.ietf.org/doc/draft-ietf-bfd-optimizing-authentication/ <https://datatracker.ietf.org/doc/draft-ietf-bfd-optimizing-authentication/>
>  
> For convenience I’ve copied the comments on the document below.
>  
> Regards,
> Reshad.
>  
> General:
> Updates RFC5880 missing from title page

Hmm. How does one add the “Updates” tag?

> Replace BFD frames by BFD packets or BFD control packets. Don’t use frames since RFC5880 uses packets.

Done.

> Use of term Null-authentication TLV. RFC5880 uses authentication section, doesn’t mention auth TLV.

Ok. Have changed all references of “NULL Auth TLV” to “NULL Auth Type”.

>  
> Abstract:
> Mention that this document updates RFC5880.

Done.

>  
> Requirements Language
> Please put this is a separate (sub)section later, e.g. after introduction.

Done.

>  
> Introduction
> First paragraph: s/is computationally intensive process/is a computationally intensive process/

Done.

> Split first sentence into 2, e.g.
>    Authenticating every BFD [RFC5880] packet with a Simple Password, or
>    with a MD5 Message-Digest Algorithm [RFC1321], or Secure Hash
>    Algorithm (SHA-1) algorithms is a computationally intensive process.
>    This makes it difficult, if not impossible, to authenticate every packet,
>    particularly at faster rates.

Done.

>  
> 2nd paragraph: “… only BFD frames that signal a state change in BFD be authenticated.” State change is not 100% correct since P/F/D bit changes aren’t state changes (as mentioned in more detail below in section 2 comments). What about this instead: “State change, a demand mode change (to D bit) or a poll sequence change (P or F bit change) in a BFD packet are categorized as a significant BFD change. This document proposes that all BFD control packets which signal a significant BFD change MUST be authenticated if the session’s bfd.AuthType is non-zero. Other BFD control packets MAY be transmitted and received without the A bit set.” If you do use “significant BFD change”, add it to terminology section.

Done.

> s/non-state change frame/BFD control packets without state or D/F/P bit change/, e.g.
> “To detect a Man In the Middle (MITM) attack, it is also proposed that BFD control packets without a significant change be authenticated occasionally.  The interval of these control packets…”

Done.

>  
> Section 2
> POLL and DEMAND are NOT strictly states. POLL refers to “Poll sequence” as specified in section 6.5 of RFC5880. DEMAND refers to “Demand mode” as specified in section 6.6 of RFC5880. In the table, the POLL entry refers to polling sequence enabled and in any BFD state. Likewise, the DEMAND entry refers to Demand mode. This means that a session in UP state, in demand mode and polling sequence enabled will match 3 entries in that table. It’s a bit confusing. Here’s what I suggest instead:
> Take POLL out of the table. Add a paragraph mentioning that if P or F bit changes value, the packet MUST be authenticated
> Take DEMAND out of the table. Add a paragraph mentioning that if D bit changes value, the packet MUST be authenticated

Have made the change as follows. I am hoping Ashesh can comment on it.

The significant changes for which authentication is being suggested
   include:

          Read   : On state change from <column> to <row>
          Auth   : Authenticate frame
          NULL   : No Authentication. Use NULL AUTH Type.
          n/a    : Invalid state transition.
          Select : Most packets NULL AUTH. Selective (periodic)
                   packets authenticated.
         +--------+-----------+---------+---------+
         |            | DOWN   | INIT   | UP     |
         +--------+--------+--------+--------+
         | DOWN   |  NULL  |  Auth  |  Auth  |
         +--------+--------+--------+--------+
         | INIT   |  Auth  |  NULL  |  n/a   |
         +--------+--------+--------+--------+
         | UP     |  Auth  |  Auth  | Select |
         +--------+--------+--------+--------+

                       Optimized Authentication Map

   If P or F bit changes value, the packet MUST be authenticated.  If
   the D bit changes value, the packet MUST be authenticated.


>  
> Another comment on the table. The text says it should be read as state change from column to row. Column INIT to row UP is n/a whereas column UP to row INIT is Auth. INIT to UP is a valid transition, UP to INIT is not (has to go through DOWN first). So I think those entries should be reversed in the table.

Done.

>  
> Last paragraph: CC frames is not defined in BFD, use “control packets” instead?

Done.

>  
> Section 3
> Sequence number mentions “as defined in [RFC5880]”. Suggest mentioning bfd.XmitAuthSeq

Done.

>  
> Security Considerations.
> I believe this needs to be beefed up:
> Use of sequence number for non-authenticated frames. Secure sequence numbers even better.
> Mention (again) that non-authenticated BFD packets which have a significant change (state, D/F/P) are dropped. So if someone injects a non-authenticated packet with Down state to take down the session, that won’t work.

The Security Considerations section now reads as follows:

  The approach described in this document enhances the ability to
   authentication a BFD session by taking away the onerous requirement
   that every frame be authenticated.  By authenticating packets that
   affect the state of the session, the security of the BFD session is
   maintained.  In this mode, packets that are a significant change but
   are not authenticated, are dropped by the system.  Therefore, a
   malicious user that tries to inject a non-authenticated packet, e.g.
   with a Down state to take a session down will fail.  That combined
   with the proposal of using sequence number defined in Secure BFD
   Sequence Numbers [I-D.ietf-bfd-secure-sequence-numbers] further
   enhances the security of BFD sessions.



>  
> Section 6.2
> RFC5880 should be a normative reference.

Done.

Mahesh Jethanandani
[e-mail]