Integrity Protection for the Network Service Header (NSH) and Encryption of Sensitive Context Headers
draft-ietf-sfc-nsh-integrity-09
Yes
(Martin Vigoureux)
No Objection
Francesca Palombini
(Robert Wilton)
Note: This ballot was opened for revision 06 and is now closed.
Erik Kline
No Objection
Comment
(2021-07-12 for -06)
Sent
[S7.{2,3}] [question] * Is the timestamp a part of the input to the MAC/encrypted metadata generation? If so, perhaps consider adding an explicit mention (perhaps I missed it). For that matter, I can't quite tell of some normalized version of the MAC#1/2 header overall is part of the input to the MAC/AAD generation or not.
Francesca Palombini
No Objection
John Scudder
No Objection
Comment
(2021-07-14 for -06)
Sent
1. Section 4.2 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. As written, this means that each of these quantities is a 32 bit string, even P and A. I think you don’t mean that. If you mean each quantity is a string of octets, then move the hyphen: “takes as input four octet-strings“. (In the unlikely event you really do mean each quantity is a string of four octets, then “… takes as input four strings, each of four octets.”) 2. Section 9 Regarding your use of the term “man-in-the-middle attacker”, you might want to take into consideration that this language may be seen as exclusionary by some readers, see also https://www.ietf.org/about/groups/iesg/statements/on-inclusive-language/. I’ve seen “on-path attacker“ used as a replacement in some cases, but I understand there may be nuances that make this inappropriate for some uses; it also appears you might just be able to use “attacker” in your case. The RFC Editor may have further guidance. 3. Section 9.1 You use “should“ in several places. As written, it isn’t clear if you’re indicating expectation, or requirement. After reading the whole section, I think you’re indicating requirement. This seems like a good place for use of the RFC 2119 style SHOULD keyword.
Murray Kucherawy
(was Discuss)
No Objection
Comment
(2021-07-26 for -07)
Sent
Thanks for the discussion about updating RFC 8300. Only nits to add, given the thorough treatment already given by others: Section 4.1.2: "The first level of assurance where all NSH data ..." -- add "is" before "where"? And the same issue in the next paragraph. Section 5.2: s/Coves/Covers/
Roman Danyliw
(was Discuss)
No Objection
Comment
(2021-09-18 for -08)
Sent for earlier
Thanks for addressing my COMMENT and DISCUSS items.
Warren Kumari
No Objection
Comment
(2021-07-14 for -06)
Sent
I support Roman and Eric's DISCUSS points. I also found: "Note that some transport encapsulations (e.g., IPsec) only provide hop-by-hop security between two SFC data plane elements (e.g., two Service Function Forwarders (SFFs), SFF to SF) and do not provide SF-to-SF security of NSH metadata. For example, if IPsec is used, SFFs or SFs within a Service Function Path (SFP) that are not authorized to access the privacy-sensitive metadata will have access to the metadata." to be incredibly hard to read/parse. I think that my confusion comes in around the "that are not authorized to access the privacy-sensitive metadata will have access to the metadata." test, and thing that the last sentence should be rewritten to start with "Because IPsec does not... it exposes privacy-sensitive metadata to..." Also, thanks to Jürgen Schönwälder for the OpsDir review, and to the authors for addressing the comments.
Zaheduzzaman Sarker
No Objection
Comment
(2021-07-13 for -06)
Sent
Thanks for the efforts on this specification. I have following non-blocking comments those I believe would improve the document if addressed -- * I agree with Alvaro and Lars's comment about updating 8300. Would like to get response(s) to their comments. * I think it will be helpful to explicitly mention if integrity and confidentiality by the transport encapsulation is needed or not when this specification is in use. This specification definitely says that one does not need to relay on the service provided by the transport encapsulation but it does not says that those services are not longer required. * Section 1 : says - "This specification fills that gap. Concretely, this document adds integrity protection and optional encryption of sensitive metadata directly to the NSH (Section 4);" Does this specification extends the use of NSH in multiple SFC domain? My little understanding of NSH says it is SFC domain specific and within one SFC domain the devices a vetted to be trusted. I think it will be very helpful to add zest from the section 3.2.1. of I-D.arkko-farrell-arch-model-t here. * Section 6 : The epoch is 1970-01-01T00:00Z in UTC time. Note this epoch value is different from the one used in Section 6 of [RFC5905]. It would be great if we can add the implications of the difference. Now I don't know what it means.
Éric Vyncke
(was Discuss)
No Objection
Comment
(2021-09-13 for -08)
Sent
Thank you for the work put into this document. As already communicated by email, I am now clearing my previous DISCUSS ballot (kept below only for archiving purposes), I am also trusting the responsible AD about my previous COMMENT points. Special thanks to Greg Mirsky for his shepherding especially about his summary of the WG consensus. I hope that this helps to improve the document, Regards, -éric PS: your reply was lost in the ‘fury’ ot the IETF week, sorry about that and please thank your AD, Martin Vigoureux, for sending me today a gentle nudge... == previous DISCUSS (for archive only as the current revision addresses all the points) == I failed to spot the order of the operations for the integrity and confidentiality operations, e.g., I did not find on what the HMAC is computed: in the unencrypted or encrypted field ? -- Section 5.1 -- What is the unit of "key length", I assume a length expressed in octets but it is not specified. -- Section 7.2 -- What is the "A" used in the HMAC computation ? The formula specifies HMAC-SHA-256-128() but what if another HMAC is used ? Section 7.3 use MAC() which is more flexible. As the MAC field is included in the integrity protected header, please specify the value of this field when computing the HMAC (I assume 0 but let's be clear) -- Section 7.5 -- What is the expected behavior when a NSH does not contain a "MAC and Encrypted Metadata" Context Header ? §1 hints to packet drop ? Should there be a local policy for this case ? I second Alvaro's and Lars' point about formally updating RFC 8300. Quite often in the text "privacy-sensitive metadata" is used but encryption is not only required for privacy but also to protect operational data from attackers (i.e., not linked to privacy), is there a real need to qualify "metadata" with "privacy-sensitive", i.e., "confidential metadata" may be more appropriate ? -- Section 4.1.1 -- The use of 'metadata' (notably in table 1) for 'context headers' is confusing for a first-time reader. Any chance to be consistent and only use 'context headers' ? -- Section 4.2 -- At first reading, I am confused by the use of the word 'secret key' for what appears to be a 'shared key'. Or is this 'secret key' never shared and simply used to generate the secondary keys, which are then shared ? Even if discussed later, some early explanations would be welcome. -- Section 5.1 -- Is there a good reason why the field name "key length" is not "key ID length" ? I also find very strange to define "Length: variable" as the header field itself as a fixed length, simply state "unsigned integer". As timestamp can include fractions of second, which is a good thing, then please reword "expressed in seconds relative" to indicate that fraction of second is included. The 32-bit epoch will wrap around in 2036, should this I-D already consider what to do at that moment ? -- Section 8 -- Indeed MTU is always an interesting 'problem' but how is this extension different than plain NSH ? The NSH neither increases nor decreases on the fly with this document. == NITS == -- Section 3 -- Should 'figure 1' be 'table 1' per consistency with section 4.1.1 ?
Martin Vigoureux Former IESG member
Yes
Yes
(for -06)
Unknown
Alvaro Retana Former IESG member
No Objection
No Objection
(2021-07-12 for -06)
Sent
(1) Given the required behavior specified in the Security Considerations section... NSH data are exposed to several threats: o A man-in-the-middle attacker modifying the NSH data. o Attacker spoofing the NSH data. o Attacker capturing and replaying the NSH data. o Data carried in Context Headers revealing privacy-sensitive information to attackers. o Attacker replacing the packet on which the NSH is imposed with a bogus packet. 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. The Base and Service Path headers are not encrypted. Why doesn't this document formally update rfc8300? Concerns that eventually led to this solution have been expressed for several other documents, including rfc8459 and rfc8979. It looks like the WG didn't consider the question of Updating the base NSH specification. I believe that this document specifies a required update to NSH, and would like the WG to consider formally Updating rfc8300. [My search of the archive didn't find any related discussion -- did I miss it?] [Even though I consider this omission a serious oversight, I don't think this issue raises to the level of a DISCUSS.] (2) §3: I find the use of normative language to describe requirements (that are met in this same document) not the best use of rfc2119 language because any interoperability concerns would result from the specification itself and not the requirements. The use of rfc2119 keywords to describe requirements may result in confusion. For example, "The solution MAY provide integrity protection for the Base Header." -- as described later, protecting the Base Header is optional, but the solution *does* provide integrity protection for it. IOW, the specification is what is reflected in the requirement, but referring to the solution, not the protection: providing integrity protection is not optional, using it is. A better working would be: "The solution must provide optional integrity protection for the Base Header."
Benjamin Kaduk Former IESG member
(was Discuss)
No Objection
No Objection
(2021-09-14 for -08)
Sent
Thanks for the updates in the -07 and -08; I think we found a reasonable way to address the issues that came up in my earlier review. I turned most of my comments into a github PR against https://github.com/boucadair/draft-ietf-sfc-nsh-integrity/ since that repo was referenced in previous responses to comments. The tip of the branch I forked off seems to have corresponded to only the -07, though, so I'm not sure if I should have gone somewhere else. (I made a separate first commit that just syncs down the -08 from the datatracker.) The PR is https://github.com/boucadair/draft-ietf-sfc-nsh-integrity/pull/6 and the actual "new" (non-08) changes are viewable via https://github.com/boucadair/draft-ietf-sfc-nsh-integrity/compare/99c6484fbe85af7179086ab243c88813e2b47b74..kaduk:kaduk-review?expand=1 Section 4.4 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 addresses the problem of synchronization of keying material. I included a suggestion in my pull request, but I want to call out as a potentially significant change that the key identifier should identify not just the key material but also the algorithms that they key is to be used with. Section 5.1 Nonce Length: Carries the length of the Nonce. If the Context Headers are only integrity protected, "Nonce Length" is set to zero (that is, no "Nonce" is included). The "Nonce Length" can be set to zero depending on the encryption algorithm used to encrypt the Context Headers. I included this in my PR, but want to call out that this last sentence seems harmful. It seems vastly preferrable to have "the nonce length field is zero" indicate one specific thing, rather than having two different possibilities for that situation, as this text appears to do. I know that my earlier comments proposed a scenario where an AEAD might validly encrypt with a zero-length nonce, but I don't think we should support that at the cost of losing the clear signal that the encrypted context headers are (not) present. Section 7.3 Using the secret key (ENC_KEY) and authenticated encryption algorithm, the NSH imposer encrypts the Context Headers (as set, for example, in Section 3) and inserts the resulting payload in the "MAC and Encrypted Metadata" Context Header (Section 5). The entire Context Header carrying a sensitive metadata is encrypted (that is, including the MD Class, Type, Length, and associated metadata of each Context Header). I included some text in my pull request, but want to call out as important that this description does not specify what is to be used as the "additional authenticated data" AEAD input. (I assume the empty string is intended, though other choices are valid.) Section 7.5 When an SFC data plane element conforms to this specification, it MUST ensure that a "MAC and Encrypted Metadata" Context Header is included in a received NSH packet. [...] This doesn't quite seem like the right condition -- this text sounds like once you implement this context header you have to require that it appears in all incoming packets, which breaks interoperability with senders that don't produc this contxt header. Section 9 The secret key (K) must have an expiration time assigned as the latest point in time before which the key may be used for integrity protection of NSH data and encryption of Context Headers. Prior to the expiration of the secret key, all participating NSH-aware nodes SHOULD have the control plane distribute a new key identifier and associated keying material so that when the secret key is expired, those nodes are prepared with the new secret key. This allows the NSH imposer to switch to the new key identifier as soon as necessary. It is RECOMMENDED that the next key identifier and associated keying material be distributed by the control plane well prior to the secret key expiration time. I note that draft-irtf-cfrg-aead-limits offers some guidance on how often to rekey (though it gives data-based criteria, not time-based ones), if that is potentially relevant.
Lars Eggert Former IESG member
No Objection
No Objection
(2021-07-12 for -06)
Sent
Section 1. , paragraph 6, 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). Thus, the NSH > does not have to rely upon an underlying transport encapsulation for > security and confidentiality. Given that, I am surprised this document doesn't formally update RFC8300? Section 6. , paragraph 16, comment: > This timestamp format is affected by leap seconds. The timestamp > represents the number of seconds elapsed since the epoch minus the > number of leap seconds. Any particular reason why leap seconds are being excluded here? This is unusual and also requires care with synchronized clocks (as identified below). Found terminology that should be reviewed for inclusivity: * Term "master"; alternatives might be "active", "central", "initiator", "leader", "main", "orchestrator", "parent", "primary", "server". * Term "man"; alternatives might be "individual", "people", "person". See https://www.rfc-editor.org/part2/#inclusive_language for background and more guidance. ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. "SFC ", paragraph 2, nit: > Also, this specification allows to encrypt sensitive metadata that is carrie > ^^^^^^^^^^ Did you mean "encrypting"? Or maybe you should add a pronoun? In active voice, "allow" + "to" takes an object, usually a pronoun. Section 2. , paragraph 7, nit: > ng a service path. The NSH allows to share context information (a.k.a., metad > ^^^^^^^^ Did you mean "sharing"? Or maybe you should add a pronoun? In active voice, "allow" + "to" takes an object, usually a pronoun. Section 9. , paragraph 10, nit: > ts This document was edited as a follow up to the discussion in IETF#104: ht > ^^^^^^^^^ This noun is spelled as one word.
Martin Duke Former IESG member
No Objection
No Objection
(2021-07-12 for -06)
Sent
Two nits: Section 3 frequently uses the passive voice (“is instructed” “may be instructed”) and that makes it hard to understand who is doing the instruction. Please add subjects to at least the first of these sentences. You frequently use the structure “allows to [verb]”. There are many ways to fix this gramatically, but recommend simply deleting “allows to” and conjugating the verb correctly.
Robert Wilton Former IESG member
No Objection
No Objection
(for -06)
Not sent