Subject: Re: [I2nsf] AD Review of draft-ietf-i2nsf-nsf-facing-interface-dm-10

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Wed, 3 Feb 2021 01:45:28 +0900

Hi Roman,
Tim, Patrick and I have revised the NSF-Facing Interface YANG Data Model
according to your comments:
https://tools.ietf.org/html/draft-ietf-i2nsf-nsf-facing-interface-dm-11

I attach the revision letter to explain how I have reflected your comments
on the revision.

Thanks for your valuable comments and encouragement.

Best Regards,
Paul

On Sat, Oct 31, 2020 at 11:47 AM Roman Danyliw <[email-masked]> wrote:

> 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-masked]
> https://www.ietf.org/mailman/listinfo/i2nsf
>

-- 
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
Department Head
Department of Computer Science and Engineering
Sungkyunkwan University
Office: +82-31-299-4957
Email: [email-masked], [email-masked]
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>