Re: [sfc] Benjamin Kaduk's Discuss on draft-ietf-sfc-nsh-tlv-09: (with DISCUSS and COMMENT)

[Donald Eastlake <[e-mail]>]

Hi,

As the lowly last listed author :-) I have done a pass over the
document with particular attention to Ben's DISCUSS. My apologies for
not reviewing again earlier. See comments below at <de> and also the
attached mark-up which contains a number of minor editorial
suggestions in addition to the changes that I spell out below.


On Mon, Mar 21, 2022 at 5:59 AM Benjamin Kaduk <[e-mail]> wrote:
Hi Yuehua,

My apologies for not replying until now.  I think I saw the updated
versions of the draft and did not notice that there were still some active
topics in the email thread (not reflected in the draft yet).

On Fri, Dec 03, 2021 at 05:30:53PM +0800, [e-mail] wrote:
> Dear Ben,
> Thank you. please see the comment resolution inline with Yuehua>>
>
> To Martin,  I really appreciate if you could provide comments to Yuehua-2
and Yuehau-6.
>
> Best Regards,
> Yuehua Wei
> ZTE Corporation
> ------------------原始邮件------------------
> 发件人：BenjaminKadukviaDatatracker
> 收件人：The IESG;
> 抄送人：[e-mail];[e-mail];[e-mail];
[e-mail];
> 日 期 ：2021年11月30日 02:59
> 主 题 ：[sfc] Benjamin Kaduk's Discuss on draft-ietf-sfc-nsh-tlv-09: (with
DISCUSS and COMMENT)
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-sfc-nsh-tlv-09: Discuss
>
> ...
>
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> (1) RFC 8300 is pretty clear that "Metadata privacy and security
> considerations are a matter for the documents that define metadata
> format."  Some of the metadata context headers defined in this document
> clearly have privacy considerations that need to be documented (e.g.,
> policy ID and source/destination group serve to concretely identify
> flows that are related in some way), though some may not have much that
> needs documenting (e.g., the forwarding context metadata seems to just
> be extracting out information that is already present in the packet
> being wrapped).  Regardless, we need to have some discussion of the
> privacy and security considerations of the new metadata context headers,
> even if that is just "no new considerations" for some of them.
>
> Yuehua-1>>Thank you, I update the section to ver10 per you, Stig and
John's comments.  please take a look and provide further comments

Unfortunately, I don't think that even the text in the -13 is providing
what is needed here.  (It is good text to have, but is generic to attacks
on context headers in general, and authentication of context headers in
general.)  My expectation is that for each of the seven context headers
that this document defines, we take a careful look at them in turn and
document any privacy and/or security considerations that apply to the
information carried in that specific context header.  (In some cases we may
also have to consider the encoding used, in addition to the information
content, but I do not remember that being relevant here.)  This could, for
example, take the form of a table or list that covers each context header
in turn.  (If, after having done the work, it ends up being a list of
mostly "no special considerations", we might consider formatting it
differently, but we definitely need to do the analysis on each context
header in turn.)

<de> Towards the end of this message, I suggest an addition to the
Security Considerations section to respond to the comment above.

> (2) I think we need to discuss the Flow ID context header further.  Is
> it intended to just be a container to hold a flow identifier already
> present in the contained packet (such as the IPv6 Flow Label or MPLS
> Entropy Label that are called out), or can it also be used to apply a
> new flow identifier at the SFC layer?
>
> The named examples of a flow ID are both 20 bits long; if that is an
> exhaustive listing, shouldn't we update the figure accordingly (to
> include Length=3, four leading bits of padding, and a trailing byte of
> padding)?  If that is not an exhaustive listing and longer flow
> identifiers are expected, how do we know what length of flow identifier
> is being conveyed?
>
> Yuehua-2>>You raised a good question. I prefer to add Context Type (CT)
to distinguish different flow IDs. I'd appreciate your advice.

An explicit Context Type to indicate the specific Flow ID used seems like
it will be very helpful for extensibility.  That would also allow each
context type to specify how long in bits the useful Flow ID value is, since
the NSH metadata framing only gives a length in bytes.  If there was no
in-band context type, a recipient would have to rely on out-of-band
configuration to know how much padding is present, which is not very
interoperable.

<de> I believe the above is resolved.

> (3) If we are to allow for specifying the "logical grouping of source
and/or
> destination objects" in §4.6 (emphasis on "and/or"), but the context
> header always conveys both a source group and dest group field, do we
> need to reserve a dedicated value for "no group information specified"?
>
> Yuehua-3>>Very good observation. Shall we state that if there is   "no
group information specified" for the source or dest field, the field MUST
be sent as zero and ignored on receipt

Yes, please!

<de> I believe the above is resolved.

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Section 1
>
> This document does not address metadata usage, updating/chaining of
> metadata, or other SFP functions.  Those topics are described in
> [RFC8300].
>
> I'm not entirely sure what is meant by "chaining of metadata" (unless
> it's just a synonym for "updating of metadata"?), and having reviewed
> all 18 instances of "chain" and 88 instances of "metadata" in RFC 8300,
> I am not sure which part you're intending to refer to by it.  Could you
> please point to a more specific part of RFC 8300 (at least in this email
> thread for now, not necessarily in a document update)?
>
> Yuehua-4>> RFC8300 doesn't use “chain”to discribe multiple metadata.
> it mentions "If multiple mandatory-to-process Context Headers are
required for a   given SFP" and "If multiple instances of the same metadata
are included in an NSH   packet" in RFC8300 section 2.5.1
> Further comments are welcome

Mostly my comment here is that I don't know what you mean by "chaining of
metadata".  If it's just that there might be interdependencies or relations
between two pieces of metadata in the same packet, we could clarify the
text in one way; if it's relating to updating (modifying) a given piece of
metadata along the path of the packet, we would do something else; etc.

<de> I think we can just drop "chaining".

> Section 4.1
>
> I suggest putting a context (forgive the pun) indicator in the figure
> legends, e.g., "Figure 4: Forwarding Context - 2 (QinQ)".  The
> information is already available elsewhere, but having it in the caption
> helps focus the reader on the important/relevant part of the figure.
>
> Yuehua-5>> Accepted, will update to ver10

<de> I do not think there is any need for the hyphen and number after
"Forwarding Context" in any of the Figure captions. These numbers are
different from the "CT=" numbers in the figures and these numbers are
never referenced anywhere. Further, I think it reads better to move
the parentheticals at the end of the captions to the front, so "VLAN
Forwarding Context" instead of "Forwarding Context - 1(VLAN)", etc.

<de> After Figure 7 it says the CT fields "defines the length and
interpretation" but the length is specified by the preceding Length
field so "the length and" should be deleted.

<de> The reference for VNI given for CT 0x3 is only RFC 8926, Geneve.
But there is also a 24-bit VNI in VXLAN (RFC 7248) and NVGRE (RFC
7637) although in NVGRE it is called "VSID". Should these references
also be included? Note that VXLAN is specifically mentioned at the
beginning of Section 4.1.

Thanks!
...

<de> Section 4.2: Figure 9: I think when the Length is variable, there
is no need to say "Length = var". If the field is simply labeled
"Length" and explained below, that implies it is variable. Same for
other similar figures.

<de> Section 4.2: It says that the Tenant ID is a value "pointing to
Orchestration system-generated tenant identifier". Is it really an
identifier pointing to an identifier? Perhaps this should say
something like "indicating an Orchestration system generated tenant
identity".

> Section 4.3

<de> "Network" is kind of generic. I think it would add clarity to
indicate in the text that this is talking about the node that is the
one at which the packet entered the SFC-enabled domain, unless I'm
confused about that.

> It might be interesting to say something about when the SPI itself
> suffices to identify the ingress node vs. when this metadata context
> header is needed.

I think it would be reasonable to add a sentence about this.

> Node ID: Represents an opaque value of the ingress network node
> ID.  The structure and semantics of this field are deployment
> specific.  For example, Node ID may be a 4 bytes IPv4 address Node
> ID, or a 16 bytes IPv6 address Node ID, or a 6 bytes MAC address,
> or 8 bytes MAC address (EUI-64), etc,.
>
> There seems to be some dissonance between saying this field is "an
> opaque value" and also saying that it might be an IP or MAC address.  In
> the vein of Francesca's Discuss point, I don't think we can have
> interoperability if the NSH implementation is expected to process this
> as IP/MAC address information (as opposed to just an opaque identifier).
>
> Yuehua-6>> it has relation to
>
> Section 8.2
>
> URLs for [GROUPBASEDPOLICY] and [GROUPPOLICY] would be helpful.
> Yuehua-7>> Accepted, will fix the problem to ver10

<de> Section 4.4: I find it a little unclear as to whether this is
supposed to identify the specific source interface or the type of
source interface. Is it a value like the values listed from IANAifType
or is it something like a physical/logical/virtual port number on a
switch/router? I think it is probably supposed to be a port number but
this could be clearer.

<de> Section 4.5: As above, delete "the length and" in the description
of CT since that function is provided by the Length field.

<de> Security Considerations:

"the MAC Context Header" ->
"the MAC and Encrypted Metadata Context Header [RFC9145]".

<de> I suggest added material at the end of the Security
Considerations Section as below. This should be checked as I am a bit
uncertain about some things...


   The security and privacy considerations for the 7 types of context
   header specified above are discussed below. Since NSH ignorant SFs
   will never see the NSH, then even if they are malign, they cannot
   compromise security or privacy based on the NSH or any of these
   context headers, although they could cause compromise based on the
   rest of the packet. To the extent that any of these headers is
   included when it would be unneeded or have no effect, they provide
   a covert channel for the entity adding the context header to
   communicate a limited amount of arbitrary information to downstream
   entities within the SFC-enabled domain.

5.1 Forwarding Context

   All of the Forwarding Context variants specified in this document
   (those with CT values between 0 and 4) merely repeat a field that
   is available in the packet encapsulated by the NSH. These variants
   repeat that field in the NSH for convenience. Thus, there are no
   special security or privacy considerations in these cases. Any
   future new values of CT for the Forwarding Context must specify the
   security and privacy considerations for those extensions.

5.2 Tenant Identifier

   The Tenant ID indicates the tenant to which traffic belongs and
   might be used to tie together and correlate packets for a tenant
   that some monitoring function could not otherwise group especially
   if other possible identifiers were being randomized. As such, it
   may reduce security by facilitating traffic analysis but only within
   the SFC-enabled domain where this context header is present in
   packets.

5.3 Ingress Network Node Information

   The SFC-enabled domain manager normally operates the initial
   ingress / classifier node and is thus potentially aware of the
   information provided by this context header. Furthermore, in many
   cases the SPI that will be present in the NSH identifies or closely
   constrains the ingress node. Also, in most cases, it is anticipated
   that many entities will be sending packets into an SFC-enabled
   domain through the same ingress node. Thus, under most
   circumstances, this context header is expected to weaken security
   and privacy to only a minor extent and only within the SFC-enabled
   domain.

5.4 Ingress Node Source Information

   This context header is likely to be meaningless unless the Ingress
   Network Node Information context header is also present. When that
   node information header is present, this source information header
   provides a more fine-grained view of the source by identifying not
   just the initial ingress / classifier node but also the port of
   that node on which the data arrived. Thus, it is more likely to
   identify a specific source entity or at least to more tightly
   constrain the set of possible source entities, than just the node
   information header. As a result, inclusion of this context header
   with the node information context header is potentially a greater
   threat to security and privacy than the node information header
   alone but this threat is still constrained to the SFC-enabled
   domain.

5.5 Flow ID

   As in Section 5.1 above, the variations of this context header
   specified in this document simply repeat fields already available
   in the packet and thus have no special security or privacy
   considerations. Any future new values of CT for the Flow ID must
   specify the security and privacy considerations for those
   extensions.

5.6 Source and/or Destination Groups

   This context header provides additional information that might help
   identify the source and/or destination of packets. Depending on the
   granularity of the groups, it could either (1) distinguish packets
   as part of flows from and/or to objects where those flows could not
   otherwise be easily distinguished but appear to be part of one or
   fewer flows or (2) group packet flows that are from and/or to an
   object where those flows could not otherwise be easily grouped for
   analysis or whatever. Thus, the presence of this context header with
   non-zero source and/or destination groups can, within the
   SFC-enabled domain, erode security and privacy to an extent that
   depends on the details of the grouping.

5.7 Policy Identifier

    This context header carries and identifier that nodes in the
    SFC-enabled domain can use to look up policy to potentially
    influence their actions with regard to the packet carrying this
    header. If there are no such action decisions, then the header
    should not be included. If there are such decisions, the information on
    which they are to be based needs to be included somewhere in the
    packet. There is no reason for inclusion in this context header to
    have any security or privacy considerations that would not apply
    to any other plaintext way of including such information. It may
    provide additional information to help identify a flow of data for
    analysis.


Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   [phone] (cell)
 2386 Panoramic Circle, Apopka, FL 32703 USA
 [e-mail]