Hi!

I performed an AD review on draft-ietf-i2nsf-nsf-facing-interface-dm-10.  
Thanks for writing it.  

My feedback is as follows:

** (This is the most significant issue) This document makes a significant 
number of normative references to the capability information model.  However, 
the WG has decided not to publish this document 
(https://mailarchive.ietf.org/arch/msg/i2nsf/5SXo9QlIvtIEDOjHKA-d2jM1TUc/).  
These are many dependencies to resolve.  These are an incomplete list I noted:

-- Section 1.  There are a multiple references to the ECA model from 
[I-D.ietf-i2nsf-capability]

-- Section 4.1.  The text is relying on the deprecated 
draft-ietf-i2nsf-capability to describe the resolution strategies and default 
action.

-- Section 4.2.  This section relies on draft-ietf-i2nsf-capability to explain 
the event clause

-- Section 4.3. The condition clause depends on ietf-i2nsf-capability.

-- Section 4.4.  The action clause depends on ietf-i2nsf-capability.  The 
definition of an ingress, egress and log action are not explained.

-- YANG.  identity target-device (and all derived from it) relies on 
draft-ietf-i2nsf-capability

-- YANG.  identity content-security-controls relies on 
draft-ietf-i2nsf-capability.

-- YANG.  identity attack-mitigation-control relies on 
draft-ietf-i2nsf-capability.

-- YANG.  identity ingress-, egress- and default-action rely on 
draft-ietf-i2nsf-capability.

** Section 1. Editorial. s/ NSF-Facing Interface in Interface to Network 
Security Functions (I2NSF) [RFC8329]/NSF-Facing Interface in the Interface to 
Network Security Functions (I2NSF) architecture [RFC8329]/

** Section 1.  Per "Security policy configuration for advanced network security 
functions can be defined in future.", what is this referencing?  Only a few 
sentences into the introduction the scope of the module isn't clear.  What 
makes it "advanced"?

** Section 4.  "Advanced network security functions can be defined in future.", 
this is the second time in two pages that references are made to advanced 
functions.  What is that referencing?

** Section 4.  Editorial. The bulleted list of the YANG module describing a 
policy rule and ECA clauses was stated at the end of Section 1 (at the top of 
page 3) and repeated almost verbatim again in Section 4 (at the bottom of page 
4).  It likely doesn't need to be said twice.

** Section 4.1.  Editorial.  The sentence "This YANG tree diagram shows the 
general I2NSF security policy rule for generic network security functions" is 
repeated twice.

** Section 4.1. Typo. s/resolutation/resolution/

** Section 4.2.  These sentences follow each other and are nearly identical:
   This section shows the YANG tree diagram for an event clause for
   I2NSF security policy rules.

   This YANG tree diagram shows an event clause of an I2NSF security
   policy rule for generic network security functions.  

** Section 4.3.  In Figure three, there is "pkt-sec-ipv4-geoip" and 
"pkt-sec-ipv4-sameip" but in the formal definition of the YANG module in 
Section 5, there are "pkt-sec-ipv4-geo-ip" and "pkt-sec-ipv4-same-ip"  

** Section 4.3.  Per "A condition clause of context is defined ... and 
geography condition", shouldn't that be a "gen-context-condition"?

** Section 4.3.
Note that this document deals only with
   simple conditions of advanced network security functions.  A
   condition clause of more advanced network security functions can be
   defined as an extension in future.  A condition clause can be
   extended according to specific vendor condition features

Previous text notes the possibility for extensions to advanced network security 
features.  Here we are talking about extensions to the condition clause.  Do we 
need a bit of text to explain the extensibility approach?

** Section 4.5.  What makes something an advanced action?  Is it the difference 
between a per packet vs. per flow decision?  Some other kind of state?

** Section 4.5.  Per "An I2NSF IPsec specification is used to define a method 
required to manage IPsec parameters for creating IPsec Security Associations 
(SAs) between two NSFs through either the IKEv2 protocol or the Security 
Controller [I-D.ietf-i2nsf-sdn-ipsec-flow-protection]"

-- this workflow of NSF-to-NSF communication is no longer documented in 
ietf-i2nsf-sdn-ipsec-flow-protection (Section 7.1. and 7.2 were removed in -08) 
after my AD review.  Where is the interplay of two NSFs described in the I2NSF 
architecture?  Could that be explained or cited here.

-- It seems like the Security Considerations should discuss the channel 
security this is enabling

** Section 5.  
(a) The main objective of this data model is to provide both an
   information model and the corresponding YANG data model of I2NSF NSF-
   Facing Interface.  

(b)    The semantics of the data model must be aligned with the information
   model of the NSF-Facing Interface.  

(c) The transformation of the
   information model is performed so that this YANG data model

I don't follow what (b) is saying.  Since the information is the data model, 
how could they diverge?

Per (c), what transformation is being performed?  There is only a YANG (data) 
model specified.  No abstraction with an information exists in this text.

** YANG.  identity icmp.  Typo. s/IPv6 nett header/IPv6 next header/

** YANG.  identity target-device (and all derived from it) relies on 
draft-ietf-i2nsf-capability

** YANG.  identity-target-device.  Completeness of taxonomy:
-- What category would I choose for network infrastructure devices (switch, 
router, etc.)

-- is a server the same as a "PC"?

** YANG identity iot.  Is iot intended to cover all operational technology (OT) 
devices?

** YANG.  identity vehicle.  Is that a car or truck?

** YANG. identity content-security-controls.  RFC8329 seems to only describe 
firewall, IDS and IPS (in section 9.1), but not the others.  Firewall isn't 
mentioned.  Where is the definition of the others?

** YANG.  identity attack-mitigation-control.  RFC8329 is cited, but the 
proposed values derived values (e.g., syn-flood) aren't found in that document. 
 The closest appears to be Section 9.2, but that list of attack types is 
different than what is presented here.

** YANG.  identity i2nsf-ipsec. Typo. s/Exchnage/Exchange/

** YANG.  leaf rule-priority.  With a range of 1..255, does a higher number 
mean a higher priority?

** YANG. leaf session-aging-time.  leaf duration.  With a value of uint16, what 
is the unit, seconds?

** YANG.  container long connection.  Typo. s/enbale/enable/

** YANG.  leaf ipv4-description.  Typo. s/texual/textual/

** Section 5.  pkt-sec-ipv4-geo-ip.  What is the normative reference for this 
format.  Are "Suricata uses GeoIP API with MaxMind database format" the format?

** YANG.  in a few places.  Typo. s/for an tcp/for a tcp/

** YANG.  container packet-security-url-category-condition.  What does "vendors 
can write instructions for context condition that vendor made"?  How is that 
different than the sentence before this text which says that "this is a 
description of the url category condition"?

** YANG.  leaf pkt-sec-alert-rate.  What is the unit of the alert rate?  Is 
this packets per second?

** YANG. leaf pkt-sec-alert-rate.  Isn't it also common in DoS mitigation tools 
to handle other rates - flow/sec? bytes/sec?  Should that be supported?

** YANG.  left-list pkt-payload-content.  I don't understand what this means 
"The content keyword is very important in signatures. Between the quotation 
marks you can write on what you would like the                signature to 
match."? What quotation marks?

** YANG. container users-conditions.  It should be noted in the Security 
Considerations explicitly that user identifying information could be exchanged

** YANG.  container user and group. These both contain the text "The user has 
many attributes such as name, id, password, type, authentication mode and so 
on. Name/id is often used in the security policy to identify the user."  
Generically, not disagreement.  However where in this model are these 
attributes represented?  It seems like the options are user-name, tenant 
information"

** YANG.  leaf tenant.  What is "tenant information"?  Is a unint8 sufficiently 
large?

** YANG.  case vn-id.  Please expand vn-id.  The description is a repeat of the 
name.

** YANG.  src and dest-geographic-location.  I don't understand the description 
"This is mapped to ip address. We can acquire destination region through ip 
address stored in the database"

** YANG.  Container advanced-action.    

-- Per "If the packet need be additionally inspected, the packet are passed to 
advanced network security functions according to the profile", what profile?

-- I don't follow the different between a "content-security control" and a 
"attack mitigation control"

-- Per the list of "content-security-controls", "antivirus, ips, ids, url 
filtering, mail filtering, file blocking, file isolate, packet capture, 
application control, voip and volte", where are each of these described?

-- Per the content-security-controls, these seem to be unequal or overlapping 
things (ips is a security technology; url blocking is a technique, possible 
with ips; volte is a protocol)

-- Per attack-mitigation-controls, same question, where are these described? 
What does ipv6 related mean?

-- What is the origin of the attack-mitigation-list - why not "ntp 
amplification attack"?  this seems like an enumeration of DoS types, with some 
reconnaissance?  

** YANG.  leaf description. Typo. s/desription/description/

** YANG.  leaf i2nsf-ipsec.  Typo. s/Exchnage/Exchange/

** Section 6.  Per "For security requirements ... described in Appendix A", 
there is no Appendix A in this document.

** Section 6.  Per "Configuration examples of 
[I-D.ietf-i2nsf-capability-data-model] are registered in I2NSF framework", I 
don't follow what is getting registered with what examples.

** Section 6.  Typo. s/registed/registered/

** Section 6.1.  Figure 7 and 8.  A few questions about both examples.

-- what is SNS?  Please expand the acronym (Social Networking Service?)

-- the text seems to suggest that this block will only occur during business 
hours.  However, the time interval is set as:
     <absolute-time-interval>
      <start-date-time>2019-08-01T09:00:00Z</start-date-time>
      <end-date-time>2019-12-31T18:00:00Z</end-date-time>
     </absolute-time-interval>

Doesn't this mean that all traffic between August 1, 2019 - December 31 2019 
would match (not just 0900 - 1800 every day).

** Section 6.1.  Figure 9.  It doesn't not feel appropriate to call out 
"facebook" and "Instagram", please come up with a more generic example.

** Section 6.1.  Figure 9.  The figure text says web filtering during business 
hours, but the XML doesn't seem to show time constraints.

** Section 6.1.  How does this model distinguish between per-packet vs. 
per-flow operations?  The time and IP based actions of the firewall could be 
executed on a per packet basis.  However, 
to do the URL filtering requires stream reassembly.  The action on URL 
filtering is to drop the packet:

<packet-action>
       <egress-action>drop</egress-action>

but shouldn't it be the flow?

** Section 6.2.  Please use an example domain to represent a malicious domain 
-- @voip.mailcious.example.com

** Section 6.3.  Typo. s/contrl/control/

** Section 8.
    o ietf-i2nsf-policy-rule-for-nsf: The attacker may provide incorrect
      policy information of any target NSFs by illegally modifying this.

I don't follow the legality of modifying the configuration.  Perhaps it would 
be clearer to say that writing to almost any element of the module would 
directly impact the configuration of the security functions on the network -  
completely turning off security monitoring and mitigation capabilities; 
altering the scope of this monitoring and mitigation; creating an overwhelming 
logging volume to overwhelm downstream analytics or storage capacity; or 
creating logging patterns which confuse or rendering useless trained statistics 
or artificial intelligence models.

** Idnits returned:

  == The document doesn't use any RFC 2119 keywords, yet seems to have RFC
     2119 boilerplate text.

  Checking references for intended status: Proposed Standard
  ----------------------------------------------------------------------------

     (See RFCs 3967 and 4897 for information about using normative references
     to lower-maturity documents in RFCs)

  ** Obsolete normative reference: RFC 1700 (Obsoleted by RFC 3232)

  ** Downref: Normative reference to an Informational RFC: RFC 3232

  ** Downref: Normative reference to an Informational RFC: RFC 3849

  ** Downref: Normative reference to an Informational RFC: RFC 5737

  ** Downref: Normative reference to an Informational RFC: RFC 8329

Per RFC3232, I'm not sure why it is being cited.  Is it being used to indicate 
that an IANA registry stores a particular protocol value (e.g., the IPv4 
protocol field)?  If so, then reference the IANA registry instead.

Per RFC3849 and RFC5737, just dropped these references outright.  They aren't 
need to explain the reserved addressed.

Per RFC8329, I've noted above in my feedback on my not understanding how this 
document explain the YANG module elements in which it is being cited.

For the shepherd write-up, please just add that idnit is throwing an error on 
RFC8177, but that is a legitimate reference (in the YANG module)

Regards,
Roman

_______________________________________________
I2nsf mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/i2nsf

Reply via email to