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

mohamed.boucadair@orange.com Fri, 16 July 2021 14:15 UTC

Return-Path: <mohamed.boucadair@orange.com>
X-Original-To: sfc@ietfa.amsl.com
Delivered-To: sfc@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 0F9B83A38AA; Fri, 16 Jul 2021 07:15:54 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.798
X-Spam-Level:
X-Spam-Status: No, score=-2.798 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=orange.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id J_LSazbBC6yH; Fri, 16 Jul 2021 07:15:48 -0700 (PDT)
Received: from relais-inet.orange.com (relais-inet.orange.com [80.12.70.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 1E7033A38A9; Fri, 16 Jul 2021 07:15:48 -0700 (PDT)
Received: from opfednr04.francetelecom.fr (unknown [xx.xx.xx.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by opfednr22.francetelecom.fr (ESMTP service) with ESMTPS id 4GRCvP4dLVz103d; Fri, 16 Jul 2021 16:15:45 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=orange.com; s=ORANGE001; t=1626444945; bh=GmL54hOe13TS9AZG/i3lkmBkt50TKtvoOxZi0V2p4Mo=; h=From:To:Subject:Date:Message-ID:Content-Type: Content-Transfer-Encoding:MIME-Version; b=deWto+DpOufkXZi4DWcEOiaeCgPtAsQML6XBfsAW9z0UZ5FDtfgNvT+nzVmcEoSEL OdT0rnhOfDShOI79QOWnA6M22tEaMfjSsIzS0EE+8+TtjdKmp6eHTOUIcgZs8PqcIN PDqUY26t1EoKCpQDS7rCWcQoSTadS2NJS9pUHX1o+mmgXy0CT/kzIwGeCFFlnSqwxg QA5okBur9WqjMRZtdGoZBF2ksGuMCbfvT7VCEmvLoHr0bLSDCTVhEMHHNnpZkzj5AR hnrWhRAUVJod5ousXUn1RLgeoyqLlS6amtK7wXb3tG6XUzvTDywutsCllJTXnedKiA fgQqq5Bb0/xIQ==
Received: from Exchangemail-eme6.itn.ftgroup (unknown [xx.xx.13.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by opfednr04.francetelecom.fr (ESMTP service) with ESMTPS id 4GRCvP3clSz1xpn; Fri, 16 Jul 2021 16:15:45 +0200 (CEST)
From: mohamed.boucadair@orange.com
To: Benjamin Kaduk <kaduk@mit.edu>, The IESG <iesg@ietf.org>
CC: "draft-ietf-sfc-nsh-integrity@ietf.org" <draft-ietf-sfc-nsh-integrity@ietf.org>, "sfc-chairs@ietf.org" <sfc-chairs@ietf.org>, "sfc@ietf.org" <sfc@ietf.org>, "gregimirsky@gmail.com" <gregimirsky@gmail.com>
Thread-Topic: Benjamin Kaduk's Discuss on draft-ietf-sfc-nsh-integrity-06: (with DISCUSS and COMMENT)
Thread-Index: AQHXePbUnGtMWTBmlECZ6vxdOGPzS6tFFEmQ
Date: Fri, 16 Jul 2021 14:15:44 +0000
Message-ID: <14311_1626444945_60F19491_14311_347_2_787AE7BB302AE849A7480A190F8B9330353C02BD@OPEXCAUBMA2.corporate.adroot.infra.ftgroup>
References: <162629795136.25767.17850752260179612361@ietfa.amsl.com>
In-Reply-To: <162629795136.25767.17850752260179612361@ietfa.amsl.com>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.114.13.247]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/sfc/_cmWtGK3ivnIoULPVJwZox1dlrA>
Subject: Re: [sfc] Benjamin Kaduk's Discuss on draft-ietf-sfc-nsh-integrity-06: (with DISCUSS and COMMENT)
X-BeenThere: sfc@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Network Service Chaining <sfc.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/sfc>, <mailto:sfc-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/sfc/>
List-Post: <mailto:sfc@ietf.org>
List-Help: <mailto:sfc-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/sfc>, <mailto:sfc-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 16 Jul 2021 14:15:54 -0000

Hi Ben, 

Many thanks again for the detailed review. Really helpful. 

Please see inline some replies to the COMMENT part (with references to some DISCUSS points).

Changes to address your COMMENTS/NITS can be tracked at: https://tinyurl.com/nsh-integrity-latest.

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk via Datatracker [mailto:noreply@ietf.org]
> Envoyé : mercredi 14 juillet 2021 23:26
> À : The IESG <iesg@ietf.org>
> Cc : draft-ietf-sfc-nsh-integrity@ietf.org; sfc-chairs@ietf.org;
> sfc@ietf.org; gregimirsky@gmail.com; gregimirsky@gmail.com
> Objet : Benjamin Kaduk's Discuss on draft-ietf-sfc-nsh-integrity-06:
> (with DISCUSS and COMMENT)
> 
> 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/
> 
> 
> 
[...]
> 
> 
> --------------------------------------------------------------------
> --
> 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.

[Med] This depends on the type of the context header, the capabilities of SFs, local policies, performance implications, etc. Encrypting only a subset of context headers is a feature we want to provide. 

Please note that the I-D gives a transition path for SFC from the current plain text without any confidentiality or integrity protection to start with integrity protection and eventually transition to (partial and full) confidentiality and integrity protection.

We can recommend deployments to consider using encryption for **any** context header to prevent pervasive monitoring.

We understand that the wording "do not need to be protected" may not be appropriate. We removed the sentence starting with "That is, some ...".

> 
> 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...

[Med] That is in reference to what we say in Section 3 that:

   o  Base Header: Provides information about the service header and the
      payload protocol.

> 
>    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).

[Med] The two levels are there for several reasons:
* ease deployment: have an option that does not involve SFFs.
* performance considerations of SFFs


> 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.

[Med] Agree, but we wanted to keep only two clear LoAs. 

> 
>    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!

[Med] The text you quoted is here to remind that the original NSH packet is integrity protected as this may be missed by the reader. Likewise, that same protection is provided to context headers, even when unencrypted. 

> 
> 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. 

[Med] FYI, we used to have such reference in draft-rebo-sfc-nsh-integrity-00:

   The Authenticated Encryption with Associated Data (AEAD) algorithm
   [RFC5116] is used to provide NSH data integrity and to encrypt
   privacy-sensitive metadata.

We changed the spec because we wanted to control by design the nodes that are allowed to access encrypted data vs integrity-only. BTW, we sent a note to the saag mailing list right after we changed our design in the hope to have more eyes on what we are doing, but unfortunately we didn't get much attention. 

Our understanding of your review is that you are not against using ENC_KEY and MAC_KEY, but you prefer AES-GCM over CBC. We are considering the following change (not yet implemented):  

OLD:

   Classifiers, NSH-aware SFs, and SFC proxies MUST implement the
   AES_128_CBC_HMAC_SHA_256 algorithm and SHOULD implement the
   AES_192_CBC_HMAC_SHA_384 and AES_256_CBC_HMAC_SHA_512 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 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).

NEW:

   Classifiers, NSH-aware SFs, and SFC proxies MUST implement the
   AES_128_GCM 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.

We will have to refer to RFC5116 and [GCM]

   [GCM]         Dworkin, M., "Recommendation for Block Cipher Modes of
                 Operation: Galois/Counter Mode (GCM) and GMAC",
                 National Institute of Standards and Technology SP 800-
                 38D, November 2007.

With that, we don't need to cite RFC7518. 


 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. 

[Med] That's it!

 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.

[Med] I guess, the comment will not be relevant after mandating AES-GCM.

> 
>    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.

[Med] As suggested above, we can refer to RFC5116 instead of RFC7518.

> 
> 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.

[Med] OK. Done. 

> 
>    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.

[Med] That depends on the bullet mentioned above (ex. per-tenant). I'm not sure adding the sentence adds much value as the scope tweaking still deployment-specific.  

> 
> 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.

[Med] This section is about introducing the new MAC context headers. I don't think this is the right place to mention this.  

BTW, the order you mentioned is what is implied by the procedure: 7.5 (NSH Data Validation) and then 7.6 (Decryption of NSH Metadata). Invalid data are discarded as part of 7.5.

No problem to add a note to 7.6 if you think this is really needed.

> 
> 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).

[Med] Fixed as per a comment from Eric. 

> 
> 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).

[Med] Added this NEW: 

"A single key identifier is used to lookup both the ENC_KEY and the MAC_KEY associated with a key." 

> 
>    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

[Med] Fixed per a comment from Eric.

> (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.

[Med] What is the advantage of using a value other than zero when encryption is not included? 

> 
> 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).

[Med] I confirm.

> 
> 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.)

[Med] It is not allowed to have both. This is why we are using the generic term: "instance of "MAC and Encrypted Metadata" Context Header". That term is used through the document when we refer indifferently to MAC#1 and MAC#2.
  
> 
> 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).

[Med] We think that RFC 4868 is the right reference as RFC 2014 does not discuss any of the HMAC algorithms used in this specification. 

> 
>    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.

[Med] Noted. We made already this change: s/A/target NSH data. 

> 
>    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.

[Med] Fair. Fixed, thanks.

> 
> 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.

[Med] If we mandate AES_GCM, all the above text will be removed.

> 
> Section 7.4
> 
> I suggest adding another word (like "Prevention") to the section
> title.
> 

[Med] Fixed. 

>    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.)

[Med] This was to accommodate any sync issues.

> 
>    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. 

[Med] Agree, but we don't specify the bits that are used. 

 Indeed, just using the MAC itself as the
> unique value would be a fairly straightforward scheme, I think.

[Med] Thanks.

> 
> 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.

[Med] Added "When an SFC data plan element conforms to this specification," to address one of Roman's comments. 

> 
> 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.

[Med] We already have the following in the introduction:

   This specification is only
   applicable to NSH MD Type 0x02 (Section 2.5 of [RFC8300]).

> 
>    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.

[Med] The SF bypass can be detected if SI was manipulated on path because integrity check will fail. 

> 
>    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.

[Med] That's part of the key management, declared out of scope.   

> 
> 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?

[Med] Good catch: proxies should be mentioned as well as re-classification. We can't just talk about classification as this is about updating the MAC. 

The NEW text that also takes into account a comment from John is as follows: 

   It is expected that specific devices in the SFC-enabled domain will
   be configured such that no device other than the classifiers (when
   reclassification is enabled), NSH-aware SFs, and SFC proxies will be
   able to update the integrity protected NSH data (Section 7.1), and
   also such that no device other than the NSH-aware SFs and SFC proxies
   will be able to decrypt and update the Context Headers carrying
   sensitive metadata (Section 7.1).

> 
> 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).

[Med] Added this NEW sentence: 

   Similar to Section 9.1, no device other than the NSH-aware SFs and
   SFC proxies in the SFC-enabled domain should be able to decrypt and
   update the Context Headers carrying sensitive metadata. 

> 
> 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.

[Med] Yes because we think that’s the symmetric mode is better vs. broadcast with trusted modes. 

  Is
> client/server mode not a BCP for trusted networks and
> broadcast/symmetric the only options? 

[Med] Not sure to get this comment. 

_________________________________________________________________________________________________________________________

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.