Last Call Review of draft-ietf-i2nsf-nsf-facing-interface-dm-17
review-ietf-i2nsf-nsf-facing-interface-dm-17-genart-lc-romascanu-2022-01-25-00

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-i2nsf-nsf-facing-interface-dm-17
Reviewer: Dan Romascanu
Review Date: 2022-01-25
IETF LC End Date: 2021-11-23
IESG Telechat date: Not scheduled for a telechat

Summary:

This document defines a YANG data model for configuring security policy rules
on Network Security Functions (NSF) in the Interface to the Network Security
Functions (I2NSF) framework. It's a solid, well-written and complete document.
It needs to be read in the context and together with several other documents
belonging to the I2NSF deliveries. The document is Ready from the perspective
of Gen-ART with a couple of minor non-blocking issues and a few editorial
problems that could be easily clarified and fixed if needed.

Major issues:

Minor issues:

1. How can RFC 8329 be only an Informative Reference. The Introduction dully
states that the YANG module is based upon the framework / architecture defined
in RFC 8329, and Section 4 uses RFC 8329 in several reference clauses.

2. Section 4.

>         leaf frequency {
               type enumeration

Is this enumeration sufficient (once, daily, weekly, monthly, yearly)? Are not
more cases  needed?  more flexibility?

Nits/editorial comments:

1. Section 3.3:

>  A condition clause of generic network security functions is defined as IPv4
condition, IPv6 condition, TCP condition, UDP condition, SCTP condition, DCCP
condition, and ICMP (ICMPv4 and ICMPv6) condition.

Should not be rather 'or' instead of 'and'?

2. Section 4:

description of identity acces-violation

>       "Identity for access-violation. Access-violation system
          event is an event when a user tries to access (read, write,
          create, or delete) any information or execute commands above
          their privilege."

'above their privilege' is vague - probably meaning not-conformant with the
access profile

3. Section 4

identity memory-alarm

description
         "Identity for memory alarm. Memory is the hardware to store
          information temporarily or for a short period, i.e., Random
          Access Memory (RAM). A memory-alarm is emitted when the RAM
          usage exceeds the threshold.";

memory-alarm is emitted when the memory usage is exceeding the threshold - RAM
example does not really help, the alarm applies to all types of memory

4. Section 4

    identity ot {
       base device-type;
       description
         "Identity for Operational Technology devices";
     }

     identity vehicle {
       base device-type;
       description
         "Identity for vehicle that connects to and shares
          data through the Internet";
     }

reference clauses would help - what is an OT and a 'vehicle' (in this context)?

5. Section 4

>     identity forwarding {
       base egress-action;
       description
         "Identity for forwarding. This action forwards the packet to
          another node in the network.";
     }

'This action forwards ... ' sounds odd. The action consists of forwarding, but
does not perform it. I suggest re-wording. There are a few more such instances
of 'This action [does] ...