[sfc] Roman Danyliw's Discuss on draft-ietf-sfc-nsh-integrity-06: (with DISCUSS and COMMENT)

[Roman Danyliw via Datatracker <[e-mail]>]

Roman Danyliw has entered the following ballot position for
draft-ietf-sfc-nsh-integrity-06: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh-integrity/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

** Section 4.6.  This section explains that an upper NSH can be encapsulated in
a lower NSH, and that “the Upper-NSH information is carried along the
lower-level chain without modification.”  I read this to mean that the upper
and lower NSH can independently be protected with different keys.  The text
even helpfully points out that “Keying material used at the upper-level domain
SHOULD NOT be the same as the one used by a lower-level domain.”  Such a
construct suggests that there are multiple MAC/Encrypted Metadata context
headers, one for the upper and another for the lower.  However, Section 7.1
later notes that “Only one instance of "MAC and Encrypted Metadata" Context
Header (Section 5) is allowed.”  This seems like conflict.  What am I missing?

** Section 7.2.  On computing the HMAC in an integrity only situation:

-- This section defines the MAC as “T = HMAC-SHA-256-128(MAC_KEY, A)”. 
Previously, A was defined as the Additional Authenticated Data (per Section
4.2).  Since this isn’t the AEAD use case, there is no A. It seems that this
should be something closer to: “T = HMAC-SHA-256-128(MAC_KEY, <part of the
packet you want to hash>)”.

-- The text would benefit from a description on how to serialize the packet for
hashing.  For example, Figure 6 and 7 are helpful logical descriptions of the
integrity scope.  However, the MAC field itself is depicted as part of the what
should get hashed.  Should that field be zeroed out? Removed?

** Section 9.
   The attacks discussed in [I-D.nguyen-sfc-security-architecture] are
   handled owing to the solution specified in this document, except for
   attacks dropping packets.

The above reference highlights the following attackers – “There are many types
of compromised switches attack: packet dropping, packet duplicating, packet
manipulating, incorrect forwarding,  eavesdropping, weight adjusting,
man-in-the-middle, state-spoofing, control-channel hijacking, etc.”  Per the
security services in this document, it doesn’t seem like all are mitigated by
this draft as described above:

-- packet dropping = noted as not being handled

-- packet manipulating, eavesdropping, weight adjusting, man-in the-middle,
state-spoofing, and control-channel hijacking = appear to be handled if both
security services are applied

-- packet duplicating = this draft doesn’t not provide a standardized approach
for mitigating this issue

-- incorrect forwarding = doesn’t appear to be mitigated.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

** Section 1.
   Thus, the NSH
   does not have to rely upon an underlying transport encapsulation for
   security and confidentiality.

Isn’t confidentiality just a subset of the provided security properties, or is
something else intended?  Recommend s/for security and confidentiality/for
security/

** Section 2.  Per “Key Identifier:  Is used to identify and deliver keys to
authorized entities.  See, for example, 'kid' usage in [RFC7635]”. How does a
key identifier “delivery keys”?  Doesn’t another protocol provide the delivery
using the kid as an identifier?

** Section 7.2.  How does one serialize the packet data to construct A?  Figure
7 is the closest guidance but it includes the MAC itself.

** Section 7.4.
   … legitimate duplicate packets will be
   tagged by NSH-aware nodes involved in that segment as replay packets
   unless sufficient entropy is added to the duplicate packet.

What is the prescribed mechanism to add entropy?

** Section 7.5
   When an SFC data plane element receives an NSH packet, it MUST first
   ensure that a "MAC and Encrypted Metadata" Context Header is
   included.

I was under the impression that the security protections specified in this
document were optional in the overall SFC architecture.  Is there some caveat
to add to this section to reminder readers?  Perhaps,  “When an SFC data plan
element conforms to this specification, it MUST first …”

** Section 7.5 and 7.6.  What is the error processing for receiving a packet
with an unknown key identifier?

** Section 9. Section 9.  Can you further clarify how is the reader supposed to
interpret the guidance in RFC4107 which frames its scope as guidelines as to be
“... use[d] by IETF working groups and protocol authors who are determining
whether to mandate automated key    management and whether manual key
management is acceptable.  Informed judgment is needed.”  Does this document
take a position on automated vs. manual key management?

** Section 9.  Per “The secret key (K) must have an expiration time assigned …”:

(I suspect many of these issues are out of scope …)

-- who assigns that expiration time (the key management system)?

-- who knows about that expiration time, is it both the key management system
and the participating nodes?  If the nodes know the expiration time, should
they refuse to decrypt a message with an expired key?  This check of expiration
is not the Section 7.5 or 7.6 procedures.

** Section 9.  In the spirit of inclusive language, please s/A
man-in-the-middle/An on-path-attacker/

** Section 9.
   In an SFC-enabled domain where the above attacks are possible, (1)
   NSH data MUST be integrity-protected and replay-protected, and (2)
   privacy-sensitive NSH metadata MUST be encrypted for confidentiality
   preservation purposes.

Related to the COMMENTs raised by Lars and Alvaro.

It’s my understanding that the security protections here are optional for the
SFC architecture – is this correct?  If so, I don’t follow the above guidance. 
With the exception of “data carried in the context headers having privacy
sensitive information”, all SFC-enabled domains have the potential for the
above described attacks.  It seems like this document is helpfully making
integrity, replay and encrypted meta-data a requirement for the entire SFC
architecture (without explicitly updating RFC8300)

** Section 9.1
   An active attacker can potentially modify the Base header (e.g.,
   decrement the TTL so the next SFF in the SFP discards the NSH
   packet).  In the meantime, an active attacker can also drop NSH
   packets.

The phrase “In the meantime …” suggests that these two active attackers are
coordinating.  Both of these sentences seem to be describing independent
attackers.

==[ Editorial
** Abstract.  Typo.  This sentence doesn’t parse.
OLD
Also,
   this specification allows to encrypt sensitive metadata that is
   carried in the NSH.

NEW
Also, this specification allows for the encryption of sensitive metadata  that
is carried in the NSH.

** Section 1.  Editorial. s/to is an information/to is information/

** Section 1.  Editorial.  s/This specification fills that gap/This
specification fills that gap for SFC/

** Section 1.  Editorial.  s/This specification limits thus access/This
specification limits access/

** Section 3.  Editorial.  s/The NSH allows to share/The NSH shares/

** Section 4.1.2.  Editorial.  s/SFFs are not thus provided/SFFs are not
provided/

** Section 4.2. Editorial.  Recommend being clearer on the section – s/The
authenticated encryption algorithm defined in [RFC7518]/ The authenticated
encryption algorithm defined in Section 5.2 of [RFC7518]/; or you could name
the algorithm directly with the reference.

** Section 4.4. Editorial. s/Doing so allows to address the problem/Doing so
addresses the problem/