Reviewer: Alexey Melnikov Review result: Has Minor 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 defines a YANG data model for configuring security policy rules on Network Security Functions (NSF) in the Interface to Network Security Functions (I2NSF) framework. The YANG data model in this document corresponds to the information model for NSF-Facing Interface in the I2NSF framework. Overall the document reads well and YANG specific security considerations that talk about access control for various elements look sufficient to me. However the document lacks some details important for implementations, specific cases listed below. Issues. identity transformation { base egress-action; description "Identity for transformation. The transformation action is used to transform the packet by modifying its protocol header such as HTTP-to-CoAP translation."; reference "RFC 8075: Guidelines for Mapping Implementations: HTTP to the Constrained Application Protocol (CoAP) - Translation between HTTP and CoAP."; } This is not listed as a choice (in a comment) in "leaf egress-action". Should it be? If it is listed, is this enough to define algorithmic transformations? identity imaps { base application-protocol; description "The identity for Internet Message Access Protocol (IMAP) over TLS"; reference "RFC 9051: Internet Message Access Protocol (IMAP) - Version 4rev2 RFC 2595: Using TLS with IMAP, POP3 and ACAP"; } Thank you for splitting "imap" from "imaps" in -21. In regards to references: please don't reference RFC 2595 for IMAPS here. IMAPS is fully described by RFC 9051, so having a reference to RFC 2595 is misleading. typedef time { type string { pattern '(0[0-9]|1[0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9](\.\d+)?' + '(Z|[\+\-]((1[0-3]|0[0-9]):([0-5][0-9])|14:00))?'; } description "The time type represents an instance of time of zero-duration that recurs every day."; } I think you should also clarify in the description that this includes timezone, for example: "The time type represents an instance of time of zero-duration in the specified timezone that recurs every day." leaf session-aging-time { type uint16; units "second"; description "This is session aging time."; } I can't figure out from the description what this means. Can you give an example? container long-connection { description "A container for long connection. A long connection is a connection that is maintained after the socket connection is established, regardless of whether it is used for data traffic or not."; leaf enable { type boolean; description "If true, the rule is enabled and enforced. If false, the rule is configured but disabled and not enforced."; } leaf duration { type uint16; units "second"; description "This is the duration of the long-connection."; Is this max connection duration or the current duration? } } container url-category { description "Condition for url category"; leaf description { type string; description "This is description for the condition of a URL's category such as SNS sites, game sites, ecommerce sites, company sites, and university sites."; } leaf-list pre-defined { type string; description "This is pre-defined-category. To specify the name of URL database."; } leaf-list user-defined { type string; description "This user-defined-category. To allow a users manual addition of URLs for URL filtering."; } } I think "user-defined" is supposed to be an URL. This needs a Normative Reference. Please use RFC 3986. leaf alert-flow-rate { type uint32; description "The alert rate of flood detection for flows per second of an IP address. If the flows per second of an IP address exceeds the alert rate threshold, an alert will be generated."; } I assume you mean the rate of flow creation requests? E.g. new TCP connection establishment. Please clarify this. container anti-virus { description "Condition for antivirus"; leaf-list profile { type string; description "The security profile for antivirus. This is used to update the security profile for improving the security. The security profile is used to scan the viruses."; } leaf-list exception-files { type string; description "The type or name of the files to be excluded by the anti-virus. This can be used to keep the known harmless files."; Is this the list of filesystem paths? Of File patterns, like "*.exe"? } } container payload { description "Condition for packet payload"; leaf description { type string; description "This is description for payload condition."; } leaf-list content { type string; description "This is a condition for packet payload content."; What does this mean? Can you give some examples? } } In "container users": leaf security-group { type string; description "security-group."; } What does this mean? How is it different from "group"? Thank you, Alexey