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

[tom petch <[e-mail]>]

On 28/08/2020 14:42, Mr. Jaehoon Paul Jeong wrote:
> 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].
> ----------------------------------------------------------------------------

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


> Thanks.
>
> Best Regards,
> Paul
>
> On Fri, Jul 17, 2020 at 6:51 PM tom petch <[e-mail]> wrote:
>
>> On 11/07/2020 08:44, Mr. Jaehoon Paul Jeong 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.
>>
>> 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
>>
>>>
>>> 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 <[e-mail]> 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 <
>> [e-mail]>
>>>> 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 <
>>>> [e-mail]> 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 <
>>>>> [e-mail]> 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
>>>>>> [e-mail]
>>>>>> https://www.ietf.org/mailman/listinfo/i2nsf
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> ===========================
>>>>> Mr. Jaehoon (Paul) Jeong, Ph.D.
>>>>> Associate Professor
>>>>> Department of Software
>>>>> Sungkyunkwan University
>>>>> Office: [phone]
>>>>> Email: [e-mail], [e-mail]
>>>>> 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: [phone]
>>>> Email: [e-mail], [e-mail]
>>>> Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
>>>> <http://cpslab.skku.edu/people-jaehoon-jeong.php>
>>>>
>>>>
>>>>
>>>
>>
>
>