Hi! I performed an AD review of draft-ietf-i2nsf-sdn-ipsec-flow-protection-07 and my feedback is as follows:
(1) As written, it wasn't clear to me how this document fits into the I2NSF architecture. There were a number of places in the document that the underlying reference architecture does not appear to link to the one specified in I2NSF. The text at times reads like I2NSF, an SDN architecture or a generic configuration of IPSec. This document needs additional text to frame the work for I2NSF so that it fits into the charter -- if it generalizes to other use cases, I don't see this as an issue. The following is a non-exhaustive list where I saw dissidence. -- Section 1. What is the relationship between the SDN controller referenced here and the I2NSF NOMS per Figure 1 of RFC8329? -- Section 1 makes no reference to I2NSF. -- Section 5. Does the proposed architecture cited in Figure 1 and 2 conform to the "I2NSF Reference Architecture" depicted in Figure 1 of RFC8329. It seems like it should. However: o this document doesn't cite it. It does however cite RFC8192. In Figure 1 and 2 of this document, there is a "client facing interface"/"vendor facing interface"/"Security Controller", but RFC8329 has a "consumer facing interface"/"registration interface"/"Network Operator Management System" o In Figure 1 and 2 of this document there is an I2NSF agent in the NSF which isn't described elsewhere. o The Client-Facing Interface is cited as RFC8192. What is the relationship between this interface and draft-ietf-i2nsf-consumer-facing-interface-dm? o What is the relationship between the NSF Facing Interface in Figure 1 and draft-ietf-i2nsf-nsf-facing-interface-dm? -- Section 5.1.1. How does the scenario of multiple Security Controllers and gateways relate to the I2NSF architecture? -- Section 5.2. Per "As shown in Figure 2, applications for flow protection run on top of the Security Controller", can you clarify what this means? Is it that the functionality associated with translating the policy shared by I2NSF client is built into the security controller? -- Section 5.3. Per "In principle, IKE case is easier to deploy than IKE-less case because current gateways ..." o s/IKE case is/the IKE case is/ o s/than IKE-less case/than the IKE-less case/ o what gateway is being referenced here - it isn't in Figure 1, 2 or RFC8329. RFC8192 defines a security gateway be subsequently doesn't use it again -- Section 5.3. Per "Moreover hosts can install easily an IKE implementation": o s/can install easily/can easily install/ o What are hosts in this context? How are they related to the NSFs? -- Section 5.3.1. "However, when IKE is not used, we have followed a different approach to avoid any packet loss during rekey: the Security Controller installs first the new inbound SAs in both NSFs and then, the outbound IPsec SAs." This text mentions multiple NSFs (i.e., "in both NSFs"), but the I2NSF is scoped to the interface between the controller and NSFs (but not between NSFs). -- Section 7. How do these use cases relate to the I2NSF architecture? There appears to be a set of actions taken by an administrator that is at a different level of abstraction that specified by I2NSF o Section 7.1 and 7.2 discuss communication between NSFs which isn't covered in I2NSF o Section 7.2 discusses communication though an "east-west" interface between controllers which isn't covered in I2NSF o I also couldn't find a reference to I2NSF interactions where there are multiple controllers operated by different administrative entities. (2) Abstract. Editorial. The sentence "This document describing ..." does not parse for me and I recommend tying it to the I2NSF architecture: OLD: This document describes how providing IPsec-based flow protection by means of a Software-Defined Network (SDN) controller (aka. Security Controller) and establishes the requirements to support this service. NEW: This document describes how to provide IPsec-based flow protection by means of a Software-Defined Network (SDN) controller (I2NSF Security Controller) and establishes the requirements to support this service. (3) Abstract. Per "The document focuses on the NSF Facing Interface ...", which NSF Facing Interface? I2NSF or something more generic? (4) Section 1. Per "Recently, several network scenarios are considering a centralized way ...", this sentence notes that there are several network scenarios motivating the work but only names one, SD-WAN. Either enumerate the others or note SD-WAN only. (5) Section 1. Editorial. OLD: SD-WAN is based on IPsec as underlying security protocol and aims to provide flexible, automated, fast deployment and on-demand security network services such as IPsec SA management from a centralized point. NEW: SD-WAN is based on IPsec as an underlying security protocol and aims to provide flexible, automated, and rapid deployment; and enable on-demand security network services such as IPsec SA management from a centralized point. (6) Section 1. "First, this document exposes the requirements to support the protection of data flows using IPsec [RFC4301].", I had trouble identifying the SDN-specific requirements in the text. In what section is state stated? (7) Section 1. Editorial s/manage autonomously/autonomously manage/ (8) Section 1. Per "The analysis of the host-to-gateway (roadwarrior) scenario is out of scope ...", -- Consider using a different, less colloquial phrase than "roadwarrior" -- EDITORIAL: OLD: The analysis of the host-to-gateway (roadwarrior) scenario is out of scope of this document NEW: Consideration for the host-to-gateway (roadwarrior) scenario is out of scope of this document (9) Section 1. Editorial. s/pays attention to the challenge/addresses the challenge of the/ (10) Section 3. Per [I-D.ietf-i2nsf-terminology]: -- [I-D.ietf-i2nsf-terminology] is expired. Are you sure you want to use? -- Is there a reason to redefine NSF here as it is defined in [I-D.ietf-i2nsf-terminology] (11) Section 3. Editorial. s/dst\/src/destination\/source/ (12) Section 3. Typo. s/outboud/outbound/ (13) Section 4. This section was a helpful scoping. I would have benefit from reading it earlier (Section 1) (14) Section 4. Per "... in order to protect specific data flows", between what parts of the I2NSF infrastructure? (15) Section 5.1. Editorial OLD In this case the NSF ships an IKEv2 implementation besides the IPsec support. NEW In this case, the NSF ships an IPSec implementation with IKEv2 support. (16) Section 5.1.1. Per Figure 1 (and applies to Figure 2) -- What is the "application support"? -- What is the "SPD entries Distr." Is that "distribution"? -- What's "data protection and forwarding"? (17) Per Section 5.1.1. Per "In order to support this capability in IKE cases, the following interface requirements need to be met:" -- To what interfaces is this referring? -- Editorial: s/in IKE case/in the IKE case/ (18) Section 5.2.1. The text references an SDN controller. Is that the Security Controller per Figure 2? I recommend consistent language. (19) Section 5.3. Per "As downside, the NSF needs more resources to hold IKEv2", what does "hold IKEv2" mean in this context? Is it an observation that the NSF will need more memory? Storage? Compute? (20) Section 5.3. Per "Moreover, the IKEv2 implementation needs to implement an internal interface" - what is an internal interface in this architecture? (21) Section 5.3. Recommend something more precise than "lighter NSFs". (22) Section 5.3. Per "On the contrary, the overload of creating and managing ...", "overload", to me, implies that the demand for a resource is out stripping the supply. Do you mean "the overhead of creating ..."? Also see the last sentence of this paragraph, "In summary, this overload may ..." (23) Section 5.3.2. Per "Moreover, the Security Controller has a register about all the NSFs ...", what is "has a register"? Is the intent to say that the controller keeps a list of NSFs? (24) Section 5.3.2. "In IKE-less case, if the Security Controller detects that a NSF has lost the IPsec SAs it will delete the old IPsec SAs on the non-failed nodes, established with the failed node (step 1). This prevents the non-failed nodes from leaking plaintext." -- Is there a reason why normative language isn't used to require this clean up (i.e., deleting the old IPSec SAs)? -- Are there constraints on how quickly this needs to happen? (25) Section 5.3.2. Per "Nevertheless other more optimized options can be considered", what is the guidance to implementors with this statement? (26) Section 5.3.3. Per "On the contrary, the IKE-less case does not have any protocol in the NSFs to detect whether they are located behind a NAT or not.", it seems odd to make assumptions about the code in the NSFs. (27) Section 5.3.3. The normative behavior of the IKE-less NAT detection isn't clear. The paragraphs "On the contrary, ..." and "For example ..." don't articulate what's to be done in the NAT case. (28) Section 5.3.4. Per "The assumption in this document is that, for both cases, before a NSF can operate in this system, it MUST be registered in the Security Controller. In this way, when the NSF comes to live and establishes a connection to the Security Controller, it knows that the NSF is valid for joining the system." -- how does this registration and validation occur? I see the text later says "This discover process is out of scope ...", but registration seems different than discovery. -- (editorially) I stumbled over "when the NSF comes to live", maybe "when the NSF starts" (29) Section 6.2. Is the spd-entry having only a single traffic selector the only simplification of RFC4301. If not, list all of the divergences. Otherwise, I recommend explicitly saying only this edit was made. (30) Section 6.2. Typo. s/instead a list/instead of a list/ (31) Section 6.2. Editorial. s/admit a traffic selector per IPsec policy/admit a single traffic selector per IPSec policy/ (32) Section 7. Are the use cases normative text? If not, please say so. Consider moving them to an appendix outside of the normative flow of the text. If the text is normative, then I'll have more feedback as they are underspecified for standards track text. (33) Section 7.1. "Besides, fresh SAD entries will be also generated by the Security Controller and enforced in the NSFs. In this case, the Security Controller does not run any IKEv2 implementation (neither the NSFs), and it provides the cryptographic material for the IPsec SAs." Editorially, what the "beside" is referencing and the double negative in the second sentence created confusion for me. Recommend: NEW: Fresh SAD entries will be also generated by the Security Controller and enforced in the NSFs. As the Security Controller is not running IKEv2, it provide the cryptographic materials for the IPSec SAs. (34) Section 7.2. Step 3. What does "NOTE: This may require extensions in the East/West interface." mean? (35) Section 9. s/MUST exit/MUST exist/ (36) Section 9. Typo. s/to protect of the critical/to protect the critical/ (37) Section 9.0 Per "For example, when NETCONF is used as southbound protocol between the Security Controller and the NSFs, it is defined that TLS or SSH security association MUST be established between both entities", what is the difference between this normative language and Section 9.3's "The lowest NETCONF layer is the secure transport layer, and the mandatory-to-implement secure transport is Secure Shell (SSH) [RFC6242]. The lowest RESTCONF layer is HTTPS, and the mandatory-to-implement secure transport is TLS [RFC8446]."? (38) Section 9. Typo. s/This default policy MUST ... before the NSF contacts with the Security Controller/This default policy MUST ... before the NSF contacts the Security Controller/ (39) Section 9. Per "Moreover, the startup configuration datastore MUST be pre-configured ..." -- where is the "startup configuration datastore" defined in the architecture? -- How is it provisioned (is this out of scope?)? (40) Section 9. Typo. s/always apply first/always first apply/ (41) Section 9. Per "In general, the Security Controller, as typically in the SDN paradigm, is a target for different type of attacks . Thus, the Security Controller is a key entity in the infrastructure and MUST be protected accordingly ...", what are the details here? Section 13 of [ITU-T.Y.3300] says "Moreover, a logically centralized controller can be a single point of failure, and can be a target of malicious attacks, thus special attention is required." and Section 7 of [8192] doesn't mention the security controller at all. (42) Section 9.1. Should configurations adhere to the algorithm recommendations of RFC8247? (43) Section 9.1 and 9.2. Per "The general recommendation is that the Security Controller MUST NOT store ...", is this a recommendation or a normative "MUST NOT". To me, the use of the language "general recommendation" suggests it is optional. I recommend: OLD The general recommendation is that the Security Controller MUST NOT store the IKE credentials after distributing them. NEW The Security Controller MUST NOT store the IKE credentials after distributing them (44) Section 9.1. Editorial. s/One option is to return always .../One option is to always return .../ (45) Section 9.1. Per "Moreover, the PSK MUST have a proper length (e.g. minimum 128 bit length) and strength", what is the recommended guidance - it is that PSKs need to have 128-bit strength? The normative guidance in a parenthetic example is not clear. (46) Section 9.1. "How the NSF generates these cryptographic material (public key/private keys) and export the public key, or it is instructed to do so , it is out of the scope of this document." -- what is meant by "or it is instructed to do so"? -- s/export/exports/ -- s/it is out of the scope/is out of scope/ Regards, Roman _______________________________________________ I2nsf mailing list [email protected] https://www.ietf.org/mailman/listinfo/i2nsf
