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:noreply@xxxxxxxx] > Envoyé : vendredi 25 juin 2021 10:18 > À : ops-dir@xxxxxxxx > Cc : draft-ietf-sfc-nsh-integrity.all@xxxxxxxx; last-call@xxxxxxxx; > sfc@xxxxxxxx > Objet : Opsdir last call review of draft-ietf-sfc-nsh-integrity-05 > > 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. [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 > 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. [Med] ... I agree that we still depend on the provisioning. Removed that sentence. > > - 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. > [Med] Fair point. Added a note. > Minor nits: > > - In section 5.2, it should say: > > OLD > > In reference to Figure 5 > > NEW > > In reference to Figure 7 > [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. > - 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 [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. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
