Re: Comments on draft-ietf-bfd-generic-crypto-auth

[Mahesh Jethanandani <[e-mail]>]

Nobo,

See comments inline.

On Apr 14, 2014, at 10:23 AM, Nobo Akiya (nobo) wrote:

> Hi Authors,
> 
> I have reviewed your draft (draft-ietf-bfd-generic-crypto-auth ), comments below.
> Since document is nearing expiration, perhaps you can address some of these comments and re-publish?
> 
> 
> (1) Abstract
> 
> s/arbitary/arbitrary/

Fixed.

> 
> 
> (2) Section 2
> 
> Before listing "Not Before Time" and "Not After Time", it'll be great if the document can provide some words to better transition from previous paragraph. 

Those are the parameters associated with a BFD SA just like Authentication Algorithm etc. The indentation might have thrown you off. Let me see if I can fix it.

> 
> 
> (3) Section 3.3
> 
> [snip]
>   The device MUST fill the Auth Type and the Auth Len fields before the
>   authentication data is computed.  The Sequence Number field MUST be
>   set to bfd.XmitAuthSeq.
> [snip]
> 
> The paragraph is slightly confusing wrt whether Sequence Number field need to be set before or after authentication data is computed. It'll be good to clarify this.


I was looking at RFC 5880 for guidance on this. It says "The Sequence Number field MUST be set to bfd.XmitAuthSeq." Period. No mention of before or after.

How about this:
"The device MUST fill the Auth Type, the Auth Len fields and set the Sequence Number field to bfd.XmitAuthSeq before the authentication data is computed."


> 
> 
> (4) Section 3.4, second paragraph
> 
> [snip]
>   If the Authentication Present (A) bit is set in the packet header and
>   the receiver will try to find a appropriate BFD SA in its local key
>   table to process the packet.
> [snip]
> 
> I believe you meant to say:
> 
>   If the Authentication Present (A) bit is set in the packet header and
>   Auth Type is TBD1 or TBD2, the receiver is to find an appropriate BFD
>   SA in its local key table to process the packet.
> 
> See below for usage of TBD1/TBD2.

Ok. Will fix it.

> 
> 
> (5) Section 3.4, fourth paragraph
> 
> Is it better to say "unsigned 64 bit circular number space" here, or keep it as "unsigned 32 bit circular number space"? If latter, then the document should clarify that it's low order 32 bits.

Ok. Will change it to say "unsigned 64 bit circular number space"

> 
> 
> (6) Section 3.4, sixth paragraph
> 
> [snip]
>  In such a case, an error event SHOULD be logged.
> [snip]
> 
> Such log could become DoS attack point? Rate limiting of such log is outside the scope of this document, but it could be beneficial to explain this in the Security Considerations section ... optional comment though.
> 

How about this:

"In such a case, an error event SHOULD be logged. Rate limiting of such a log to prevent a DoS attack is outside the scope of this document."

> 
> (7) IANA Considerations
> 
> Indeed values 6 and 7 are next, but it's technically not correct to just use them [yet].
> Can we use TBD1 and TBD2? If someone is implementing and early allocation is required, then we can do that.
> 
> Also it'll be beneficial for this section to clearly describe what is needed from where. Maybe something like:
> 
>  IANA is requested to assign two authentication types from the
>  "BFD Authentication Types" sub-registry within the "Bidirectional
>  Forwarding Detection (BFD) Parameters" registry.
> 
>  Address  BFD Authentication Type Name             Reference
>  -------  ---------------------------------------  -------------
>     TBD1  Cryptographic Authentication             this document
>     TBD2  Meticulous Cryptographic Authentication  this document
> 
> 

Ok. Will make the suggested changes.

> (8) Security Considerations
> 
> s/reestablishing/re-establishing/
> 

Will fix.

> 
> (9) References
> 
> RFC5880 should be a normative reference instead of informative, as document is even referencing a variable from RFC5880 (ex: bfd.XmitAuthSeq).
> 

That is correct. Consider it done.

Thanks

> 
> -Nobo
> 
> 

Mahesh Jethanandani
[e-mail]