On May 15, 2025, at 9:48 AM, Ketan Talaulikar <[email protected]> wrote:
> 85 1.  Introduction
> 
> 87   BFD [RFC5880] (Section 6.7) defines a number of authentication
> 88   mechanisms, including Simple Password, and various other methods
> 89   based on MD5 and SHA1 hashes.  The benefit of using cryptographic
> 90   hashes is that they are secure.  The downside to cryptographic hashes
> 91   is that they are expensive and time consuming on resource-constrained
> 92   hardware.
> 
> <minor> The term "Meticulous Keyed" that is used in the abstract and further
> in this section and document may trip readers. It would be good to have some
> text here to explain that term along with the reference to the specific
> sections of RFC5880.

  I'll add some text in the introduction to make this clear.

> 117 2.  Requirements Language
> 
> 119   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> 120   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
> 121   document are to be interpreted as described in RFC 2119 [RFC2119].
> 
> <major> Please align the above blob with BCP14 including RFC8174.

  OK.

> 136   Some of the state variables in BFD [RFC5880] (Section 6.8.1), are
> 137   related to the authentication type being used for a particular
> 138   session.  However, the definitions given in BFD [RFC5880] are
> 139   specific to Keyed MD5 or SHA1 Authentication, which limit their
> 140   utility for new authentication types.  For the purpose of the
> 141   experiment, this specification updates the definition of some of the
> 142   state variables as given below.
> 
> <minor> Perhaps rephrase the above sentence to convey
> that this document does not "update" RFC5880 since it is narrowly scoped to
> the types introduced in this document alone and to this experiment only?

  The text updates RFC5880 definitions.   This document mandates that state 
transitions use a method which provides for full packet integrity checks, where 
the definition of bfd.AuthType in RFC5880 doesn't include such text.

> 185 4.  Architecture of the Auth Type Method
> 
> 187   When BFD uses authentication, methods using MD5 or SHA1 are CPU
> 188   intensive, and can negatively impact systems with limited
> 189   computational power.
> 
> <minor> Do we want to add that the BFD function has been offloaded to 
> hardware or
>  lower powered CPUs or something like that in some router implementations ? 
> Or,
> better still move all these discussions to the optimizing authentication 
> document since
> those considerations apply there and are not specific to these two new types.

  I think such text can go into the Optimizing Authentication draft.

> 252   Opt Mode:
> 253      The Optimized Authentication Mode, as defined in
> 254      [I-D.ietf-bfd-optimizing-authentication].  allows for systems to
> 255      switch to a less computationally intensive authentication method,
> 256      without changing the main bdf.AuthType.
> 
> <major> Define this field here and specific to the new types introduced. The
> reference to optimizing-auth draft is perhaps only to the extent of indicating
> that this field is where the 1 or 2 values are encoded to indicate the shift
> in modes.

  I'll copy the text from the optimizing authentication mode.

> 271   Auth-Key:
> 
> 273      This field carries the 32-bit (4 octet) ISAAC output which is
> 274      associated with the Sequence Number.  The ISAAC PRNG MUST be
> 
> <minor> Here, a forward reference is needed to the section where the
> description of the ISAAC Page and how to use the sequence number to pick
> the 32-bit value from it is described.

  I've added a new section towards the start of the document which explains 
both why ISAAC was chosen, and how ISAAC is used in correlation with the 
sequence number to pick a pseudo-random number.

> 296   The receiving system accepts the packet if the key ID matches one of
> 297   the configured Keys, and the Auth-Key derived from the selected Key,
> 298   Seed, and Sequence Number matches the Auth-Key carried in the packet,
> 299   and the sequence number is strictly greater than the last sequence
> 300   number received (modulo wrap at 2^32)
> 
> <nit> missing a "." above. There are multiple such instances in this document
> where the trailing period has been missed. Please check and fix them all.

  Fixed.

> 304      The Auth Type field MUST be set to one of two TBDs (Optimized MD5
> 305      Meticulous Keyed ISAAC Authentication or Optimized SHA-1
> 306      Meticulous Keyed ISAAC Authentication).
> 
> <minor> Could we avoid the use of "TBDs" here? IOW, can we reference them by 
> their names that are in the brackets?

  I've updated the text to address this issue.

> 332      If the received BFD Control packet does not contain an
> 333      Authentication Section, or the Auth Type is not correct (one of
> 334      two possible TBDs, Optimized MD5 Meticulous Keyed ISAAC
> 
> <minor> Same as the previous comment - avoiding the "TBDs".

  Updated.

> 367      Note that in some cases, calculating the expected output of ISAAC
> 368      will result in the creation of a new "page" of 256 numbers.  This
> 369      process will be irreversible, and will destroy the current "page".
> 370      As a result, if the generation of a new output will create a new
> 371      "page", the receiving party MUST save a copy of the entire ISAAC
> 372      state before proceeding with this calculation.  If the outputs
> 373      match, then the saved copy can be discarded, and the new ISAAC
> 374      state is used.  If the outputs do not match, then the saved copy
> 375      MUST be restored, and the modified copy discarded, or cached for
> 376      later use.
> 
> <major> Can we have some text in section 4 to describe this thing with "pages"
> so the reader has that context before they come to this section? IOW 
> describe the term ISAAC Page and how these pages are generated and used.
> Then the text in this section can be simplified. We can also use the term
> ISAAC Page without any quotes? Taking it further (as you will see in further 
> comments),
> please consider describing and specifying all ISAAC working and procedures
> up front.

  I've added some text near the front of the document which explains this.

> 404   bfd.MetKeyIsaacRcvAuthData:
> 405      A data structure which contains the ISAAC data for the received
> 406      Auth Type method.
> 
> <minor> Is this data structure the figure in section 10? If so, please put a
> forward reference to it. Same goes for Xmit.

  No, it's not.  It's the internal data structures defined by the ISAAC method. 
 The figure in Section 10 is just defining the seed for the CSPRNG.

  I'll clarify the text.

> 439   A particular Secret Key is identified via the Auth Key ID field.
> 440   This Auth Key ID is either placed in the packet by the sender, or
> 441   verified by the receiver.  Meticulous Keyed ISAAC Authentication
> 442   permits systems to have multiple Secret Keys configured, but we do
> 443   not discuss how those keys are managed or used.  We do, however,
> 
> <minor> Please find all usage of "we" in this document and replace it with
> something more suitable for an RFC. E.g. use "this document".

  I think this is a stylistic choice.  Personally, I find it difficult to read 
neutral / non-personal text.

> 454   For interoperability, the management interface by which the key is
> 455   configured MUST accept ASCII strings, and SHOULD also allow for the
> 
> <major> Is the terminating \0 of the ASCII string considered as part of the 
> seed
>  or not?

  No.  Implementation representations of strings shouldn't affect protocol 
operations.  I'll add some text in Section 10, where the seeding data structure 
is define.

> 475   There is no negotiation as to when authentication switches from the
> 476   original type, to using Meticulous Keyed ISAAC.  The sender simply
> 477   begins authentication with a relevant Auth-Type, and with the
> 478   Optimized Authentication Mode field set to 1.  When the sender
> 479   switches to using using Meticulous Keyed ISAAC Authentication, it
> 480   sets the Optimized Authentication Mode field to 2, and starts
> 481   performing the ISAAC calculations as described here.
> 
> <major> If the switch to ISAAC is not strictly specified, and implementations
> are free to switch back/forth whenever they desire, does that not open up a
> security hole where an attacker could exploit these transitions themselves or
> cause a transition?

  How?  If the packets are authenticated, the only possible attack is for the 
attacker to cause packets to be dropped.

> Why not mandate that switch to/from happen immediately
> after a state transition? Better still why not all this is specified in the
> optimizing-auth draft and this document only points to that spec? Am I
> missing something?

  I'll add some text clarifying this issue.

> 517   The ISAAC PRNG is initialized by setting all internal variables and
> 518   data structures to zero (0).  The PRNG is then seeded by using the
> 519   the following structure:
> 
> 521       0                   1                   2                   3
> 522       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 523      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 524      |                             Seed                              |
> 525      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 526      |                       Your Discriminator                      |
> 527      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 528      |                          Secret Key ...            |  Counter |
> 529      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> <minor> Please insert a figure name and reference for the picture. It will be 
> a
> helpful reference. This goes for all figures in the document.

  OK.  Will do.

> 534   systems, the field are taken from the received packet.  The length of
> 535   the Secret Key MUST be 1016 octets or less.  The Counter field is use
> 
> <major> This conflicts with the limits of the secret key in section 8. Or am I
>  missing something?

  Section 8 says MUST be at least 8 octets, and SHOULD be no more than 128 
octets.  This text says MUST be no more than 10-16 octets.

  So I don't see any conflict here.  I'll clarify that this final limitation is 
due to the size of the buffer used to seed ISAAC.

> 541   Counter field is incremented, starting from zero (0).  This process
> 542   may finish with a partial copy of the above structure, in which case
> 543   only a prefix of the structure is copied.
> 
> <major> What is a "prefix of the structure"? Do you mean how much ever part of
> the structure that can be accomodated in what remains of that page?

  Yes,

> 545   Once the ISAAC "page" is initialized, the data is processed throught
> 
> <nit> s/throught/through ... there are several other spelling mistakes that a 
> spell check 
> can help find/fix easily.

  OK.  I'll address that.

> 546   the "randinit()" function of ISAAC.  Pseudo-random numbers are then
> 547   produced 32 bits at a time by calling the "isaac()" function.
> 
> <minor> Please put a reference to ISAAC here.

  OK.

> 555   The following figure gives Seed and Your-Discriminator as 32-bit hex
> 
> <nit> expand hex to hexadecimal

  OK.

> 556   values, and the Secret Key as an eleven-character string.  The
> 557   subsequent figure shows the first eight Sequence numbers and
> 
> <minor> There is no figure. It would be better to either put this example blob
> in a figure or suitably formatted so it is distinguished from the normative
> text of the document.

  Ok.

> 561   Seed    0x0bfd5eed
> 562   Y-Disc  0x4002d15c
> 563   Key     RFC5880June
> 
> <major> Isn't this Key the Secret Key? If so, please correct.

  Yes.

> 575   Note that this construct requires that the "Your Discriminator" field
> 576   not change during a session.  However, it does allow the "My
> 577   Discriminator" field to change as permitted by RFC5880 Section 6.3
> 578   [RFC5880]
> 
> <major> If My Discriminator is changed, then does that not result in the 
> change of "Your Discriminator" for the other end? RFC5880 says the
> implications of discriminator change are outside scope. Perhaps the same can 
> be
> said here or better still make it a prerequisite that there is no
> discriminator change while the session is in UP state (somewhat similar to
> what is indicated for the Seed a couple of paragraphs below)?

  The discriminators are controlled by the sending party.  So if My 
Descriminator changes, that has no impact on the Your Discriminator field.  
There is no requirement in RFC5880 that the discriminators change at the same 
time.

> 581   controlled by each party in a BFD session.  For security, each
> 582   implemention SHOULD randomize their discrimator fields at the start
> 583   of a session, as discussed in RFC5880 Section 10 [RFC5880].
> 
> <nit> fix redundant use of RFC5880 above.
> 
  OK.

> 650   If, however, the packet's Sequence Number differs from the expected
> 651   value, then the difference "N" indicates how many packets were lost.
> 652   The receiver then has to search through the first "N" Auth Keys
> 653   derived from its calculated ISAAC state in order to find one which
> 654   matches.  If no key matches the Auth Key in the packets, the packet
> 655   is deemed to be inauthentic, and is discarded.
> 
> <major> If the sequence is incrementing monotonically, then why is there a
> need to look at multiple records instead of only the offset between the
> previous and the received number? Perhaps there is something here that I
> have not understood ...

  You're right, it can just find the correct key directly.  I'll update the 
text.

> 657   If a calculated key at index "I" does match the Auth Key in the
> 
> <major> what is this index "I"?

  I'll clarify the text.

> 659   this value.  The bfd.MetKeyIsaacRcvAuthBase field is then initialized
> 660   to contain the value of bfd.RcvAuthSeq, minus the value of
> 661   bfd.MetKeyIsaacRcvAuthIndex.  This process allows the pseudo-random
> 662   stream to be re-synchronized in the event of lost packets.
> 
> <question> Isn't it problematic to do this? What if it is an attack? Or, 
> likely I 
> may be missing something ...

  Why would there be an issue?  If the Auth Key matches the expected key for 
the given Sequence number, then the packet is authentic

  i.e. an attacker would need to be able to forge the Auth Key in order to 
force an erroneous synchronization.

  The BFD state machine ensures that the packets can't be replayed, as the 
Sequence Number is increasing by 1 for each packet.  So knowledge of one Auth 
Key doesn't help an attacker predict the next one.

> 669   This document does not make provisions for dealing with the case of
> 670   losing more than 256 packets.  Implementors should limit the value of
> 671   "Detect Multi" to a small number in order to keep the number of lost
> 672   packets within an acceptable limit.
> 
> <major> Isn't this a MUST? Also, this is something for the operator and is
> better if placed in an operational considerations section of its own for
> proper focus. Same goes for other similar considerations.

  OK.  I'll move the relevant text to the Operation section.

> 674 11.  Operation
> 
> <major-editorial> This is buried far too deep in the document. It will be
> easier, if this were all explained towards the start or at an appropriate
> point in the document. Assume that the reader is familiar with BFD - what is
> new is this ISAAC thing. So, perhaps explaining that at the beginning will be
> helpful.

  I've added a section at the start which explains why ISAAC was chosen, and 
the high level theory of how ISAAC is used here.  This section then describes 
more detailed operational and implementation issues.

> 696   The receiving party can then look at the Sequence Number to determine
> 697   which particular PRNG value is being used in the packet.  By
> 698   subtracting the bfd.MetKeyIsaacAuthBase from the Sequence Number
> 699   (with possible wrapping), an expected "Index" can be derived, and a
> 700   corresponding Auth Key found.  This process thus permits the two
> 701   parties to synchronize if/when a packet or packets are lost.
> 
> <major> Please see my previous comment about index I - I assume the above is
> the description. Do consider consolidating the procedure and descriptions for
> a consistent flow for the reader.

  OK.

> 722 11.1.  Page Flipping
> 
> 724   Once all 256 Auth Keys from the current page have been used, the
> 725   "next" page is calculated by calling the isaac() function.  This
> 726   function processes the current "page" to create the "next" page, and
> 
> <minor> Is this "processes the current page" or "flushes the current page"?

  "modifies".  The internal ISAAC process involves running hashes over the 
page, and update the page contents with the results of the hash.

> 759 12.  Transition away from using ISAAC
> 
> <major> Don't we want to put a reference to 
> [I-D.ietf-bfd-optimizing-authentication]
> to indicate how the switching to/from ISAAC is done?

  Yes.  Arguably much of the text on transitioning to/from ISAAC could be in 
I-D.ietf-bfd-optimizing-authentication.

> 767   Since Meticulous Keyed ISAAC Authentication does not provide for full
> 768   packet integrity checks, it may be desirable for a party to
> 769   periodically use a strong Auth Type.  The switch to a different Auth
> 770   Type can be done at any time during a session.  The different Auth
> 771   Type can signal that the session is still in the Up state.
> 
> <major> This seems somewhat conflicting with the previous section where the 
> start
> of ISAAC auth mode was not immediately after a state change or the use of
> strong Auth Type. What is stated here is much more desirable than doing a
> switch where there is no auth mode enabled.

  I've added some text clarifying this transition, and updated the earlier text.

> 778   The nature of Meticulous Keyed ISAAC Authentication means that there
> 779   is no issue with this switch, so long as it is for a small number of
> 780   packets.  From the point of view of the Meticulous Keyed ISAAC state
> 781   machine, this switch can be handled similarly to a lost packet.  The
> 782   state machine simply notices that instead of Sequence Number value
> 783   being one more than the last value used for ISAAC, it is larger by
> 784   two.  The ISAAC state machine then calculates the index into the
> 785   current "page", and uses the found number to validate (or send) the
> 786   Auth Key.
> 
> <major> I find this strange and one is expected to retain the ISAAC pages even
> when the auth mode changes. Based on previous section, the receiver
> initializes when it receives the first packet with the ISAAC auth mode. Am I
> missing something? The clean start also enables one to change the seed,
> discriminator, etc.

  The optimized authentication mode draft clarifies this issue (or should).  
The BFD Authentication Type is not changing during this switch.  Instead, the 
Optimized Authentication Mode is changing.  And then only changing for a small 
number of packets.

  Since the session state doesn't change, the ISAAC RNG is not re-seeded.  I've 
added text here and elsewhere to clarify this issue.

> 788   If the non-ISAAC Auth Type instead runs for extended periods of time,
> 789   then the ISAAC process must continue "in the background" in order to
> 790   maintain synchronization.  This process is needed because this method
> 791   does not provide for a way to reinitialize the ISAAC method with new
> 792   Seed value.
> 
> <major> Seems like a contradiction to me ... see previous comment.

  I'll clarify the text.

> 794 13.  IANA Considerations
> 
> 796   For IANA Consideration, please refer to the IANA Considerations
> 797   section of Optimizing BFD Authentication
> 798   [I-D.ietf-bfd-optimizing-authentication].
> 
> 800   Note to RFC Editor: this section may be removed on publication as an
> 801   RFC.
> 
> <major> I am not sure why this complication is needed with creating this IANA
> dependency with the optimizing authentication draft. Please just ask for
> allocating the code points 7 and 8 for the two new ISAAC auth types in this 
> document. What would be the issue with doing so?

  We'll update the documents to clarify IANA issues.

> 818   The security of this proposal depends strongly on ISAAC.  This
> 819   generator has been analyzed for almost three decades, and has not
> 820   been broken.  Research shows that there are few other CSRNGs which
> 821   are as simple and as fast as ISAAC.  For example, many other
> 822   generators are based on AES, which is infeasibe for resource
> 823   constrained systems.
> 
> <minor> perhaps provide some BFD offload context here?

  I think this issue is better addressed in the optimized authentication draft.

> 825   In a keyed algorithm, the key is shared between the two systems.
> 826   Distribution of this key to all the systems at the same time can be
> 827   quite a cumbersome task.  BFD sessions running a fast rate may
> 828   require these keys to be refreshed often, which poses a further
> 829   challenge.  Therefore, it is difficult to change the keys during the
> 830   operation of a BFD session without affecting the stability of the BFD
> 831   session.  Therefore, it is recommended to administratively disable
> 832   the BFD session before changing the keys.
> 
> <minor> Isn't this an operational consideration?

  Sure.

> 838   The Auth Type method defined here allows the BFD end-points to detect
> 839   a malicious packet, as the calculated hash value will not match the
> 840   value found in the packet.  The behavior of the session, when such a
> 841   packet is detected, is based on the implementation.  A flood of such
> 842   malicious packets may cause a BFD session to be operationally down.
> 
> <major> Could you please elaborate on why this would be the case? And why the
> behavior is based on the implementation when this spec says that those 
> packets are
> to be discarded?

  That text is left over from earlier revisions.   Bad packets will simply be 
ignored.  I'll update the text.

> 871   However, the usual actual attack which we are protecting BFD from is
> 872   availability.  That is, the attacker is trying to shut down then
> 873   connection when the attacked parties are trying to keep it up.  As a
> 874   result, the attacks here seem to be irrelevant in practice.
> 
> <major> I don't buy this argument, giving the false impression that a link is
> up when it is down will result in dropping of packets and is equally harmful.
> Is it necessary to call this "irrelevant"?

  I'll rephrase.  The attacker can't artificially keep the connection up, as 
the attacker can't determine the correct ISAAC number to use.

> 925 16.2.  Informative References
> 
> 927   [ISAAC]    Jenkins, R. J., "ISAAC",
> 928              http://www.burtleburtle.net/bob/rand/isaac.html, 1996.
> 
> 930   [ISAAC_]   Aumasson, J-P., "On the pseudo-random generator ISAAC",
> 931              https://eprint.iacr.org/2006/438.pdf, 2006.
> 
> <major> The first reference has got to be normative.

  Sure.

> Additionally,
> the reference is to a private blog site and its availability is suspect
> post publication as an RFC. Please see if there is a better/stable reference. 
> Another option is if the actual algorithm or code could be placed in the 
> appendix (subject to copyright considerations - I am not an expert on this
> and so we will need to consult - but if the inventor could agree for the
> portion of the paper/code can be included in the appendix). There may be 
> other 
> options/ways. The 2nd one is at least an organization and it is
> informational - so I am not worried about that.  

  As noted earlier, other RFCs also reference the authors web pages, and ISAAC. 
 So I think we're fine there.

  Alan DeKok.

Reply via email to