Subject: Re: [yang-doctors] [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Fri, 28 Aug 2020 22:39:48 +0900

Hi Jan,
I have addressed all your comments and submitted the revised draft:
https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-10

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

Thanks.

Best Regards,
Paul

On Thu, Jul 23, 2020 at 2:00 AM Jan Lindblad <[email-masked]> wrote:

> Paul,
>
> Good work with the module, and sorry for the slow response. I have been
> OOO. I read through this again today, and I have some comments, if you are
> interested. I guess this is not part of any formal review any more.
>
>
> o  Figure 1: Not sure I understand what the arrow from "Consumer-Facing
> Interface Information Model" ---> "Consumer-Facing Interface Data Model"
> means, but it probably does no harm.
>
> o  Figure 5: The tree diagram leafref paths are strange (e.g.
>  -> /../../user-group/name)
>
> o  Figure 7, figure 12: The UML diagram cardinality is given as "1..n" in
> several places. In the actual YANG, the cardinality is "0..n"
>
> o  Section 5: The endpoint groups are mapped to a single IP or IP range.
> Is that sufficient for your use cases? Also, much of this information re IP
> addresses for users, devices and geo locations in the world are probably
> available in other systems with most network operators. Is it advisable to
> duplicate that information here? Sounds difficult to keep all this
> information in sync.
>
> o  Section 6: Threat signatures and content patterns can be configured
> here. Is the expectation that the I2NSF client (operator?) configures these
> patterns, and the I2NSF server communicates these patterns to the threat
> feed servers, as a sort of controller? How this part of the model would be
> used is not clear to me.
>
> o  Section 9: The examples should use prefixed identity names. For example
>       <protocol>http</protocol>  should be
> <protocol>i2nsf-cfi:http</protocol>
> and
>             <day>monday</day>  should be   <day>i2nsf-cfi:monday</day>
>
> o Section 9.2 and 9.3: Even though the examples text talks about the value
> of "destination", no such tag is actually present in the XML.
>
>
> Then in the YANG module itself:
>
> o  In ip-ranges like container range-ipv4-address, what happens if either
> of start- or end- address is omitted? Maybe explain in the description, or
> make both leafs mandatory?
>
> o  In container period there are several when-expressions that mean
> something else than you think:
>             container period{
>               when
>
> "/i2nsf-cfi-policy/rules/rule/event/frequency!='only-once'";
> By using an abosolute path like this, the XPath expression looks for
> matching instances across all rules. This means this expression is true as
> soon as there is at least one rule with a frequency != 'only-once'. There
> are several other when expressions here with the same problem. What I think
> you mean is this:
>             container period{
>               when "../../frequency!='only-once'";
> This expression looks only at the frequency leaf in the same rule instance
> as the period container is in. The other when expressions can be fixed in a
> similar way.
>
> o  leaf frequency: What happens if this leaf is set to weekly and no day
> is specified? Or monthly, etc? One way of modeling this to avoid the
> problem is to make the leaf-list day, leaf-list date, leaf-list month etc a
> choice, so that the frequency is implicit by configuring a day, date or
> month. If none of them are set, that would mean only-once. Just a thought.
>
>
> Best Regards,
> /jan
>
>
> On 13 Jul 2020, at 14:20, Mr. Jaehoon Paul Jeong <[email-masked]>
> wrote:
>
> Hi Jan,
> I have uploaded the revised draft into the IETF repository:
>
> https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-09
>
>
> I attach the revision letter, too.
>
> Could you review the draft and complete your YANG review if you are
> satisfied with the revision?
>
> https://datatracker.ietf.org/doc/draft-ietf-i2nsf-consumer-facing-interface-dm/
>
>
> Thanks.
>
> Best Regards,
> Paul
>
>
> On Sat, Jul 11, 2020 at 4:44 PM Mr. Jaehoon Paul Jeong <
> [email-masked]> wrote:
>
>> Hi Jan and Tom,
>> I have revised our I2NSF Consumer-Facing Interface (CFI) Data Model Draft
>> according to both your comments.
>>
>> Jan,
>> I attach the revised draft and the revision letter to explain how I have
>> reflected your comments one by one.
>>
>> Tom,
>> the references to RFC inside our YANG module cannot be cited in my I-D
>> XML file, so I cannot include them
>> in Normative References.
>>
>> Also, the choice of the prefix  is i2nsf-cfi.
>>
>> I put "Note: This section is informative" for Sections 7 and 10, which
>> include XML configuration examples.
>>
>> If you have further comments, please let me know by July 12, 2020, in EST.
>> If possible, I want to post this revision on July 13, 2020 after
>> reflecting your further comments on the revision.
>>
>> Thanks.
>>
>> Paul
>>
>>
>> On Fri, Mar 20, 2020 at 2:25 AM Jan Lindblad <[email-masked]> wrote:
>>
>>> Paul,
>>>
>>> Thank you for all your work with the module, and for the reminder for me
>>> to verify all the changes.
>>>
>>> I am afraid I think the module is still not ready for last call, even if
>>> it is better shape than ever thanks to your efforts. I went through the
>>> module from top to bottom, so this is sorted in order of appearance.
>>>
>>> Line 107-204: The following identities are declared in the module, but
>>> never referenced. They should either have a common base with something, or
>>> be referenced somewhere. If not, why are they defined here? They currently
>>> serve no purpose in this YANG module.
>>>   identity ddos {
>>>   identity enforce-type {
>>>   identity admin {
>>>   identity time {
>>>
>>> Line 377: Defining a custom date-and-time type seems odd. You should
>>> probably use one that has already been defined
>>>   typedef date-and-time {
>>>
>>> Line 513: The leaf represents the name of a user, but the format is
>>> undefined. What should be the format for the string value? How would a user
>>> know what to configure here? Email addresses? If implementation dependent,
>>> say so.
>>>     leaf name {
>>>       type string;
>>>       description
>>>         "This represents the name of a user.";
>>>
>>> Line 518: If no IP address information is specified for the user-group,
>>> what happens then? Is the user access accepted, rejected, or something else?
>>>     uses ip-address-info;
>>>
>>> Line 658: Key leaf declared mandatory. All keys are mandatory, so
>>> mandatory is not needed on this leaf.
>>>     leaf policy-name {
>>>       type string;
>>>       mandatory true;
>>>
>>> Line 664: Users mentioned in the owners-ref should have full CRUD
>>> privileges to the policy. But what about everyone else? Should they have
>>> R(ead) privileges? Can anyone create new policies? If not, who can? If
>>> someone creates a policy, but does not mention his own name among owners
>>> (e.g. misspells or does not get the format right), he will not be able to
>>> modify or remove the policy. If no owner is mentioned, then noone can.
>>>     uses owners-ref;
>>>
>>> Line 673: Key leaf declared mandatory. All keys are mandatory, so
>>> mandatory is not needed on this leaf.
>>>         leaf rule-name {
>>>           type string;
>>>           mandatory true;
>>>
>>> Line 682: Users mentioned in the owners-ref should have full CRUD
>>> privileges to the rule. But what about everyone else? Should they have
>>> R(ead) privileges? Can anyone create new rules, or only those that have
>>> full CRUD privileges for the policy? If someone creates a rule, but does
>>> not mention his own name among owners (e.g. misspells or does not get the
>>> format right), he will not be able to modify or remove the rule.
>>>     uses owners-ref;
>>>
>>> Line 697: Choice enforce-type has a description that I can't understand.
>>> What does this mean?
>>>           choice enforce-type {
>>>             description
>>>               "There are two different enforcement types;
>>>               admin, and time.
>>>               It cannot be allowed to configure
>>>               admin=='time' or enforce-time=='admin'.";
>>>
>>> Line 703: In case of enforce-type admin (whatever that means), a string
>>> value needs to be configured. What are the valid values for this leaf?
>>>             case enforce-admin {
>>>               leaf admin {
>>>                 type string;
>>>                 description
>>>                   "This represents the enforcement type
>>>                   based on admin's decision.";
>>>
>>> Line 711: In case of enforce-type time, three times can be configured.
>>> What is the relation between enforce-time, and the other two (begin-time,
>>> end-time)?
>>>             case time {
>>>               container time-information {
>>>                 description
>>>                   "The begin-time and end-time information
>>>                   when the security rule should be applied.";
>>>                 leaf enforce-time {
>>>                   type date-and-time;
>>>                   description
>>>                     "The enforcement type is time-enforced.";
>>>                 }
>>>                 leaf begin-time {
>>>                   type date-and-time;
>>>                   description
>>>                     "This is start time for time zone";
>>>                 }
>>>                 leaf end-time {
>>>                   type date-and-time;
>>>                   description
>>>                     "This is end time for time zone";
>>>                 }
>>>
>>> Furthermore, the locally defined date-and-time type used includes both a
>>> date and time, which seems to be at odds with the example configurations in
>>> the draft. Example 9.2:
>>>   <rules>
>>>     <rule>
>>>       <rule-name>block_access_to_sns_during_office_hours</rule-name>
>>>       <event>
>>>         <time-information>
>>>           <begin-time>2020-03-11T09:00:00.00Z</begin-time>
>>>           <end-time>2020-03-11T18:00:00.00Z</end-time>
>>>
>>> In the example, the rule-name "block_access_to_sns_during_office_hours "
>>> suggests that the begin-time and end-time should be times of day between
>>> which the policy should be enforced. E.g. every day between 9.00 and 18.00.
>>> If that is a valid use case, using a time type with a date doesn't make
>>> much sense. In the context of the policy that repeats "daily", how should
>>> the start date-and-time value "2020-03-11T09:00:00.00Z " be interpreted?
>>> What if it was "monthly"?
>>>
>>> Line 736: In the frequency leaf, the enumeration value only-once is for
>>> rules that don't repeat. But how long do they apply? A single packet? A
>>> single time the rule is triggered? How does a user know if the rule is
>>> still in effect, i.e. if the "once" has happened or not?
>>>               enum only-once {
>>>
>>> Line 835: Maybe it's just my limited understanding of how threat-feeds
>>> work, but I wonder i this construct with source and destinations for threat
>>> feeds is meaningful?
>>>           container threat-feed-condition {
>>>             description
>>>               "The condition based on the threat-feed information.";
>>>             leaf-list source {
>>>               type leafref {
>>>                 path
>>> "/i2nsf-cfi-policy/threat-preventions/threat-feed-list/name";
>>>               }
>>>             description
>>>               "Describes the threat-feed condition source.";
>>>             }
>>>             leaf dest-target {
>>>               type leafref {
>>>                 path
>>> "/i2nsf-cfi-policy/threat-preventions/threat-feed-list/name";
>>>               }
>>>             description
>>>               "Describes the threat-feed condition destination.";
>>>
>>> Line 920: Location groups can be configured, but there seems to be no
>>> references to them. How are they supposed to be used?
>>>       list location-group{
>>>         key "name";
>>>         uses location-group;
>>>
>>> Line 931: Regarding point 16.1 in your revision letter, you say "We
>>> think list type of threat-feed-list can be configured more than one feed of
>>> the same type". I'm afraid that is not the case with the current YANG
>>> model. If you do wish to allow more than one threat-feed-list for the same
>>> threat-feed-type, you need to add an additional key to your
>>> threat-feed-list.
>>>       list threat-feed-list {
>>>         key "name";
>>>         description
>>>           "There can be a single or multiple number of
>>>           threat-feeds.";
>>>         uses threat-feed-info;
>>>
>>> ...
>>>   grouping threat-feed-info {
>>>     description
>>>       "This is the grouping for the threat-feed-list";
>>>     leaf name {
>>>       type identityref {
>>>         base threat-feed-type;
>>>
>>>
>>> Generally, the indentation in the module is much improved. Some lines
>>> are still a bit off, however, so I would recommend using a tool that
>>> indents consistently.
>>>
>>> Generally, I also wonder whether there has been any discussion with
>>> implementors around the admin security model proposed here. As noted
>>> before, it's a bit different from everything else I have seen. Is it well
>>> thought through? Do implementors feel this is doable and user friendly?
>>> Currently there are no examples involving owner. Perhaps an example that
>>> sheds some light over how different users create, modify and see the
>>> various rules would shed some light over this.
>>>
>>> Best Regards,
>>> /jan
>>>
>>>
>>>
>>>
>>>
>>> On 18 Mar 2020, at 18:41, Mr. Jaehoon Paul Jeong <[email-masked]>
>>> wrote:
>>>
>>> Hi Jan,
>>> Could you update the state of YANGDOCTORS Last Call Review on
>>> I2NSF Consumer-Facing Interface YANG Data Model  if the updates are fine
>>> to you?
>>>
>>>
>>> https://datatracker.ietf.org/doc/draft-ietf-i2nsf-consumer-facing-interface-dm/
>>>
>>> I think your comments are all addressed in this version.
>>>
>>> Thanks.
>>>
>>> Best Regards,
>>> Paul
>>>
>>> On Thu, Mar 12, 2020 at 1:15 AM Mr. Jaehoon Paul Jeong <
>>> [email-masked]> wrote:
>>>
>>>> Hi Jan,
>>>> We authors have addressed your comments with the revision:
>>>>
>>>> https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-08
>>>>
>>>> I attach a revision letter to explain how to respond to your comments.
>>>>
>>>> If you have further comments, please let me know.
>>>>
>>>> Thanks.
>>>>
>>>> Best Regards,
>>>> Paul
>>>>
>>>> On Tue, Nov 12, 2019 at 1:53 AM Jan Lindblad via Datatracker <
>>>> [email-masked]> wrote:
>>>>
>>>>> Reviewer: Jan Lindblad
>>>>> Review result: Almost Ready
>>>>>
>>>>> This is my YD review of
>>>>> draft-ietf-i2nsf-consumer-facing-interface-dm-07. I
>>>>> have previously reviewed the -05 revision (~end June). I find the new
>>>>> revision
>>>>> much improved, but still with much to discuss. I will call this
>>>>> "almost ready".
>>>>>
>>>>> Generally speaking, I think the YANG module lacks the precision and
>>>>> descriptions needed to foster interoperability. The examples at the
>>>>> end are
>>>>> very enlightening however, and compensate for much of that, but their
>>>>> informal
>>>>> nature can never replace proper YANG. The module usage needs to be
>>>>> mostly clear
>>>>> from the module itself.
>>>>>
>>>>> The management access control model proposed here is, even with its
>>>>> latest
>>>>> adaptation towards NACM, is still quite different from NACM author's
>>>>> original
>>>>> ideas. I will therefore bring this use case up in the NETMOD WG for
>>>>> discussion.
>>>>>
>>>>> 1. Network access control principles
>>>>>
>>>>> Network access control is about which users are able to use the
>>>>> network being
>>>>> managed, for example connect to facebook. The purpose of the NSF
>>>>> module is to
>>>>> control this access. This version of the YANG module is now based on a
>>>>> list of
>>>>> policies.
>>>>>
>>>>> Each policy has a list of rules. Each rule has an event -- condition
>>>>> -- action
>>>>> triplet. This resembles traditional firewall management, which is a
>>>>> good thing,
>>>>> because that concept is stable and much tried. This allows operators
>>>>> to create
>>>>> lists of rules in this style:
>>>>>
>>>>> if pkt.x == 1: drop                     // Rule 1
>>>>> elif pkt.y > 2: alert                   // Rule 2
>>>>> elif pkt.z == 10: pass          // Rule 3
>>>>> else: drop                              // Rule 4
>>>>>
>>>>> This pattern relies heavily on the ability to control the order of the
>>>>> rules.
>>>>> The current model relies on the alphabetical sorting of names rules
>>>>> for the
>>>>> ordering. The YANG trick I would recommend to give operators the
>>>>> ability to
>>>>> insert and move rules as they wish is to add ordered-by user on the
>>>>> list:
>>>>>
>>>>>     list rule {
>>>>>       ordered-by user;  // <== Add this line
>>>>>       leaf rule-name {
>>>>>
>>>>> Nothing is said about what the system should do in case policies
>>>>> conflict. What
>>>>> if one policy says pass, the other drop for the same packet? Please
>>>>> clarify.
>>>>> What should happen to packets that do not match any of the policies?
>>>>>
>>>>> This module also assumes that all users in the operator's organization
>>>>> are
>>>>> listed in one or more NACM groups (e.g. "employees"). That wasn't
>>>>> really the
>>>>> NACM authors' original intent. Even if this could be made to work
>>>>> maybe, there
>>>>> is no strong reason to repurpose the NACM group concept for user
>>>>> network access
>>>>> purposes. It could easily be modeled differently. So in the cases
>>>>> where there
>>>>> are leafrefs to NACM groups when dealing with network access rather
>>>>> than
>>>>> management access, don't use NACM groups.
>>>>>
>>>>>                 leaf src-target {
>>>>>                   type leafref {
>>>>>                     path
>>>>> /nacm:nacm/nacm:groups/nacm:group/nacm:user-name;  //
>>>>>                     <== Point to some other list of network users
>>>>>
>>>>> 2. Management access control principles
>>>>>
>>>>> Management access control is about which users are able to
>>>>> configure/run
>>>>> actions/the policies and rules. IMO, the most controversial aspect of
>>>>> this
>>>>> module has always been its new and creative management access control
>>>>> model. In
>>>>> this revision, the management principles have been remodeled greatly
>>>>> to fit in
>>>>> with NACM. I find this redesign very promising, but the result is
>>>>> still not
>>>>> quite ready for publication.
>>>>>
>>>>> The point where integration with NACM concepts is important is when it
>>>>> comes to
>>>>> allow some users to CRUD the NSF policies and rules themselves. There
>>>>> is now a
>>>>> leaf-list "owners" on each policy and rule which point to a list of
>>>>> NACM
>>>>> groups. My understanding is that the idea is that the NSF module
>>>>> should be seen
>>>>> as a service model that translate high level ownership information to
>>>>> specific
>>>>> NACM rules. It would be good to mention these ideas somewhere in the
>>>>> NSF
>>>>> document.
>>>>>
>>>>>   leaf-list owners {
>>>>>     type leafref {
>>>>>       path /nacm:nacm/nacm:groups/nacm:group/nacm:name;
>>>>>
>>>>> I expect the intent is that any user listed in a NACM group mentioned
>>>>> in the
>>>>> owners list would get full CRUD privileges for the contents of the
>>>>> rule the
>>>>> owners leaf sits on. That is never spelled out anywhere, however.
>>>>>
>>>>> It is a little less clear how leaf-list owners on policy objects
>>>>> should be
>>>>> handled. Should owners listed on a policy object get full CRUD powers
>>>>> over the
>>>>> entire policy, including all the rules inside? Or would they need to
>>>>> be listed
>>>>> on the rules as well? Not clear. Is the intent that users not listed
>>>>> on the
>>>>> policy object are unable to create new rules, but to be able to update
>>>>> rules
>>>>> they are listed as owners of, if any?
>>>>>
>>>>> Who is allowed to create new policy objects? Should users that are not
>>>>> owners
>>>>> get read access to all the policies and rules?
>>>>>
>>>>> Finally, there is an "owner" leaf on each rule with an identityref
>>>>> allowing
>>>>> operators to configure a role name like dept-head or sec-admin. It is
>>>>> marked
>>>>> mandatory, but never included in any of the examples at the end of the
>>>>> document. This makes me think this may be a remnant from bygone times
>>>>> and
>>>>> should be removed from the YANG. If not, an explanation of how to use
>>>>> this
>>>>> leaf, and how it interacts with "owners" needs to be added, and the
>>>>> examples
>>>>> updated.
>>>>>
>>>>> 3. leafrefs crosspointing between policy instances
>>>>>
>>>>> There are six leafrefs that point to various objects inside a policy,
>>>>> e.g. a
>>>>> rule condition pointing to a device group name. None of them restrict
>>>>> what can
>>>>> be pointed to so that only names within the current policy are valid.
>>>>> It is
>>>>> therefore possible to configure the name of a device group defined in a
>>>>> different policy. I suspect this is not the intention. In order to
>>>>> restrict the
>>>>> leafrefs to the same policy, the following predicate can be added:
>>>>>
>>>>>                 leaf-list src-target {
>>>>>                   type leafref {
>>>>>                   path
>>>>>
>>>>> "/i2nsf-cfi-policy[policy-name=current()/../../../../../policy-name]/endpoint-group/device-group/name";
>>>>>                    // <== Add predicate
>>>>>
>>>>> 4. Mandatory to implement all events, conditions, actions
>>>>>
>>>>> Each rule is defined with a choice of different events (admin, time),
>>>>> conditions (firewall, ddos, custom, threat-feed) and actions (pass,
>>>>> drop,
>>>>> alert, mirror, ...). Is the intent that all of these options should be
>>>>> mandatory to implement? The current model requires this. Also, would
>>>>> it make
>>>>> sense to allow additional mechanisms here? If so, it may be good to
>>>>> explain to
>>>>> readers how the set of choices and identities can be extended by
>>>>> implementations.
>>>>>
>>>>> 5. Optional and mandatory elements
>>>>>
>>>>> In this revision of the module, 8 leafs have been marked mandatory. A
>>>>> few of
>>>>> them are list keys, which are conventionally not marked mandatory,
>>>>> since list
>>>>> keys are always mandatory. A few others are skipped in the XML
>>>>> examples at the
>>>>> end of the NSF document, which makes me believe they might not really
>>>>> be
>>>>> mandatory after all.
>>>>>
>>>>> Three leafs have a default, but most leafs are left optional without
>>>>> any
>>>>> default. In many cases I think I understand what it means to not set a
>>>>> leaf,
>>>>> but with the ones listed here, I don't think it clear at all. Either
>>>>> add a
>>>>> default to make it clear, make them mandatory if they should be, or
>>>>> explain in
>>>>> the leaf description what happens if not set.
>>>>>
>>>>> 493: leaf-list name
>>>>> 513: leaf-list protocol
>>>>> 531: leaf geo-ip-ipv4
>>>>> 541: leaf continent
>>>>> 562: leaf feed-server-ipv4
>>>>> 585: leaf payload-description
>>>>> 590: leaf-list content
>>>>> 600: leaf-list owners
>>>>> 870: leaf method
>>>>>
>>>>> 6. Indentation
>>>>>
>>>>> The YANG indentation is mostly wrong. Run the YANG text through pyang
>>>>> or some
>>>>> other tool that can indent the content correctly before you put it
>>>>> into a
>>>>> document.
>>>>>
>>>>> 7. YANG element naming
>>>>>
>>>>> The YANG convention is to not have lists on the top level in the YANG
>>>>> module,
>>>>> but to surround lists with a container. The surrounding container
>>>>> often has a
>>>>> name in the plural and the list in singluar, like this
>>>>>
>>>>> container interfaces {
>>>>>     list interface {
>>>>>
>>>>> To better fit into the world of IETF YANG modules, I'd recommend
>>>>> turning the
>>>>> top level list i2nsf-cfi-policy into this instead:
>>>>>
>>>>> container i2nsf-cfi-policies {
>>>>>     list policy {
>>>>>
>>>>> Further down, I would change container rule to rules:
>>>>>
>>>>> container rules {
>>>>>     list rule {
>>>>>
>>>>> Finally, it is customary to not repeat the names of parent object in
>>>>> the names
>>>>> of elements. For example, in the following:
>>>>>
>>>>> list threat-feed-list
>>>>>     leaf feed-name
>>>>>     leaf feed-server-ipv4
>>>>>     leaf feed-server-ipv6
>>>>>     leaf feed-description
>>>>>
>>>>> all the leafs should normally not repeat "feed-". Just leaf name, leaf
>>>>> server-ipv4, leaf server-ipv6, leaf description. There are many more
>>>>> examples
>>>>> of this throughout the module.
>>>>>
>>>>> The condition choice has many containers with a single leaf inside
>>>>> (e.g.
>>>>> ddos-source). Their purpose is rather unclear to me. Remove?
>>>>>
>>>>>               container ddos-source {
>>>>>                 description
>>>>>                 "This represents the source.";
>>>>>                 leaf-list src-target {
>>>>>
>>>>> Also, I find the name "src-target" rather confusing. How about
>>>>> "source"?
>>>>>
>>>>> 8. No date leaf
>>>>>
>>>>> The draft text near fig 2 talks about a date leaf. There is no date
>>>>> object in
>>>>> this revision of te YANG.
>>>>>
>>>>> "Date:  Date when this object was created or last modified"
>>>>>
>>>>> 9. leaf owner
>>>>>
>>>>> Near fig.3 leaf Owner is mentioned. Is this leaf still current?
>>>>>
>>>>> "Owner: This field contains the onwer of the rule.  For example,
>>>>>              the person who created it, and eligible for modifying it."
>>>>>
>>>>> 10. leaf packet-per-second
>>>>>
>>>>> This is now modeled as uint16. Is this future proof? Many packet flows
>>>>> on the
>>>>> internet exceed 64k pps.
>>>>>
>>>>> 11. container custon-source
>>>>>
>>>>> Misspelled. Should be custom-source
>>>>>
>>>>> 12. identity ddos
>>>>>
>>>>> Is ddos a malware file-type? This is not exactly in line with my
>>>>> intuition.
>>>>>
>>>>> 13. identity protocol-type
>>>>>
>>>>> There are other modules that already define protocol-types. Would it
>>>>> be worth
>>>>> reusing one of them?
>>>>>
>>>>> 14. identity palo-alto
>>>>>
>>>>> Is it a good IETF practice to list vendor names in modules? Can we
>>>>> consider
>>>>> this a protocol name? Is there perhaps an RFC/specification name for
>>>>> it that we
>>>>> could reference instead?
>>>>>
>>>>> 15. grouping ipsec-based-method
>>>>>
>>>>> This grouping contains a list which allows listing none of, either of
>>>>> or both
>>>>> of ipsecike and ikeless. Are all valid configurations?
>>>>>
>>>>> 16. leaf feed-name
>>>>>
>>>>> This leaf is the key in a list, which makes it possible to have at
>>>>> most one
>>>>> feed of each type. If it would make sense to configure more than one
>>>>> feed of
>>>>> the same type, the YANG needs to be updated here.
>>>>>
>>>>> 17. leaf-list content
>>>>>
>>>>> This leaflist is of type string. What is the format of this string?
>>>>> Does the
>>>>> name refer to something?
>>>>>
>>>>> 18. Event types
>>>>>
>>>>> container event has a choice between enforce-admin and time
>>>>> alternatives. Each
>>>>> of those choices have a leaf that allows the operator to configure an
>>>>> identityref to an enforce-type value. What does that mean? What would
>>>>> it mean
>>>>> if an operator configured admin == 'time' (or enforce-time == 'admin')?
>>>>>
>>>>> 19. leaf begin-time, end-time
>>>>>
>>>>> >From the examples, it can be seen that these are meant to be a time
>>>>> of day
>>>>> values. Currently they are modeled as yang:date-and-time, which means
>>>>> they are
>>>>> a concrete time a specific day, e.g. 2019-11-11T16:07. This needs to
>>>>> be changed
>>>>> in order to be what the modeler intended. Perhaps like this:
>>>>>
>>>>> typedef time-of-day {
>>>>>     type string {
>>>>>         pattern '(2[0-3]|[01]?[0-9]):[0-5][0-9]';
>>>>>     }
>>>>> }
>>>>>
>>>>> 20. leaf frequency
>>>>>
>>>>> This leaf is now modeled properly from a YANG perspective. But what
>>>>> does it
>>>>> mean? If this leaf is set to 'once-only', what exactly will happen
>>>>> only once?
>>>>> Please write a description that explains this.
>>>>>
>>>>> 21. Example in Fig.17
>>>>>
>>>>> The example contains XML that refers to "endpoint-group/user-group".
>>>>> There is
>>>>> no such element in the YANG.
>>>>>
>>>>> <endpoint-group
>>>>> xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
>>>>>   <user-group>
>>>>>
>>>>> Furthermore, there is nothing called range-ip-address,
>>>>> start-ip-address,
>>>>> end-ip-address. They are called range-ipv4-address, start-ipv4-address,
>>>>> end-ipv4-address.
>>>>>
>>>>>     <range-ip-address>
>>>>>       <start-ip-address>221.159.112.1</start-ip-address>
>>>>>       <end-ip-address>221.159.112.90</end-ip-address>
>>>>>     </range-ip-address>
>>>>>
>>>>> Finally, there must not be any xmlns attribute on an closing XML tag.
>>>>> So
>>>>>
>>>>> </endpoint-group
>>>>> xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
>>>>>
>>>>> should be
>>>>>
>>>>> </endpoint-group>
>>>>>
>>>>> This happens in several of the examples.
>>>>>
>>>>> 22. Example in Fig.18
>>>>>
>>>>> There is no element called policy any more. It's now i2nsf-cfi-policy.
>>>>>
>>>>>    <policy xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
>>>>>      <policy-name>security_policy_for_blocking_sns</policy-name>
>>>>>
>>>>> The rules are modeled in a container and list, both by the name rule.
>>>>> So there
>>>>> needs to be two <rule> tags.
>>>>>
>>>>>      <rule>
>>>>>        <rule-name>block_access_to_sns_during_office_hours</rule-name>
>>>>>
>>>>> The security-event element is marked mandatory in the YANG, but
>>>>> missing in the
>>>>> example. The times given below may be what is intended, but do not
>>>>> match the
>>>>> date format for the type used (which include a date, etc).
>>>>>
>>>>>        <event>
>>>>>          <time-information>
>>>>>            <begin-time>09:00</begin-time>
>>>>>            <end-time>18:00</end-time>
>>>>>          </time-information>
>>>>>        </event>
>>>>>
>>>>> Since the example is not mentioning leaf frequency, it will have the
>>>>> value
>>>>> 'once-only'. Maybe explain what that means in the context of the
>>>>> example?
>>>>>
>>>>> The condition/firewall-condition says the src-target is mandatory and
>>>>> dest-target optional, exactly like below.
>>>>> condition/custom-destination/dest-target is mandatory and src-target is
>>>>> optional, exactly like below. Is this pure luck, or is there a logical
>>>>> explanation why exactly those should be mandatory, and the example use
>>>>> precisely those?
>>>>>
>>>>>        <condition>
>>>>>          <firewall-condition>
>>>>>            <source-target>
>>>>>              <src-target>employees</src-target>
>>>>>            </source-target>
>>>>>          </firewall-condition>
>>>>>          <custom-condition>
>>>>>            <destination-target>
>>>>>              <dest-target>sns-websites</dest-target>
>>>>>            </destination-target>
>>>>>          </custom-condition>
>>>>>
>>>>> The current YANG model does not allow setting both a
>>>>> firewall-condition and
>>>>> custom-condition. If that should be allowed, the model needs to
>>>>> change. Should
>>>>> it be possible to have multiple firewall- or other conditions? That is
>>>>> not
>>>>> currently possible.
>>>>>
>>>>> This example leaves out the mandatory leaf owner. Is that a sign that
>>>>> it should
>>>>> not be mandatory, or perhaps not exist at all?
>>>>>
>>>>> 23. Example in Fig.19
>>>>>
>>>>> This example lists a firewall-condition with no src-target, which is
>>>>> mandatory.
>>>>>
>>>>>       <firewall-condition>
>>>>>         <destination-target>
>>>>>           <dest-target>employees</dest-target>
>>>>>         </destination-target>
>>>>>       </firewall-condition>
>>>>>
>>>>> Under condition, there is a container rate-limit with a leaf
>>>>> packet-per-second.
>>>>> Is this a trigger value for the condition, or is it an actual limit
>>>>> that the
>>>>> system is expected to enforce? If it's a trigger, it may be good to
>>>>> find a
>>>>> clearer name. If it's enforced, it's placement under condition is
>>>>> deceiving.
>>>>>
>>>>> If a rule's action is set to 'rate-limit', to which rate will it be
>>>>> limited?
>>>>>
>>>>> 24. Security Considerations
>>>>>
>>>>> Section 10 in the NSF document under review is the Security
>>>>> Considerations. I
>>>>> think it would make sense to mention something about the management
>>>>> access
>>>>> control mechanism here, and its relation to NACM.
>>>>>
>>>>> (End of list)
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> I2nsf mailing list
>>>>> [email-masked]
>>>>> https://www.ietf.org/mailman/listinfo/i2nsf
>>>>>
>>>>
>>>>
>>>> --
>>>> ===========================
>>>> Mr. Jaehoon (Paul) Jeong, Ph.D.
>>>> Associate Professor
>>>> Department of Software
>>>> 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>
>>>>
>>>
>>>
>>> --
>>> ===========================
>>> Mr. Jaehoon (Paul) Jeong, Ph.D.
>>> Associate Professor
>>> Department of Software
>>> 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>
>>>
>>>
>>>
>>
>> --
>> ===========================
>> Mr. Jaehoon (Paul) Jeong, Ph.D.
>> Associate Professor
>> 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>
>>
>
>
> --
> ===========================
> Mr. Jaehoon (Paul) Jeong, Ph.D.
> Associate Professor
> 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>
>
> <Revision-Letter-for-I2NSF-Consumer-Facing-Interface-YANG-Data-Model-20200711-v1.pdf>
>
>
>

-- 
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
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>

---

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Fri, 28 Aug 2020 22:42:40 +0900

Hi Tom,
I have included RFCs used in the YANG module as follows:
----------------------------------------------------------------------------
8.1. YANG Module of Consumer-Facing Interface
 This section describes a YANG module of Consumer-Facing Interface.
 This YANG module imports from [RFC6991] and uses the typedef of time
 in [I-D.ietf-netmod-rfc6991-bis]. It makes references to [RFC0854]
 [RFC0913][RFC0959][RFC1081][RFC1631][RFC2616][RFC2818][RFC4250]
 [RFC5321].
----------------------------------------------------------------------------

Thanks.

Best Regards,
Paul

On Fri, Jul 17, 2020 at 6:51 PM tom petch <[email-masked]> wrote:

> On 11/07/2020 08:44, Mr. Jaehoon Paul Jeong wrote:
> > Hi Jan and Tom,
> Yes you can and in fact you must:-)
> You can put anything you want to in Normative References with a
> corresponding [RFC0913] in the text part of the I-D so you add Section
> 8.1 "This YANG module imports from [RFC6991], ... and makes reference to
> [RFC0854], [RFC0913], ......"
>
> Note that all import must have a reference clause and the referenced
> work must appear in Normative References; same technique applies.
>
> Tom Petch
>
> >
> think
> >> list type of threat-feed-list can be configured more than one feed of
> the
> >> same type". I'm afraid that is not the case with the current YANG
> model. If
> >> you do wish to allow more than one threat-feed-list for the same
> >> still a bit off, however, so I would recommend using a tool that indents
> >> consistently.
> >>
> "almost
> >>>> ready".
> end
> network
> >>>> being
> module
> >>>> is to
> >>>> action
> good
> >>>> thing,
> to
> >>>> create
> for
> >>>> the
> ability
> >>>> to
> really
> >>>> the
> maybe,
> >>>> there
> network
> where
> >>>> there
> to
> >>>> fit in
> still
> is
> >>>> now a
> should
> >>>> be seen
> in
> >>>> the
> rule
> >>>> the
> should
> >>>> listed
> on
> >>>> the
> It
> >>>> is
> it
> >>>> make
> since
> >>>> list
> >>>> at the
> add
> >>>> a
> or
> >>>> some
> often
> >>>> has a
> turning
> >>>> the
> the
> >>>> names
> >>>> worth
> it
> >>>> that we
> or
> >>>> both
> most
> >>>> one
> Does
> >>>> the
> it
> >>>> mean
> of
> >>>> day
> >>>> changed
> does
> >>>> it
> only
> >>>> once?
> >>>> there
> missing
> match
> >>>> the
> firewall-condition
> change.
> >>>> Should
> it
> >>>> should
> that
> >>>> the
> find
> >>>> a

---

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Fri, 28 Aug 2020 22:46:02 +0900

Hi Tom,
I have addressed all your comments in the following revision:
https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-10

Here are my answers for your comments:
------------------------------------------------------------------------------------
Some more minor tweaks
s.5.1 /gorup/group/
=> The replacement is done.

YANG module

WG Chairs are not usually listed in the module - they used to be
=> The information of WG Chairs is removed.

description is a bit terse - some quote the Abstract
=> I have improved the descriptions in the YANG module.

YARA, SNORT, SURICATA would benefit from references; they are not ones I
see in TLS or SSH!
=> I have added the references to YARA, SNORT, and SURICATA.

typedef time I see in RFC6991bis
=> I used typedef time in RFC6991bis.

does the ipv6 addresss ever need the interface?
=> Yes, the IPv6 address needs the CFI interface.
   I added an XML example using IPv6 addresses.

start/end ipv4/ipv6 could do with a must end > start
=> I put a description that an IPv4/IPv6 start address is lower than
   an IPv4/IPv6 end address.

   "A range match for IPv4 addresses is provided.  Note that the
    start IPv4 address must be lower than the end IPv4 address.";

   "A range match for IPv6 addresses is provided.  Note that the
    start IPv6 address must be lower than the end IPv4 address.";

geo-ip could do with a reference
=> I added a reference to geo-ip as follows.
   RFC8805: A Format for Self-Published IP Geolocation Feeds

s.9.1 221.159 is not a documentation address - see RFC5737
=> I used documentation addresses for IPv4 from RFC5737.
   I also used documentation addresses for IPv6 from RFC3849.

IESG often expect an ipv6 example alongside ipv4
=> I added an XML example using IPv6 addresses in Figure 19.

s.12 Registrant should be IESG
=> I modified the IANA considerations section such that Registrant is the
IESG.

prefix is not that of the module
=> I am not sure of this comment. I think we use the correct prefix of
"cfi-policy".
   CFI stands for Consumer-Facing Interface.
------------------------------------------------------------------------------------

Thanks for your valuable comments.

Best Regards,
Paul

On Fri, Jul 17, 2020 at 8:07 PM tom petch <[email-masked]> wrote:

> On 17/07/2020 10:51, tom petch wrote:
> Some more minor tweaks
> s.5.1 /gorup/group/
> YANG module
> WG Chairs are not usually listed in the module - they used to be
>
> descripton is a bit terse - some quote the Abstract
>
> YARA, SNORT, SURICATA would benefit from references; they are not ones I
> see in TLS or SSH!
>
> typedef time I see in RFC6991bis
>
> does the ipv6 addresss ever need the interface?
>
> start/end ipv4/ipv6 could do with a must end > start
>
> geo-ip could do with a reference
>
> s.9.1  221.159 is not a documentation address - see RFC5737
>
> IESG often expect an ipv6 example alongside ipv4
>
> s.12 Registrant should be IESG
>
> prefix is not that of the module
>
>
> Tom Petch
>
>
> > Tom Petch
> >
> >>
> >>>>> Each policy has a list of rules. Each rule has an event --
> >>>>> condition --
> >>>>> action
> >>>>> because that concept is stable and much tried. This allows
> >>>>> operators to
> >>>>> create
> >>>>> this revision, the management principles have been remodeled
> >>>>> greatly to
> >>>>> fit in
> >>>>> allow some users to CRUD the NSF policies and rules themselves.
> >>>>> There is
> >>>>> now a
> >>>>> NACM rules. It would be good to mention these ideas somewhere in
> >>>>> the NSF
> >>>>> document.
> >>>>>
> >>>>>    leaf-list owners {
> >>>>>      type leafref {
> >>>>> I expect the intent is that any user listed in a NACM group
> >>>>> mentioned in
> >>>>> the
> >>>>> entire policy, including all the rules inside? Or would they need
> >>>>> to be
> >>>>> listed
> >>>>> on the rules as well? Not clear. Is the intent that users not
> >>>>> listed on
> >>>>> the
> >>>>> document. This makes me think this may be a remnant from bygone
> >>>>> times and
> >>>>> be pointed to so that only names within the current policy are
> >>>>> valid. It
> >>>>> is
> >>>>> mandatory to implement? The current model requires this. Also,
> >>>>> would it
> >>>>> make
> >>>>> end of the NSF document, which makes me believe they might not
> >>>>> really be
> >>>>> mandatory after all.
> >>>>>
> >>>>> Three leafs have a default, but most leafs are left optional
> >>>>> without any
> >>>>> but with the ones listed here, I don't think it clear at all.
> >>>>> Either add
> >>>>> a
> >>>>> The YANG indentation is mostly wrong. Run the YANG text through
> >>>>> pyang or
> >>>>> some
> >>>>> Finally, it is customary to not repeat the names of parent object
> >>>>> in the
> >>>>> names
> >>>>> There are other modules that already define protocol-types. Would
> >>>>> it be
> >>>>> worth
> >>>>> reusing one of them?
> >>>>>
> >>>>> 14. identity palo-alto
> >>>>>
> >>>>> this a protocol name? Is there perhaps an RFC/specification name
> >>>>> for it
> >>>>> that we
> >>>>> could reference instead?
> >>>>>
> >>>>> This grouping contains a list which allows listing none of, either
> >>>>> of or
> >>>>> both
> >>>>> identityref to an enforce-type value. What does that mean? What
> >>>>> would it
> >>>>> mean
> >>>>> >From the examples, it can be seen that these are meant to be a
> >>>>> time of
> >>>>> day
> >>>>> a concrete time a specific day, e.g. 2019-11-11T16:07. This needs
> >>>>> to be
> >>>>> changed
> >>>>> Finally, there must not be any xmlns attribute on an closing XML
> >>>>> tag. So
> >>>>>
> >>>>> </endpoint-group
> >>>>>     <policy
> >>>>> The rules are modeled in a container and list, both by the name
> >>>>> rule. So
> >>>>> there
> >>>>> This example leaves out the mandatory leaf owner. Is that a sign
> >>>>> that it
> >>>>> should

---

From: tom petch <[email-masked]>
Date: Fri, 28 Aug 2020 17:21:55 +0100

> I have included RFCs used in the YANG module as follows:

Hi,

YANG import need reference clauses too so you need to add there RFC6991, 
RFC8519 except that what I meant for time was not to copy the definition 
from 6991-bis but to import 6991-bis which means using an import by date 
citing the current date of 6991-bis in order to get a version with date 
defined in it as opposed to the version of the module in RFC6991; this 
makes you dependent on 6991-bis so have to wait for 6991-bis to become 
an RFC before this can become an RFC so using the standard=to-be 
definition is a mixed blessing.

The other references look ok except that RFC1081 RFC1631 RFC2616 seem to 
have been obsoleted by later RFC.

Tom Petch

---

From: tom petch <[email-masked]>
Date: Fri, 28 Aug 2020 17:31:27 +0100

> https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-10

Inline

> Here are my answers for your comments:

See my other note about importing from 6991-bis rather than from 6991

good

>     CFI stands for Consumer-Facing Interface.

Yes indeed you do - my mistake.  What I had intended to say, looking at 
other NSF modules, was that there are a number of NSF modules and the 
chosen prefix have nothing in common.  Bear in mind that all the YANG 
modules get mixed up together on the box so while the prefix need to be 
compact, there is something to be said for them to be meaningful so 
RTGWG modules could start rt... or MPLS ones mpls... or PCE ones pce.. 
and so on so you could consider using a prefix of nsf... such as nsfcfi 
or if there are several such nsfcfi-p or some such (but that is getting 
a bit long)

Tom Petch