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

"Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com> Sat, 11 July 2020 07:45 UTC

Return-Path: <jaehoon.paul@gmail.com>
X-Original-To: i2nsf@ietfa.amsl.com
Delivered-To: i2nsf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CE8EF3A07A8; Sat, 11 Jul 2020 00:45:50 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.077
X-Spam-Level:
X-Spam-Status: No, score=-2.077 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_FREEMAIL_DOC_PDF=0.01, T_HK_NAME_FM_MR_MRS=0.01, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yfVV0sIrSQq1; Sat, 11 Jul 2020 00:45:45 -0700 (PDT)
Received: from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com [IPv6:2a00:1450:4864:20::12b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 938BB3A07A5; Sat, 11 Jul 2020 00:45:44 -0700 (PDT)
Received: by mail-lf1-x12b.google.com with SMTP id s16so4410170lfp.12; Sat, 11 Jul 2020 00:45:44 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=5rMN8LHWrukWdT27X9Ixu7rzOhagNhksEzWia1/bASY=; b=PBeRMC2aShMiYkd3K03N4eb7DHznP1jxUkJGt61FkcgsvtLC9bW3E0Hc4rRz3gmF4v PE6kp3Zf1YfmsoR4uLLxzIYNCMnsdB4aipmw8TS0kfS2p3JPoHmePJl+WSvvuXquHj2Z p5b0jcCdGcMhXD4+YMq+pI6N79AyP+n7retYyhQR4dXVycT0cfHKyIg98wKutTHDYSAM 5L8T8ghLP/bv1GG2nsTpSA9PStmDqJH3b/LE2qdQxUSjH5yD62hkiQeA8OXwhn5z4AdI FVQL31zG2Moy6TODcjFOOHUzdve8/ThTfWIEVC9tkPKD540PvYvo3x2d49TWa6x98TNU 2nWw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5rMN8LHWrukWdT27X9Ixu7rzOhagNhksEzWia1/bASY=; b=KOWGxVWVFIEY9eW9HY/cQ0vw65lYQpfy3GbfAh8JB+USFXPZFU8zcmUq1q2ZK1bY8V iIWHBOjzqs4n6RWKr3+fFk/uc3Lq0snINZnZhLqQ7abyfIcyZruEJCK3Y812sU04qKnG +7KqiF6vcbGRKIHxKpOaVJcsHQ1mZyDng5F99S+91WaT7dGtMhvDxsotzMzRELmrdmO0 VeKXtw/YsdSzftG5tPcSA5TlCYniF5AUKUjgLV/Bx74OddB4f+iMjFz/KPtfgOEYpAXw a/LadZnZuVtes/QWY+ExjzGvgI8TnUt9O4y74WDZtxh1slEITv9y3vTyRSu7Ol5KEnIm IFjg==
X-Gm-Message-State: AOAM532IuSpMpJTo6vHjyK6HYIsSUCy5csGXe+xbntYrXGqfl6Flt/be VI8yUqWssh91gtzcUoPUZk6Ad4peGqkeHWIvxso=
X-Google-Smtp-Source: ABdhPJxb4xcgSscHYIxNn+B0+xuH4sCmpmFIR+38aNyiFe8z0qXPOikrbREd8gueEnk0KYOf4qUlwJYPkJvnu1MEy/I=
X-Received: by 2002:a19:7d84:: with SMTP id y126mr46443306lfc.149.1594453541789; Sat, 11 Jul 2020 00:45:41 -0700 (PDT)
MIME-Version: 1.0
References: <157349122063.7571.1978842562243958252@ietfa.amsl.com> <CAPK2Dexgk81Saufei3z67E4XZg=LLra1HdTUWU-kU33Pj_o+eg@mail.gmail.com> <CAPK2DeyWEzR6Qy6HPURnKp481mH=y+3O2xpLBS9kLc1MPbcjBg@mail.gmail.com> <44A4E4A8-AF9A-47AF-A31A-8AAACAF0A6BA@tail-f.com>
In-Reply-To: <44A4E4A8-AF9A-47AF-A31A-8AAACAF0A6BA@tail-f.com>
From: "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Date: Sat, 11 Jul 2020 16:44:58 +0900
Message-ID: <CAPK2Dey7GzzAWh8AeKA8e5Ng8skxZBf1SYKGLyuatpZDJ+YPWQ@mail.gmail.com>
To: Jan Lindblad <janl@tail-f.com>, tom petch <daedulus@btconnect.com>
Cc: YANG Doctors <yang-doctors@ietf.org>, "i2nsf@ietf.org" <i2nsf@ietf.org>, skku-iotlab-members <skku-iotlab-members@googlegroups.com>, "Mr. Jaehoon Paul Jeong" <jaehoon.paul@gmail.com>
Content-Type: multipart/mixed; boundary="000000000000f8b9cd05aa25a4ac"
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2nsf/jM8FBzwIWvvrs5yqAo4o5vGry_0>
Subject: Re: [I2nsf] Yangdoctors last call review of draft-ietf-i2nsf-consumer-facing-interface-dm-07
X-BeenThere: i2nsf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "*I2NSF: Interface to Network Security Functions mailing list*" <i2nsf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2nsf>, <mailto:i2nsf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2nsf/>
List-Post: <mailto:i2nsf@ietf.org>
List-Help: <mailto:i2nsf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2nsf>, <mailto:i2nsf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 11 Jul 2020 07:45:51 -0000

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 <janl@tail-f.com> 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 <jaehoon.paul@gmail.com>
> 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 <
> jaehoon.paul@gmail.com> 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 <
>> noreply@ietf.org> 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
>>> I2nsf@ietf.org
>>> 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: jaehoon.paul@gmail.com, pauljeong@skku.edu
>> 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: jaehoon.paul@gmail.com, pauljeong@skku.edu
> 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: jaehoon.paul@gmail.com, pauljeong@skku.edu
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>