Re: [sfc] Secdir early review of draft-ietf-sfc-nsh-integrity-01

[[e-mail]]

Hi Steve, 

Many thanks for the review.

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Steve Hanna via Datatracker [mailto:[e-mail]]
> Envoyé : jeudi 24 décembre 2020 19:22
> À : [e-mail]
> Cc : [e-mail]; [e-mail]
> Objet : Secdir early review of draft-ietf-sfc-nsh-integrity-01
> 
> Reviewer: Steve Hanna
> Review result: Has Issues
> 
> 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.
> 
> This document describes adds integrity and optional encryption of
> sensitive metadata directly to the Network Service Header (NSH)
> protocol defined in RFC 8300, thus reducing or eliminating several
> attack vectors against Service Function Chaining (SFC). The document
> is well written and seems adequate for the goals articulated here
> and elsewhere in the SFC document suite.

[Med] Thanks. 

 However, I have some
> issues, questions, and nits.

[Med] This was exactly what we were looking for! Thanks. 

> 
> Note that I have not previously worked with SFC. In the last few
> days, I have read the documents on this document so I am fairly
> confident that I understand the relevant security aspects.
> 
> ISSUES and QUESTIONS:
> Why include a MUST in section 9.2? Isn't that already covered
> earlier (in the last sentence of section 7.5)?

[Med] Fair point. Will reword to avoid redundant use of normative language. 

> 
> The Timestamp field is supposed to handle replay attacks. However,
> this permits unlimited replays within the Delta interval. Is that
> acceptable?

[Med] This is a discussion we had among the authors and we believe that 2 seconds is acceptable especially that the timestamp is set by the classifier and kept along an chain (except, if reclassification happens). That value can be decreased if required as a function of a local deployment.

FWY, we considered the use of sequence numbers in the -00 but abandoned that design because some SF instances may not receive all the packets of a given flow. That is typically the case of load-balancing or the so-called SFC Offloads. 

> 
> What is the ? operator in section 7.4 supposed to connote?
> Subtraction seems like a better choice.

[Med] We are calculating the diff between TS and TSrt. Will clarify in the next iteration. 

> 
> Is the Timestamp field only set by the first imposer in the SFP or
> should it be updated whenever an imposer changes the MAC? This
> should be documented somewhere, maybe in section 7.4.

[Med] The timestamp is left untouched as long a packet progresses in a service path. It will be updated when we do reclassification, though. Will add text to make this clear. 

> 
> The threat model described in draft-arkko-farrell-arch-model-t-04
> includes compromised nodes. The security considerations section of
> this document should describe how and to what extent compromised
> nodes are handled by the protections provided by this document and
> what residual risks remain.

[Med] A compromised entity can drop packets or bypass an SF is discussed in the draft:   

   The attacks discussed in [I-D.nguyen-sfc-security-architecture] are
   handled owing to the solution specified in this document, except for
   attacks dropping packets.  Such attacks can be detected relying upon
   statistical analysis; such analysis is out of scope of this document.
   Also, if SFFs are not involved in the integrity checks, a misbehaving
   SFF which decrements SI while this should be done by an SF (SF bypass
   attack) will be detected by an upstream SF because the integrity
   check will fail.

Other damage may be induced by compromised devices. Will see what we can add more.  

> 
> The Security Considerations section should explicitly acknowledge
> that authentication is not provided by this method.
> 
> What does this statement mean?
>         • If HMAC algorithm is used, IV length is set to zero.
> Don't all the current algorithms use HMAC?
> 

[Med] Actually we wanted to refer to this mode: 

   o  If the Context Headers are not encrypted, the Hashed Message
      Authentication Mode (HMAC) algorithm discussed in [RFC4868] is
      used to integrity protect the NSH data.

> What is the expected behavior if these Context Headers are missing?
> The last paragraph at the bottom of page 18 seems to be ambiguous on
> this topic, with the first sentence saying that this "SHOULD be
> logged locally" 

[Med] That one is from the perspective of the entity (e.g., classifier) that fails to inject the header at the first place. 

while the last sentence says that this "MUST cause
> that packet to be discarded".

[Med] This one is the behavior for on-path entities. 

 Probably this is clear to the writer
> but not to this reader!

[Med] Point taken. Will reword. 

> 
> NITS:

[Med] All good points. Will be fixed. Thanks. 

> At the top of page 6, "unecrypted" should be "unencrypted".
> 
> In the last line of page 18, "depend" should be "depending".
> 
> Just below Figure 9 on page 20, a comma is needed after "doing so".
> 
> In the second paragraph of section 7.5, "successfuly" should be
> spelled "successfully".
> 
> At the end of the first paragraph of section 9, change the sentence
> from:
>         • Also, that section indicates that metadata considerations
> that
>         operators can take into account when using NSH are discussed
> in
>         [RFC8165].
> to
>         • Also, that section indicates that [RFC8165] discusses
> metadata
>         considerations that operators can take into account when
> using NSH.
> 
> The last sentence of the third paragraph of section 9 recommends
> that "the next key identifier" be distributed long before the key is
> changed. This should say "the next key identifier and associated
> keying material".
> 
> In the second paragraph of section 9.1, "domain be able" should be
> "domain should be able".
> 
> 


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.