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

[tirumal reddy <[e-mail]>]

Hi Ben,

Please see inline

On Thu, 15 Jul 2021 at 02:55, Benjamin Kaduk via Datatracker <
[e-mail]> wrote:

> Benjamin Kaduk 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:
> ----------------------------------------------------------------------
>
> (0) (I came to this realization rather late in my review process, so
> there may be places where the COMMENT and this discuss point are in
> disagreement; this DISCUS takes precedence.)
> I fear that the construction that separately distributes ENC_KEY and
> MAC_KEY in an attempt to achieve privilege separation is fatally flawed.
> In particular, the CBC encryption mode is a malleable encryption mode,
> in that flipping a bit of ciphertext will filp the corresponding bit of
> the next block of recovered plaintext (at the cost of completely
> garbling the recovered block containing the bit that was modified).
> Subsequent blocks are unaffected.  Typically we combine CBC mode with a
> MAC such as HMAC in order to prevent such modifications from being
> exposed as attack vectors, and while we do use HMAC here for that
> purpose, we also introduce a separate class of actors that have access
> to the HMAC key but not the encryption key.  Accordingly, those actors
> can produce a new, valid, integrity tag after making a modification to
> the ciphertext, allowing them to engage in attacks that make use of
> ciphertext malleability.  Ciphertext malleability is particularly useful
> as an attack vector when the structure of the plaintext being encrypted
> is known, and there are portions of the plaintext that the application
> will either ignore if they are garbled or are expected to be near random
> in the normal case (and thus for which garbled output does not cause
> rejection by the application).  In a SFC environment it seems highly
> likely that the structure of the plaintext will be known or guessable,
> and we don't have any real mechanisms to control what types of metadata
> go into encrypted context headers, so it seems that we must act as if we
> are exposed to this risk.
>
> While §4.3 does have a note that use of GCM with HMAC is undesirable due
> to the additional authentication tag, it may be unavoidable in order to
> provide the properties that we need.
>

Thanks for explaining the attack. We will modify the text in Section 4.3 as
follows to address the comment:

   Classifiers, NSH-aware SFs, and SFC proxies MUST implement the
   AES_128_GCM [GCM][RFC5116] algorithm and SHOULD implement the
   AES_192_GCM and AES_256_GCM algorithms.

   Classifiers, NSH-aware SFs, and SFC proxies MUST implement the HMAC-
   SHA-256-128 algorithm and SHOULD implement the HMAC-SHA-384-192 and
   HMAC-SHA-512-256 algorithms.

   SFFs MAY implement the aforementioned cipher suites and HMAC
   algorithms.

Note:
The use of AES_128_CBC_HMAC_SHA_256 algorithm would have avoided the need
for a second 128-bit authentication tag but due to the nature of how CBC
mode operates, the mode allows for malleability of plaintexts. This
malleability allows for attackers to make changes in the ciphertext and, if
parts of the plaintext are known, create arbitrary blocks of
plaintext. This specification mandates the use of AES-GCM to prevent this
type of attack.


>
> (1) Section 5.1 describes the MAC as:
>
>    Message Authentication Code:   Covers the entire NSH data, excluding
>       the Base header.  The Additional Authenticated Data (defined in
>       [RFC7518]) MUST be the Service Path header, the unencrypted
>       Context headers, and the inner packet on which the NSH is imposed.
>
> This description seems to exclude from the MAC most of the MAC context
> header itself (if we go by the corresponding figure), which is very bad
> for security.  We definitely need to include under the MAC the MAC
> context header bits from metadata class through and including at least
> timestamp, and I think IV length as well.  (The IV itself would be
> incorporated via the ciphertext, since the IV is an input to encryption,
> but since the IV length field indicates whether or not encryption was
> performed, we'd need to protect that information.)
>

No, the MAC context header is also used for generating the digest.


>
> Similarly, Section 5.2 has the description:
>
>    Message Authentication Code:  Coves the entire NSH data.  The
>       Additional Authenticated Data (defined in [RFC7518]) MUST be the
>       entire NSH data (i.e., including the Base Header) excluding the
>       Context Headers to be encrypted.
>
> which on the face of it includes the field that holds the MAC itself
> (and is not yet populated), i.e., is self-referential.
>

The below update will address this comment:

The NSH imposer sets the MAC field to zero and then computes the message
integrity for the target NSH data (depending on the integrity protection
scope discussed in Section 5) using MAC_KEY and HMAC algorithm. It inserts
the computed digest in the MAC field in the "MAC and Encrypted Metadata"
Context Header.



>
> I think we need to be much more precise about the construction of the
> AAD in both cases.  It's possible that the HMAC construction for the
> no-encrypted-context-headers case can inherit a definition from the AAD
> description, but if not we'll need to have some more precision there as
> well.
>
> (2) In order for the MAC-only construction in §7.2 to be compatible with
> the AEAD integrity tag construction, we would need to include the 64-bit
> AL after A.  While HMAC is intrinsically immune to length-extension
> attacks, I think that having the explicit AL is useful to avoid any risk
> of malleability, since the same MAC_KEY is used for constructing both
> types of MACs.
>

I don't think this comment is applicable after AES_128_CBC_HMAC_SHA_256 is
removed.


>
> (3) Section 5.1 describes the Timestamp field as an "unsigned 64-bit
> integer value", which is inconsistent with the actual format given in
> Section 6.
>

Thanks, we will fix it in the next revision.


>
> (4) Section 7.5 directs the verifier to check if "the value of the newly
> generated digest is identical to the one enclosed in the NSH".  It is
> critical for the security of the system that this comparison be done in
> a constant-time manner that does not provide a side channel into whether
> the generated digest and the value in the NSH share a common substring.
>

Agreed, will update text as follows:

After storing the value of the MAC field in the "MAC and Encrypted
Metadata" Context Header, the SFC data plane element fills the MAC
field with zeros.  Then, the SFC data plane element generates the message
integrity
for the target NSH data (depending on the integrity protection scope
discussed in Section 5) using MAC_KEY and HMAC algorithm.  If the value of
the newly generated digest is identical to the stored one, the SFC data
plane element is certain that the NSH data has not been tampered and
validation is therefore successful. Otherwise, the NSH packet MUST be
discarded. The comparison of the computed HMAC value to the store value MUST
be done in a constant-time manner to thwart timing attacks.


>
> (5) Do the MAC context headers always have to be the last metadata
> entries in the packet (to simplify the cryptographic calculations)?
>

Yes.

Cheers,
-Tiru


> Certainly the diagrams only show "unencrypted context headers" appearing
> prior to the MAC context header, so if we expect unencrypted context
> headers to appear after the MAC context header as well, we should be
> clear about that both in the figures and in the specification for how to
> prepare the AAD.
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Section 3
>
>    o  Both encrypted and unencrypted Context Headers MAY be included in
>       the same NSH.  That is, some Context Headers may be protected
>       while others do not need to be protected.
>
> In the security area we're pretty strongly moved towards a stance of
> "encrypt everything that doesn't have a strong need to be plaintext
> (since we don't want to assume that we can predict all threats and
> attacks in advance), and QUIC has even taken that to new levels.  It's
> quite surprising to see "don't need to be protected" as a justification
> for lack of encryption, rather than "need to be unprotected".  Given
> that we're going to be solving key distribution for integrity protection
> anyway, it remains unclear to me why we wouldn't just encrypt
> everything.
>
> Section 4.1.1
>
> Is the column heading for the middle column of Table 1 referring to the
> Base Header and "Service Path Header"?  I'm not sure how to interpret
> just "Service Header" other than the entire NSH, which doesn't seem
> right...
>
>    The Service Path Header (Section 2 of [RFC8300]) is not encrypted
>    because SFFs use Service Index (SI) in conjunction with Service Path
>    Identifier (SPI) for determining the next SF in the path.
>
> It's good to see this spelled out like this; I'd love to have this kind
> of explanation for every un-encrypted protocol element.
>
> Section 4.1.2
>
>    The solution provides integrity protection for the NSH data.  Two
>    levels of assurance (LoAs) are supported.
>
> I see (reading onward) that the two LoAs just differ in what is covered
> by the integrity tag, but each involve only a single MAC.  There's a
> reasonable argument that when there are overlapping needs for what data
> needs protecting and who needs access to it, that multiple MACs (or
> encryption, if needed) are most appropriate, which can be tailored to
> the respective needs.  This is something that Phillip Hallam-Baker has
> ascribed as the "short-blanket theory" of cryptography, in that it's
> like a blanket that's too short to cover everything you want, so you
> need to have more than one and put them in partially overlapping layers.
> Perhaps this "short-blanket theory" is more applicable to Discuss point
> (0) than what is covered by the MAC, though...
>
>    The first level of assurance where all NSH data except the Base
>    Header are integrity protected (Figure 2).  In this case, the NSH
>    imposer may be a Classifier, an NSH-aware SF, or an SFC Proxy.  SFFs
>    are not thus provided with authentication material.  [...]
>
> There seems to be a gap in the chain of reasoning as to why SFFs are not
> provided with authentication material.  It seems that the intent is
> something along the lines of considering which entities need access to
> the key material in order to successfully add an NSH or update context
> headers, but that alone is not itself cause to say that other entities
> can never be provided with the keys (subject to the security
> considerations of group-shared PSKs, of course).  Furthermore, if a
> particular SFF is trusted, it may have grounds to verify the integrity
> tag even if it is administratively prohibited from writing such a tag.
>
>    In both levels of assurance, the unencrypted Context Headers and the
>    packet on which the NSH is imposed are subject to integrity
>    protection.
>
> Surely any encrypted Context Headers are also given integrity
> protection, as excluding them from the MAC computation would be quite
> complex.  I see that the encrypted context headers are not part of the
> AAD input, but in the AEAD model the ciphertext is still given integrity
> protection!
>
> Section 4.2
>
>    The authenticated encryption algorithm defined in [RFC7518] is used
>    to provide NSH data integrity and to encrypt the Context Headers that
>    carry privacy-sensitive metadata.
>
> RFC 7518 (JSON Web Algorithms) is a very surprising reference for
> generic AEAD encryption.  Continuing on, it becomes clear that we in
> fact do not want a generic AEAD operation here, but instead have need
> for separation of encryption and integrity protection due to the various
> separations of roles that are possible.  Because this is a divergence
> from current best practices (use of native AEAD cipher modes like GCM,
> SIV, etc.), I might suggest a rather different introductory material:
>
> % The AEAD interface specified in RFC 5116 is a very powerful tool for
> % providing strong confidentiality and integrity protection for protocol
> % messages, and has become quite widely adopted as a result.  For
> % protecting NSH data we adhere to the spirit of the AEAD interface but
> % use a slightly modified version in order to remain consistent with the
> % privilege separation needed (e.g., in the second level of assurance,
> % SFFs need to be able to update the message integrity tag but do not
> % need access to the decrypted plaintext).  Accordingly, a compound AEAD
> % mechanism combining CBC encryption and HMAC integrity tag is used, so
> % that distribution of the encryption key can be limited.  Rather than
> % define a new scheme from scratch, the AES_CBC_HMAC_SHA2 scheme from
> % [RFC7518] is used.  In order to simplify key distribution, a single
> % secret key material seed K is used, which can be kept private
> % centrally or distributed to all parties that will have access to
> % decrypted plaintext, and the secondary keys MAC_KEY and ENC_KEY are
> % derived from K as discussed in Section 5.2.2.1 of [RFC7518].  Entities
> % that need to update the integrity tag but are not authorized to see
> % the decrypted plaintext will only receive MAC_KEY and not K or
> % ENC_KEY.
>
>    o  If the Context Headers are not encrypted, the Hashed Message
>       Authentication Mode (HMAC) algorithm discussed in [RFC4868] is
>       used to protect the integrity of the NSH data.
>
> It's not clear to me that this is adding much value; HMAC is used
> whether or not there are encrypted headers, and the RFC 4868 reference
> is rather odd (see also my Discuss point (2)).
>
>    The advantage of using the authenticated encryption algorithm is that
>    NSH-aware SFs and SFC proxies only need to re-compute the message
>    integrity of the NSH data after decrementing the Service Index (SI)
>    and do not have to re-compute the ciphertext.  The other advantage is
>    that SFFs do not have access to the ENC_KEY and cannot act on the
>    encrypted Context Headers and, only in case of the second level of
>    assurance, SFFs do have access to the MAC_KEY.  Similarly, an NSH-
>    aware SF or SFC Proxy not allowed to decrypt the Context Headers will
>    not have access to the ENC_KEY.
>
> [If my above suggestion is used, this text has probably become mostly
> redundant and could be trimmed down or removed.]
>
>    The authenticated encryption process takes as input four-octet
>    strings: a secret key (K), a plaintext (P), Additional Authenticated
>    Data (A) (which contains the data to be authenticated, but not
>    encrypted), and an Initialization Vector (IV).  The ciphertext value
>    (E) and the Authentication Tag value (T) are provided as outputs.
>
>    In order to decrypt and verify, the cipher takes as input K, IV, A,
>    T, and E.  The output is either the plaintext or an error indicating
>    that the decryption failed as described in Section 5.2.2.2 of
>    [RFC7518].
>
> This seems true for the abstract AEAD interface, but since we're peeling
> off the HMAC integrity tag for standalone use, we may need to say
> something about what inputs/outputs are needed for just using the
> integrity tag without doing decryption.  In particular, we cannot use K
> as an input for standalone HMAC verification!
>
> Also, this is just the generic AEAD interface, so RFC 5116 is probably a
> more apropos reference than RFC 7518.
>
> Section 4.3
>
>       Note: The use of Advanced Encryption Standard (AES) in Galois/
>       Counter Mode (GCM) + HMAC may have CPU and packet size
>       implications (need for a second 128-bit authentication tag).
>
> [This might also be redundant if my earlier proposal is accepted.]
>
> Section 4.4
>
>    The document does not assume nor preclude the following:
>
>    o  The same keying material is used for all the service functions
>       used within an SFC-enabled domain.
>
>    o  Distinct keying material is used per SFP by all involved SFC data
>       path elements.
>
>    o  Per-tenant keys are used.
>
> I suggest a forward reference to the security considerations with a
> brief note that the risk involved in using a group-shared symmetric key
> increases with the size of the group to which it is shared.
>
>    In order to accommodate deployments relying upon keying material per
>    SFC/SFP and also the need to update keys after encrypting NSH data
>    for a certain amount of time, this document uses key identifiers to
>    unambiguously identify the appropriate keying material.  Doing so
>    allows to address the problem of synchronization of keying material.
>
> I'd suggest making a statement about the scope in which key identifiers
> need to be unique.
>
> Section 4.5
>
>    An NSH-aware SF or SFC Proxy that needs to decrypt some Context
>    Headers uses ENC_Key and the decryption algorithm for the key
>    identifier being carried in the NSH.
>
> I think we need to say that decryption MUST NOT proceed unless the
> integrity tag has been successfully validated.  To decrypt without
> checking the integrity tag violates the AEAD interface.  Alternately, we
> could only ever describe decryption as the AEAD operation that uses K as
> input (not just ENC_KEY) -- §7.6 seems to attempt to take this latter
> approach, but the document as a whole is not consistent about it.
>
> Section 5, 5.x
>
> I strongly suggest changing "Key Length" to "keyid length", since there
> is already convention on "key length" (e.g., for AES-128 vs AES-192 vs
> AES-256).
>
> Section 5.1
>
>    Key Identifier:  Carries a variable-length Key Identifier object used
>       to identify and deliver keys to SFC data plane elements.  This
>       identifier is helpful to accommodate deployments relying upon
>       keying material per SFC/SFP.  The key identifier helps in
>       resolving the problem of synchronization of keying material.
>
> I suggest clarifying that a single key identifier is used to lookup both
> the associated ENC_KEY and the associated MAC_KEY (or, alternately, that
> it is logically bound to K and remind the reader that ENC_KEY and
> MAC_KEY are extracted from K and might be separately distributed).
>
>    Key Length:  Variable.  Carries the length of the key identifier.
>    [...]
>    IV Length:  Carries the length of the IV (Section 5.2 of [RFC7518]).
>       If encryption is not used, IV length is set to zero (that is, no
>       "Initialization Vector" is included).
>
> Please also specify that the Key/IV lengths are measured in bytes (the
> reference for IV length, at least, talks about IVs in bits).
>
> I strongly suggest using a value other than zero as the sentinel value
> for "no encryption" -- there already exist modes like AES-SIV (RFC 5297)
> that do not require an IV input, which would be a natural use for
> zero-length IV.
>
> Additionally, one might imagine using a non-zero-length IV as input to
> an HMAC calculation as a per-message nonce.  On cursory examination,
> since we are already using timestamp-based replay detection, the
> additional nonce is unlikely to add much value, but I'm not willing to
> completely rule it out on the basis of the minimal analysis I performed.
>
> Section 7.1
>
>    Only one instance of "MAC and Encrypted Metadata" Context Header
>    (Section 5) is allowed.  If multiple instances of "MAC and Encrypted
>    Metadata" Context Header are included in an NSH packet, the SFC data
>
> I think Roman may have mentioned this already, but please clarify if
> this is one instance per NSH or per packet (e.g., in hierarchical SFC we
> might have more than one NSH in a packet).
>
> Please also clarify it if is permitted to have both MAC#1 and MAC#2
> present in the same packet.  (IIUC there are not obvious situations
> where such a setup would be useful, but it's still good to be clear
> about expectations.)
>
> Section 7.2
>
>    If the Context Headers are not encrypted, the HMAC algorithm
>    discussed in [RFC4868] is used to integrity protect the target NSH
>    data.  [...]
>
> It's not really clear to me why RFC 4868 is the right reference here, as
> opposed to (e.g.) RFC 2104.  In particular, see my Discuss point (2).
>
>    The Message Authentication Code (T) computation process can be
>    illustrated as follows:
>
>          T = HMAC-SHA-256-128(MAC_KEY, A)
>
> I agree with the other reviewer that wondered about just using "MAC" (or
> maybe "HMAC") rather than "HMAC-SHA-256-128" here (I do see the "can be
> illustrated as" qualifier).
>
> It's also a little weird to reuse T and A from the AEAD construction (or
> RFC 4868) here without additional discussion on how they apply and how
> the value of A is constructed.
>
>    An entity in the SFP that intends to update the NSH MUST follow the
>    above behavior to maintain message integrity of the NSH for
>    subsequent validations.
>
> I'm not sure that "intends to" is needed; the requirement applies to
> actions, not intent.
>
> Section 7.3
>
>    In an SFC-enabled domain where pervasive monitoring [RFC7258] is
>    possible, all Context Headers carrying privacy-sensitive metadata
>    MUST be encrypted; [...]
>
> I agree with the other reviewer that noted that this seems to
> effectively be an unqualified "MUST", since there is always some
> possibility of monitoring.
>
>          MAC_KEY = initial MAC_KEY_LEN octets of K,
>          ENC_KEY = final ENC_KEY_LEN octets of K,
>          E = CBC-PKCS7-ENC(ENC_KEY, P),
>          M = MAC(MAC_KEY, A || IV || E || AL),
>          T = initial T_LEN octets of M.
>          MAC and Encrypted Metadata = E || T
>
> I recognize that this is just copy+paste from RFC 7518 (with the
> addition of the last line), but it seems a glaring omission that the
> CBC-PKCS7-ENC() invocation does not include the IV as an input!
> (In §5.2.2.1 of RFC 7518 we do see that "The plaintext is CBC encrypted
> using PKCS #7 padding using ENC_KEY as the key and the IV", so it's just
> the figure/formula that omits the IV.)
>
> We also don't define or mention T_LEN in this document other than in
> this formula and the analogous one for MAC-without-encryption, which
> might be worth remedying.
>
> Section 7.4
>
> I suggest adding another word (like "Prevention") to the section title.
>
>    The received NSH is accepted by an NSH-aware node if the Timestamp
>    (TS) in the NSH is recent enough to the reception time of the NSH
>    (TSrt).  The following formula is used for this check:
>
>              -Delta < (TSrt - TS) < +Delta
>
> (There's no real reason why the accepted timestamp skew parameter Delta
> needs to be symmetric in the positive and negative directions.)
>
>    Replay attacks within the Delta window may be detected by an NSH-
>    aware node by recording a unique value derived, for example, from the
>    NSH data and Original packet (e.g., using SHA2).  [...]
>
> Note that the unique value MUST NOT use as input any bits that are not
> covered by the MAC.  Indeed, just using the MAC itself as the unique
> value would be a fairly straightforward scheme, I think.
>
> 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.  [...]
>
> This sort of requirement (as written) would seem to support the position
> of some other reviewers that this document would need to formally update
> RFC 8300.
>
> Section 9
>
> We might discuss that this mechanism is only for metadata type 2 and not
> type 1 (as well as that with the relatively small fixed length of type 1
> metadata, it's quite challenging to provide useful cryptographic
> protection), so that type-1 metadata still has no generic protection
> mechanism.  A given metadata element could define its own protection
> scheme, of course.
>
>    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.  [...]
>
> >From a very quick read, draft-nguyen-sfc-security-architecture proposes
> a probe-based scheme that monitors all middleboxes (or at least those in
> the expected path) to detect a middlebox-bypass attack and verify that
> the probe traversed the expected SFP.  I am not sure that this document
> on its own suffices to prevent the middlebox-bypass attack for all
> traffic (i.e., the probe-based scheme might still be needed).  If some
> network entities know what the expected service index will be for a
> given SPI at a given node, then middlebox-bypass attacks could be
> natively detected, but my understanding was that such advance knowledge
> of expected service index was not going to be available.
>
>    The solution specified in this document does not provide data origin
>    authentication.
>
> I would have expected more in-depth discussion of the security
> considerations for a group-shared PSK scheme, including how the risk of
> key loss/compromise goes up when the key is distributed to more
> locations.
>
> Section 9.1
>
>    No device other than the NSH-aware SFs in the SFC-enabled domain
>    should be able to update the integrity protected NSH data.
>
> What about classifiers and SFC proxies?
>
> Section 9.2
>
> I expected to see some remarks here analogous to the second paragraph of
> §9.1 (even if just by reference to that section).
>
> Section 9.3
>
>    Section 5.6 of [RFC8633] describes best current practices to be
>    considered in deployments where SFC data plane elements use NTP for
>    time synchronization purposes.
>
> The referenced section covers NTP symmetric mode only.  Is client/server
> mode not a BCP for trusted networks and broadcast/symmetric the only
> options?
>
> NITS
>
> Section 1
>
>    network infrastructures against them, network slicing, etc.  Because
>    of the proliferation of such advanced SFs together with complex
>    service deployment constraints that demand more agile service
>    delivery procedures, operators need to rationalize their service
>    delivery logics and master their complexity while optimising service
>    activation time cycles.  The overall problem space is described in
>    [RFC7498].
>
> The "their" in "master their complexity" is a bit ambiguous between
> "operators" and "service delivery logics" (noting that the previous use
> of "their" did refer to "operators").  (The sentence as a whole reads a
> bit like marketing language, but that's not intended to be an actionable
> comment.)
>
>    This specification fills that gap.  Concretely, this document adds
>    integrity protection and optional encryption of sensitive metadata
>    directly to the NSH (Section 4); integrity protects the packet
>    payload and provides replay protection (Section 7.4).  [...]
>
> The clause after the semicolon needs to be able to stand alone as a
> complete sentence in order to be grammatical, but it currently has no
> subject.  Maybe prefixing "it also" would be enough to fix it.
>
> Section 4.1.2
>
>    The integrity protection scope is explicitly signaled to NSH-aware
>    SFs and SFC proxies in the NSH by means of a dedicated MD Type
>    (Section 5).
>
> Signaled to SFFs as well, since in the second LoA they can use it.
>
> Section 4.6
>
>    As discussed in [RFC8459], an SFC-enabled domain (called, upper-level
>    domain) may be decomposed into many sub-domains (called, lower-level
>    domains).  In order to avoid maintaining state to restore back upper-
>    lower NSH information at the boundaries of lower-level domains, two
>
> I think s/upper-lower/upper-layer/ (across the line break)?
>
> Section 5.1
>
>    MAC#1 Context Header is a variable-length Context Header that carries
>    the Message Authentication Code (MAC) for the Service Path Header,
>    Context Headers, and the inner packet on which NSH is imposed,
>    calculated using MAC_KEY and optionally Context Headers encrypted
>    using ENC_KEY.  [...]
>
> Comma after "calculated using MAC_KEY", please.  ("optionally Context
> Headers [...]" are not used along with MAC_KEY in the calculation)
>
> Section 5.2
>
>    Message Authentication Code:  Coves the entire NSH data.  The
>
> s/Coves/Covers/
>
> Section 6
>
>       The epoch is 1970-01-01T00:00Z in UTC time.  [...]
>
> (IIUC, the "in UTC time" is redundant with the trailing 'Z'.)
>
> Section 7.1
>
>    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 in the alarm) SHOULD be configurable.
>
> The grammar in the parenthetical seems off to me.  Possibly
> s/depend/depending/ would be a fix, but I'm not sure.
>
>    NSH-aware SFs and SFC proxies MUST re-apply the integrity protection
>    if any modification is made to the Context Headers (strip a Context
>    Header, update the content of an existing Context Header, insert a
>    new Context Header).
>
> I think maybe an "e.g.," in the parenthetical is in order, since this
> doesn't seem to include things like "decrement the TTL".
>
> Section 9
>
>    The interaction between the SFC data plane elements and a key
>    management system MUST NOT be transmitted in clear since this would
>    completely destroy the security benefits of the integrity protection
>    solution defined in this document.  The secret key (K) must have an
>    expiration time assigned as the latest point in time before which the
>
> This first sentence seems like it could probably be a paragraph of its
> own; the subsequent discussion of expiration time seems to be mostly
> unrelated.
>
> We might also add another sentence to reiterate that "the security and
> integrity of the key-distribution mechanism is vital to the security of
> the system as a whole".
>
>    o  Attacker replacing the packet on which the NSH is imposed with a
>       bogus packet.
>
> I'd probably say "modified or bogus".
>
>    In an SFC-enabled domain where the above attacks are possible, (1)
>
> This is perhaps ambiguous about whether an "any" or "all" criterion
> should be used.  Perhaps that's desired :)
>
>
>
>