Re: [Rift] RIFT security review

["Jeffrey (Zhaohui) Zhang" <[e-mail]>]

Hi Scott,

Thanks for your security review on RIFT spec. I am copying this to the RIFT mailing list.

Please see Tony's response in the email below and the revisions in https://www.ietf.org/id/draft-ietf-rift-rift-05.txt. Could you review again to make sure all your comments/concerns have been addressed?

Thanks!
Jeffrey



Juniper Internal
From: Antoni Przygienda <[e-mail]>
Sent: Thursday, April 18, 2019 12:34 PM
To: Jeffrey (Zhaohui) Zhang <[e-mail]>; [e-mail]; Bruno Rijsman <[e-mail]>
Subject: Re: RIFT security review

My comments on the security review:


I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.



The summary of the review is ready with issues



>From the abstract, this document outlines a specialized, dynamic routing protocol for Clos and fat-tree network topologies.



(should that read CLOS?)

Clos was a French mathematician in Bell Labs who invented the stuff so it's really "Clos"

Wikipedia: "Clos networks are named after Bell Labs researcher

Charles Clos, who proposed the model in 1952 as a way to overcome the

performance- and cost-related challenges of electromechanical switches then used in telephone networks."

 Following is a brief summary of comments and questions by section.



5.4.1 includes this sentence:



   The most security conscious operators will want to have full control

   over which port on which router/switch is connected to the respective

   port on the "other side", which we will call the "port-association

   model" (PAM) achievable e.g. by pairwise-key PKI.



What is "pairwise-key PKI"?

pair-wise set of private/public key, i.e. a designated key pair per port. I'll try to word it better


 Secion 5.4.2 says "Low processing overhead and efficiency messaging are also a goal."



I suggest replacing efficiency with efficient

ack

 It also says "Message privacy achieved through full encryption is a non-goal"



I suggest saying "Message confidentiality is a non-goal" instead.

ack

 Section 5.4.3

"Length of Fingerprint:  8 bits.  Length in 32-bit multiples of the

      following fingerprint not including lifetime or nonces.  It allows

      to navigate the structure when an unknown key type is present.  To

      clarify a common cornercase a fingerprint with length of 0 bits is

      presenting this field with value of 0."



Does length 0 mean no fingerprint is present (i.e. fingerprints are not provided)? I don't understand that last sentence.

yes, it does. I try to improve the wording.

 The definition for "Security Fingerprint" includes this  sentence:



"If the fingerprint is shorter than the significant bits are left aligned and remaining bits are set to 0."



I don't understand this sentence. I think you mean that      if the fingerprint bit length is not an even multiple of 32, then it is left-aligned, and the rightmost unused bits are set to 0. But that's just a guess.

yes, I try to word better.

 5.4.4

"Any implementation including RIFT security MUST generate and wrap around local nonces properly"



I see the term "nonce" used elsewhere, but because it can wrap (and therefore repeat with regularity),
I think this is a poor choice for naming this field. It seems to be more of a counter.
I think most security folks would agree that a nonce used for security purposes should,
by definition, repeat only with negligible probability.

I was under the impression that nonce is a well-known term in cryptography

https://en.wikipedia.org/wiki/Cryptographic_nonce
[https://upload.wikimedia.org/wikipedia/commons/thumb/4/4f/Nonce-cnonce-uml.svg/1200px-Nonce-cnonce-uml.svg.png]<https://en.wikipedia.org/wiki/Cryptographic_nonce>

Cryptographic nonce - Wikipedia<https://en.wikipedia.org/wiki/Cryptographic_nonce>
In cryptography, a nonce is an arbitrary number that can be used just once in a cryptographic communication. It is similar in spirit to a nonce word, hence the name.It is often a random or pseudo-random number issued in an authentication protocol to ensure that old communications cannot be reused in replay attacks.They can also be useful as initialization vectors and in cryptographic hash ...
en.wikipedia.org



 On a related note, does this really provide anti-replay protection?
Elsewhere in the document (e.g. section 5.4.4) it says that implementations could go up to
5 minutes without incrementing nonces. Can they send multiple packets with the same
nonce during this interval? If so, what prevents replay of a captured packet within that interval?



Also, because wrapping (of this 16 bit value) is supported, it's also possible that an earlier packet could be replayed (assuming the peer nonce also aligned), right? The odds of this seem low, but could the protocol/endpoint states be manipulated to improve the odds? Not sure. But if you are assuming this can't happen, this security-relevant assumption should be called out.



  1.  Correct, for efficieny purposes we open up to a 5 min window which we consider an acceptable risk per point 2
  2.  it is the combination of local and remote nonce so it's really a 32 bit number. The chance that the combination repeats is obviously very small.


 5.4.7 says



   "If an implementation supports disabling the security envelope

   requirements while sending a security envelope an implementation

   could shut down the security envelope procedures while maintaining an

   adjacency and make changes to the algorithms on both sides then re

   enable the security envelope procedures but that introduces security

   holes during the disabled period."



Aside from the fact that this needs word-smithing, should this be called out in the security
considerations section? This eeems to be saying that it's not a good idea to temporarily
maintain adjacency while disabling security, so is this a SHOULD NOT?


Will improve wording. Yes, it is a SHOULD NOT but sometimes implementations do
that to not loose adjacency and change keys easily.


 section 8.4

flodding -> flooding



section 8.4 also says



   It is expected that an

   implementation detecting too many fake losses or misorderings due to

   the attack on the number would simply suppress its further processing.



what are "fake losses"?



I am not a routing expert, so there may be additional concerns that someone better versed in routing would raise.
Will improve wording. "Fake losses" is a possible attack vector where an attacker intercepts packets, modifies the packet number  to simulate a "packet number loss/misorder" and forwards the packet on.

Please fwd' to reviewer if needed & tell me whether you want notification on new version ...

--- tony

________________________________
From: Jeffrey (Zhaohui) Zhang
Sent: Thursday, April 18, 2019 7:26 AM
To: Antoni Przygienda
Subject: RIFT security review

Hi Tony,

Please see https://datatracker.ietf.org/doc/review-ietf-rift-rift-04-secdir-early-kelly-2019-04-11/.

Thanks.
Jeffrey

Juniper Internal