Early Review of draft-ietf-sfc-nsh-integrity-01
review-ietf-sfc-nsh-integrity-01-secdir-early-hanna-2020-12-24-00

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".