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

["Joel M. Halpern" <[e-mail]>]

Thank you Steve.  I expect the authors will get back to you after the 
holidays (although they have impressed me with the rapidity of their 
responses in the past.)

Yours,
Joel

On 12/24/2020 1:21 PM, Steve Hanna via Datatracker wrote:
> 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. However, I
> have some issues, questions, and nits.
> 
> 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)?
> 
> The Timestamp field is supposed to handle replay attacks. However, this permits
> unlimited replays within the Delta interval. Is that acceptable?
> 
> What is the ? operator in section 7.4 supposed to connote? Subtraction seems
> like a better choice.
> 
> 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.
> 
> 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.
> 
> 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?
> 
> 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" while the last
> sentence says that this "MUST cause that packet to be discarded". Probably this
> is clear to the writer but not to this reader!
> 
> NITS:
> 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".
> 
> 
>