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

From: Robert Sparks via Datatracker <[email-masked]>
Date: Mon, 26 Oct 2020 15:25:24 -0700

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.

---

From: Robert Sparks <[email-masked]>
Date: Mon, 26 Oct 2020 17:45:26 -0500

Heh - one typo correction below (that might have been obvious, but...)

> Introduction paragraph 3. This whole paragraph has complicated, paragraph
That last word should have been "comma"

---

From: <[email-masked]>
Date: Wed, 28 Oct 2020 13:12:11 +0000

Hi Roberts, 

Many thanks for this useful review. Much appreciated!

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-sfc-serviceid-header-11 

Cheers,
Med

> -----Message d'origine-----
> De : Robert Sparks [mailto:[email-masked]]
> Envoyé : lundi 26 octobre 2020 23:45
> À : [email-masked]
> Cc : [email-masked]; [email-masked];
> [email-masked]
> Objet : Re: [Last-Call] Secdir last call review of draft-ietf-sfc-
> serviceid-header-10
> 
> > 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.
> >
> > 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.
> >
> > (addresses) that would normally be hidden/contained behind a nat
> to be
> > shared beyond that nat, but again, within that single
> administrative domain.
> >
> > NITS:
> >
> > simplified. There are also many places where 2119 language is
> being
> > used in a way that does not make sense.
> >
> > In document order:
> >
> > 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.
> >
> > 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 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 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.
> >
> >
> >

_________________________________________________________________________________________________________________________

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.

---

From: Robert Sparks <[email-masked]>
Date: Wed, 28 Oct 2020 09:27:21 -0500

Thanks Med -

This sufficiently addresses my comments. Thanks for adding clarity with 
the edits.

I do want to ask again, though, whether you're closing off an 
opportunity with the requirement that all the identifiers belong to a 
single subscriber, and that the group made that choice explicitly rather 
than just falling into it.

I could see an alternative that looked like "allow multiple identities, 
and the use of them in inferred policies is 'is any of' ". Stretching a 
little into slightly different problem domains, consider, for example, 
the doctor that could be using his own phone number or the office phone 
number.

I'm not suggesting that allowing such multiple identities is the right 
thing to do, but I want to verify that simple alternatives like that 
were considered and rejected, and that you have a way to do them in the 
future if you want.

The text does imply that the group looked at the general problem of 
trying to capture arbitrary relationships between multiple identities, 
and decided not to solve that. If that's how it happened, you might 
consider explicitly saying that so future related work isn't guessing or 
making assumptions about the motivation here.

RjS

---

From: <[email-masked]>
Date: Wed, 28 Oct 2020 14:52:27 +0000

Re-,

Thank you. 

One of the motivation for allowing multiple identifiers, but all belong to the same subscriber, is to accommodate cases such as https://datatracker.ietf.org/doc/html/draft-ietf-sfc-use-case-mobility-09#section-3.3.2 where some service functions in a mobile network can enforce policies based on an internal IP address while others may rely on the MSISDN or the IMSI. 

As discussed in https://mailarchive.ietf.org/arch/msg/sfc/TvshkAMGpwxQrLrCZG3BNqreUg0/, because these identifiers belong to the same subscriber, stripping the identifiers is straightforward. Things would be much complex, otherwise. 

Cheers,
Med

> Envoyé : mercredi 28 octobre 2020 15:27
> À : BOUCADAIR Mohamed TGI/OLN <[email-masked]>;
> [email-masked]
> This sufficiently addresses my comments. Thanks for adding clarity
> with the edits.
> 
> opportunity with the requirement that all the identifiers belong to
> a single subscriber, and that the group made that choice explicitly
> rather than just falling into it.
> 
> I could see an alternative that looked like "allow multiple
> identities, and the use of them in inferred policies is 'is any of'
> ". Stretching a little into slightly different problem domains,
> consider, for example, the doctor that could be using his own phone
> number or the office phone number.
> 
> I'm not suggesting that allowing such multiple identities is the
> right thing to do, but I want to verify that simple alternatives
> like that were considered and rejected, and that you have a way to
> do them in the future if you want.
> 
> trying to capture arbitrary relationships between multiple
> identities, and decided not to solve that. If that's how it
> happened, you might consider explicitly saying that so future
> related work isn't guessing or making assumptions about the
> motivation here.
> 
> RjS
> 
> >>> There are many places where the language in the document needs
> to
> >> be
> >>> Perhaps you mean "policies can be inferred"? Please simplify
> this
> >> sentence.
> >>> Section 4 fifth paragraph: "defined as optional" -> "defined as
> an
> >> optional"