Reviewer: Kyle Rose Review result: Has Issues I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments. This document Has Issues. I don't actually have a lot to say about this document from a security perspective: its purpose is to describe, using YANG, a data model intended as the basis for configuration schemas developed for implementations of the Interface to Network Security Functions (I2NSF) framework. Security considerations for the most part should be addressed in documents describing system architecture or normatively detailing how implementations are to make use of the data model described here. I'm not going to relitigate any such issues here. The main issues I found in this document are ones that I, as someone not terribly familiar with YANG, found troubling from a general engineering perspective. These are best illustrated by example: * Why are `ipvX-prefix`, `-range`, and (the misleadingly-named) `-address` defined here? These concepts are generic enough that they should be in modules of their own to minimize variation among data models and the errors that will inevitably result from capturing the same concept in slightly different ways that are not obvious to the user. * Overall, I have to imagine that much of the `nsfintf` data model is generic enough that it should be captured externally. For instance, `tcp-flags`, `port-range`, `flow-label`, `dscp`, etc. are generally useful elements of an abstract transport data model that they shouldn't be defined here, but rather incorporated from an external data model that is maintained by those in (for example) the transport area. Am I just commenting on the YANG ecosystem in general? If these are standard practices, then the overall ecosystem has major latent problems. Ideally, a particular YANG module seems like it should describe only those elements defined at a particular layer, in this case rules and actions, and use reference external modules for elements that are defined at lower layers. Also some nits: * `ipvX-address` describes a subspace of the global IPvX address space, not a single address. The name is going to cause problems. * The descriptions given are often (usually?) just restatements of the entity name. Example is `identity priority-by-order` described as "Identity for priority by order". The description should provide some value for the user beyond simply restating the name. * The headings in section 5 should be clearly labeled with the word "example", such as "Example Security Requirement 1". * IPv6 addresses in text MUST be represented in lowercase, according to RFC 5952 section 4.3. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
