I2NSF Network Security Function-Facing Interface YANG Data Model
draft-ietf-i2nsf-nsf-facing-interface-dm-29

[S5.1; nit]

* 2001:db8:0:1::0/120 should probably be 2001:db8:0:1::/120, which I think
  is more in keeping with RFC 5952 canonical string format.

Thank you for the work on this document.

Thank you for addressing my previous DISCUSS.

Francesca

In Section 3, there's this:

   A default action is used to execute I2NSF policy rule when no rule
   matches a packet.  The default action is defined as pass, drop,
   reject, rate-limit, and mirror.  The default action can be extended
   according to specific vendor action features.  The default action is
   described in detail in [I-D.ietf-i2nsf-capability-data-model].

The second sentence is confusing.  It appears to say the default is all five of those, when I think it means to say is that the default is one of those.

Anywhere "http" or "https" appear as words in prose, but not as part of a URL or part of the YANG module, I believe they should be in all caps since they're acronyms.  See, for instance, the numerous instances of this in Section 5.

A very minor point: I suggest Section 6 be broken into two subsections, one per major action being requested.

Please do update the versioned draft's referenced to their latest version, and as the SecDir review suggested, why not use  RFC 9051 as the imaps reference.

Thank you to the authors and the WG for this document - it seems useful and complete.

Also, thank you to Joe for the OpsDir review, and to the authors for addressing his comments. 

Finally, I wanted to note that this was a really good Document Shepherd writeup, and was very helpful.

Thanks for working on this specification. Thanks to Yoshifumi Nishida for the TSVART review.

A bit of regrets that this specification does not include QUIC as transport protocol. A note in the specification regarding why it does not include QUIC would be nice.

Nits:

  Duplicate https identity in section 4.1

     identity https-2 {
    base application-protocol;
    description
      "The identity for Hypertext Transfer Protocol version 2
       (HTTP/2) over TLS.";
    reference
      "draft-ietf-httpbis-http2bis-07: HTTP/2";}

This document should be tagged as replacing draft-kim-i2nsf-nsf-facing-interface-data-model.

Thanks to Yoshi for the TSVART review, and the authors for both addressing his comments and switching to heavy reliance on RFC8519. I'm glad we're reusing YANG models instead of reinventing slightly different ones.

It appears the RFC8519 port model is inadequate because it doesn't support multiple ranges; would it better to update RFC8519 to include the construct here, or is there some different use case where port lists are needless complexity?

Hi,

Thanks for this document.  I have a few relatively minor comments:

3.2.  Event Clause
 
 - It looks like your tree diagram is reproduced twice?
 
 3.3.  Condition Clause
 
How do the conditions combine?  E.g., if I set both fields in ipv4 and ipv6 in the same rule?  Is the requirements that all fields set in the condition must match for the rule to be evaluated?  Perhaps this could be clarified here, and in the description of the condition container in the YANG module.
 
 
In terms of the YANG:

     identity priority-by-order {
       base priority-usage;
       description
         "Identity for priority by order. This indicates the
          priority of a security policy rule follows the order of the
          configuration. The earlier the configuration is, the higher
          the priority is.";
     }
     
Except for the base identities, where it helpful for the description to make it clear that it is a base identity, I don't think that you need text like "Identity for priority by order", although strictly is does no harm either.  I'll leave it to the authors to decide whether they change this, but if you do, then please consistently change it for all non base identities.


     identity disk-alarm {
       base system-alarm;
       description
         "Identity for disk alarm. Disk is the hardware to store
          information for a long period, i.e., Hard Disk and Solid-State
          Drive. A disk-alarm is emitted when the disk usage is
          exceeding a threshold.";
       reference
         "draft-ietf-i2nsf-nsf-monitoring-data-model-14: I2NSF NSF
          Monitoring Interface YANG Data Model - System alarm for disk";
     }
     
Would "storage-alarm" be a better generic name than disk-alarm?


     identity device-type {
       description
         "Base identity for types of device. This identity is used for
          type of the device for the source or destination of a packet
          or traffic flow.";
       reference
         "draft-ietf-i2nsf-capability-data-model-26:
          I2NSF Capability YANG Data Model";
     }
     
I assume that in many cases, particular packet flows, then NSF would not be able to know what type of device it was?  I wasn't quite sure how this would work.


     identity redirection {
       base egress-action;
       description
         "Identity for redirection. This action redirects the packet to
          another destination.";
     }
     
Could the description clarify the difference between redirection and forwarding please?


5.  XML Configuration Examples of Low-Level Security Policy Rules

Should all the example rules have the same name?  Would they be expected to have different names?

Am I right in understanding that a rule in figure 6 (or 7) are combined with the rule in figure 8.  If so, what in the policy actually binds these together so that the NSFs know to work together?  Is it the common name that binds them all together?  If both v4 and v6 are handling by the same device then you can't have two list entries in the same list with the same key.

Regards,
Rob