Subject: [Last-Call] Opsdir last call review of draft-ietf-sfc-nsh-integrity-05

From: Jürgen Schönwälder via Datatracker <[email-masked]>
Date: Fri, 25 Jun 2021 01:18:12 -0700

Reviewer: Jürgen Schönwälder
Review result: Has Issues

The document is very well written and organized, thanks for that. I
have some comments and a few nits. The issues I have a minor, I think
the document overall is in a good shape.

- From an operational perspective, I very much appreciate that there
  is some text discussing error and failure reporting and that there
  are some MTU configuration guidelines.

- The document claims:

    As depicted in Table 1, SFFs are not involved in data encryption.
    This document enforces this design approach by encrypting Context
    Headers with keys that are not supplied to SFFs, thus enforcing this
    limitation by protocol (rather than requirements language).

  I am a bit puzzled about this statement since a protocol definition
  at the end is just some form of requirements language. Well, putting
  that remark aside, I do not really see anything in the specification
  that ensures that SFFs won't get the keys. We also read:

    [...] Encryption keying material is only provided to
    these SFC data plane elements.

  Well, the specification is actually silent about how keys are
  distributed. Section 4.4. says:

    The procedure for the allocation/provisioning of secret keys (K) and
    authenticated encryption algorithm or MAC_KEY and HMAC algorithm is
    outside the scope of this specification.  As such, this specification
    does not mandate the support of any specific mechanism.

  To me, it seems the claims made in section 4.4. do not really hold.

- In section 6, you picked as the epoch 1970-01-01T00:00Z while NTP
  uses the epoch 1900-01-01T00:00Z. This leads to rollovers in the
  year 2106 for your timestamps while the NTP rollover would be in
  2036. Given that you use an NTP compatible format and a different
  epoch, I think this deserves to be spelled out explicitly so that
  people understand that they have to do conversions.

  (Alternatively, you could adopt the epoch NTP is using and the need
  for conversions goes away at the price of an earlier rollover.)

  Anyway, what I am saying is that if you pick a different epoch, I
  suggest to spell this out explicitly and to not rely on
  implementers to discover this.

Minor nits:

- In section 5.2, it should say:

  OLD

  In reference to Figure 5

  NEW

  In reference to Figure 7

- In section 7.3, I suggest this change to make it clear again to the
  reader what K is:

  OLD

  Using K and

  NEW

  Using the secret key K and

---

From: <[email-masked]>
Date: Fri, 25 Jun 2021 09:55:35 +0000

Hi Jürgen, 

Thank you for the review. You can track the changes at: https://tinyurl.com/nsh-integrity-latest 
 
Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Jürgen Schönwälder via Datatracker [mailto:[email-masked]]
> Envoyé : vendredi 25 juin 2021 10:18
> À : [email-masked]
> Cc : [email-masked]; [email-masked];
> [email-masked]
> Objet : Opsdir last call review of draft-ietf-sfc-nsh-integrity-05
> 
> have some comments and a few nits. The issues I have a minor, I
> think the document overall is in a good shape.
> 
>   at the end is just some form of requirements language.

[Med] FWIW, that sentence was drawn compared to an alternative design we considered in the past (use of AEAD which relies upon a single key for both encryption and integrity) and for which we indicated the following at that time:

      In order to avoid the overhead of multiple authentication tags and
      multiple keys, and to prevent SFFs from acquiring the secret key
      to decrypt the metadata, the recommendation is not to integrity
      protect the base header.  Such approach does not require to
      recompute the MAC each time TTL is decremented by an SFF.  As a
      reminder, an SFF is not supposed to act on the metadata or look
      into the content of the metadata.

We were concerned with relying on the requirement language (mainly the last sentence) for the correct protocol behavior. We changed the design so that the limitation is imposed by the protocol: use distinct keys for encryption (ENC_KEY) and integrity (MAC_KEY). 

We do have the following: 

   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.

but ...

 Well,
> putting

[Med] ... I agree that we still depend on the provisioning. Removed that sentence. 

>   implementers to discover this.

[Med] Fair point. Added a note. 

> - In section 5.2, it should say:

[Med] For both 5.1 and 5.2 we provide a reference to the figure with the format of the context header (i.e., Figure 5). Figures 6/7 cover the NSH header and so on.  

>   Using the secret key K and

[Med] Fixed. Thanks.

_________________________________________________________________________________________________________________________

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.