Telechat Review of draft-ietf-i2nsf-nsf-facing-interface-dm-21
review-ietf-i2nsf-nsf-facing-interface-dm-21-secdir-telechat-melnikov-2022-02-15-00

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