[secdir] Secdir last call review of draft-ietf-sfc-serviceid-header-10

[Robert Sparks via Datatracker <[e-mail]>]

Reviewer: Robert Sparks
Review result: Has Nits

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 has nits that should be addressed before publication as Proposed
Standard RFC.

Document reviewed: draft-ietf-sfc-serviceid-header-10

This document's security considerations rest on the premise that the
information it discusses will be added, transported, consumed, and removed
within a single administrative domain that is protected from eavesdroppers and
malicious on-path attackers through administrative controls. It is quite
upfront about that assumption, and the sfc documents it relies on make the same
assertions.

This document introduces a mechanism that intentionally allows identifiers
(addresses) that would normally be hidden/contained behind a nat to be shared
beyond that nat, but again, within that single administrative domain.

NITS:

There are many places where the language in the document needs to be
simplified. There are also many places where 2119 language is being used in a
way that does not make sense.

In document order:

Introduction paragraph 2, sentence 1. What does it mean to infer enforcement?
Perhaps you mean "policies can be inferred"? Please simplify this sentence.
Break the example into a separate sentence and add detail if you can. If
possible, provide a reference to what you mean by "SFC-based differentiated
traffic policies" means.

Introduction paragraph 3. This whole paragraph has complicated, paragraph
separated, phrases that should be written more simply. The next to last
sentence (at 53 words) is particularly hard to follow.

Introduction paragraph 4, last sentence. This sentence does not parse. Please
rework it.

Introduction paragraph 5. This one sentence paragraph was the most difficult in
the document for me to read. Please break it into several simpler sentences.

Section 3: at "In order to prevent interoperability issues, the data and their
format to be injected in such header SHOULD be configured to nodes authorized
to inject such headers." This is not a 2119 SHOULD. It's even not clear what
"the data" is here. I suggest something like "The identifiers carried in the
Context Headers are opaque. Nodes providing them and consuming them will be
configured with the necessary information needed see into them."

Section 3, at "Failures to inject such headers SHOULD be logged locally while a
notification alarm MAY be sent to a Control Element.  The details of sending
notification alarms (i.e., the parameters affecting the transmission of the
notification alarms depend on the information in the Context Header such as
frequency, thresholds, and content of the alarm (full header, timestamp, etc.))
SHOULD be configurable." None of these are correct use of 2119. Consider
re-framing the paragraph without using these terms.

Section 3 at "That is, to strip such Context Headers." is not a sentence.

Section 3 at the paragraph beginning "Depending on local policy". The structure
of this paragraph is very odd. Doesn't base SFC say to preserve NSH at
intermediaries? I suggest replacing this paragraph with something like "Normal
SFC intermediary behavior preserves Network Service Headers. Local policy can
require a proxy to strip ..."

Section 3 at "NSH-aware SFs MUST be capable to run additional validation
checks". This isn't 2119. Perhaps you mean to say "Be aware that local policies
may require running additional validation checks at NSH-aware SFs". The whole
paragraph can be simplified.

Section 3 at "Multiple Subscriber Identifier Context Headers MAY be present in
the NSH, each carrying a distinct opaque value but all pointing to the same
subscriber." This is a place where a 2119 MUST _would_ make sense. Say "Such
multiple headers MUST identify the same subscriber". I wonder, though, if
you're closing a door on carrying multiple identities that you'll regret later.

Section 4 first paragraph, first sentence: I suggest replacing "identifier is"
with "identifiers are"

Section 4 fifth paragraph: "defined as optional" -> "defined as an optional"

Section 4 sixth paragraph. Similar to the section 3 paragraph I point to above,
the structure here is very odd. Again, I suggest something like "Normal SFC
intermediaries will preserve Context Headers, but local policy may require a
proxy to to strip one after processing it."

Section 4 seventh paragraph: "ocnlficting"->"conflicting". I assume the default
behavior described here is specified in the base NSH docs?

Security Considerations section paragraph 4: Consider "logging the content of
... is not advised" rather than using 2119 here. The paragraph would be better
formulated if it explained the risk - SFs that _did_ use the context might be
better off not logging them in many cases as well.